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

feat: add YQL autocomplete #755

Merged
merged 1 commit into from
Mar 14, 2024
Merged

feat: add YQL autocomplete #755

merged 1 commit into from
Mar 14, 2024

Conversation

Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Mar 12, 2024

Closes #611

@Raubzeug
Copy link
Contributor Author

Test stand: https://nda.ya.ru/t/8dR2DKuN74wsF6

Comment on lines 68 to 80
let completionProvider: monaco.IDisposable | undefined;

function disableCodeSuggestions(): void {
if (completionProvider) {
completionProvider.dispose();
}
}

export function registerYQLCompletionItemProvider(database: string) {
disableCodeSuggestions();
completionProvider = monaco.languages.registerCompletionItemProvider('sql', {
triggerCharacters: [' ', '\n', '', ',', '.', '`', '('],
provideCompletionItems: createProvideSuggestionsFunction(),
provideCompletionItems: createProvideSuggestionsFunction(database),
Copy link
Member

Choose a reason for hiding this comment

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

Why it's here, but not in QueryEditor component?

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 decided to do it in the closest place to tenantName emergence.

Copy link
Member

Choose a reason for hiding this comment

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

In this case, Tenant component seems to be a better place, since it's parent for all Tenant page content

Copy link
Member

Choose a reason for hiding this comment

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

This comment should be originally addressed to ObjectGeneral file)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you suggest to lift query string parsing from ObjectGeneral to Tenant? As for me it make sense.

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

There is already query parsing in Tenant, tenantName could be passed in props

artemmufazalov
artemmufazalov previously approved these changes Mar 14, 2024
@Raubzeug Raubzeug merged commit 799a05f into main Mar 14, 2024
4 checks passed
@Raubzeug Raubzeug deleted the add-yql-autocomplete branch March 14, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add autocomplete for basic SQL
2 participants