Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-5185 SQL Editor queries highlighting is broken #2842

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d801125
CB-5185 adds script parsing on cursor change
sergeyteleshev Aug 14, 2024
50b5420
Revert "CB-5185 adds script parsing on cursor change"
sergeyteleshev Aug 14, 2024
a99cdfc
chore: remove commented out code in _base-code-editor.scss
sergeyteleshev Aug 14, 2024
1def9f8
chore: Refactor SQLCodeEditorPanel to improve performance and readabi…
sergeyteleshev Aug 14, 2024
9384769
Refactor SQLCodeEditorPanel to improve performance
sergeyteleshev Aug 14, 2024
9a032a8
CB-5185 renames delete method for UsersResource so there is no name c…
sergeyteleshev Aug 14, 2024
f7b5ed4
refactor: rename delete method in UsersResource to avoid name collision
sergeyteleshev Aug 15, 2024
eaf5646
Merge branch 'devel' into CB-5185-sql-editor-queries-highlighting-is-…
EvgeniaBzzz Aug 20, 2024
fc4eca2
Merge branch 'devel' into CB-5185-sql-editor-queries-highlighting-is-…
sergeyteleshev Aug 21, 2024
9a03973
CB-5185 fixes incorrect highlight when typing new script
sergeyteleshev Aug 21, 2024
f002c4a
Merge branch 'devel' into CB-5185-sql-editor-queries-highlighting-is-…
sergeyteleshev Aug 21, 2024
1a193ad
CB-5185 execute correct active query
sergeyteleshev Aug 21, 2024
5497ef9
Revert "CB-5185 execute correct active query"
sergeyteleshev Aug 21, 2024
417c0cc
CB-5185 returns activeSegment for queries to execute
sergeyteleshev Aug 21, 2024
c24f436
CB-5185 does not execute query for empty query selected with cursor
sergeyteleshev Aug 21, 2024
79263d3
CB-5185 highlights queries only if cursor is in segment
sergeyteleshev Aug 21, 2024
3e25251
CB-5185 cleanup
sergeyteleshev Aug 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions webapp/packages/core-authentication/src/UsersResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,13 @@ export class UsersResource extends CachedMapResource<string, AdminUser, UserReso
await this.refresh(userId);
}

async delete(key: ResourceKeySimple<string>): Promise<void> {
async deleteUsers(key: ResourceKeySimple<string>): Promise<void> {
await ResourceKeyUtils.forEachAsync(key, async key => {
if (this.isActiveUser(key)) {
throw new Error("You can't delete current logged user");
}
await this.graphQLService.sdk.deleteUser({ userId: key });
super.delete(key);
this.delete(key);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const DeleteUserDialog: DialogComponent<IPayload> = function DeleteUserDi

async function deleteUser() {
try {
await usersResource.resource.delete(props.payload.userId);
await usersResource.resource.deleteUsers(props.payload.userId);
notificationService.logSuccess({ title: 'authentication_administration_users_delete_user_success', message: props.payload.userId });
props.resolveDialog();
} catch (exception: any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,8 @@
}

.editor .active-query {
/* disable because look too ugly */
/*
display: inline-block;
@include mdc-theme-prop(background, secondary, false);
*/
}

.editor .cm-content ::selection {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { action } from 'mobx';
import { useCallback } from 'react';

import { useExecutor, useObservableRef } from '@cloudbeaver/core-blocks';
import { throttle } from '@cloudbeaver/core-utils';
import { debounce } from '@cloudbeaver/core-utils';
import type { ISQLEditorData } from '@cloudbeaver/plugin-sql-editor';

import type { IEditor } from '../SQLCodeEditor/useSQLCodeEditor';
Expand All @@ -21,16 +21,20 @@ interface State {
}

export function useSQLCodeEditorPanel(data: ISQLEditorData, editor: IEditor) {
const script = data.dataSource?.script;

const state: State = useObservableRef(
() => ({
highlightActiveQuery() {
this.editor.clearActiveQueryHighlight();
queueMicrotask(() => {
this.editor.clearActiveQueryHighlight();

const segment = this.data.activeSegment;
const segment = this.data.activeSegment;

if (segment) {
this.editor.highlightActiveQuery(segment.begin, segment.end);
}
if (segment && data.isCursorInActiveQuerySegment) {
this.editor.highlightActiveQuery(segment.begin, segment.end);
}
});
},
onQueryChange(query: string) {
this.data.setScript(query);
Expand All @@ -44,13 +48,29 @@ export function useSQLCodeEditorPanel(data: ISQLEditorData, editor: IEditor) {
);

const updateHighlight = useCallback(
throttle(() => state.highlightActiveQuery(), 1000),
debounce(() => state.highlightActiveQuery(), 300),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is more performance and UX-friendly. Now with 120k rows in SQL editor I have almost no lags on my M1 PRO

[state],
);

function onUpdate() {
const newScript = data.dataSource?.script;
const isScriptChanged = script !== newScript;

editor.clearActiveQueryHighlight();

if (!isScriptChanged) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for the case when we just move the cursor with mouse or keyboard arrows

updateHighlight();
}
}

useExecutor({
executor: data.onUpdate,
handlers: [updateHighlight],
handlers: [onUpdate],
});

useExecutor({
executor: data.dataSource?.onUpdate,
handlers: [onUpdate],
});

useExecutor({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface ISQLEditorData {
readonly editing: boolean;
readonly isScriptEmpty: boolean;
readonly isDisabled: boolean;
readonly isCursorInActiveQuerySegment: boolean;
readonly isIncomingChanges: boolean;
readonly value: string;
readonly incomingValue?: string;
Expand Down
16 changes: 12 additions & 4 deletions webapp/packages/plugin-sql-editor/src/SqlEditor/useSqlEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ interface ISQLEditorDataPrivate extends ISQLEditorData {
}

const MAX_HINTS_LIMIT = 200;
const END_OF_THE_ACTIVE_QUERY_OFFSET = 2;

export function useSqlEditor(state: ISqlEditorTabState): ISQLEditorData {
const connectionExecutionContextService = useService(ConnectionExecutionContextService);
Expand Down Expand Up @@ -88,6 +89,13 @@ export function useSqlEditor(state: ISqlEditorTabState): ISQLEditorData {
activeSegmentMode: false,
},

get isCursorInActiveQuerySegment(): boolean {
const segment = this.activeSegment;
const cursor = this.cursorSegment;

return !!segment && !!cursor && cursor.begin >= segment.begin && cursor.end - END_OF_THE_ACTIVE_QUERY_OFFSET <= segment.end;
},

get activeSegment(): ISQLScriptSegment | undefined {
return this.activeSegmentMode.activeSegment;
},
Expand Down Expand Up @@ -441,16 +449,16 @@ export function useSqlEditor(state: ISqlEditorTabState): ISQLEditorData {
return this.getSubQuery();
}

if (this.activeSegmentMode.activeSegmentMode) {
return this.activeSegment;
}

const result = await this.sqlEditorService.parseSQLQuery(projectId, connectionId, this.value, this.cursor.begin);

if (result.end === 0 && result.start === 0) {
return;
}

if (this.activeSegment) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it below cause I don't want to execute the query if selected line of SQL is empty (no query)

also this.activeSegmentMode.activeSegmentMode not really works so changed it also to this.activeSegment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please revert, this will affect other places, please in general don't touch code that work fine (our code related to query execution is working fine so there no need to change anything) when you have problem with different things (highlighting)

return this.activeSegment;
}

const segment = this.parser.getSegment(result.start, result.end);
return segment;
},
Expand Down
Loading