-
Notifications
You must be signed in to change notification settings - Fork 917
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
[RFC] Monaco Code Editor provider registration #7594
Labels
enhancement
New feature or request
Comments
joshuali925
pushed a commit
that referenced
this issue
Aug 27, 2024
Update to DQL Autocomplete: #7391 Every file removal under `grammar/.antlr` can be ignored. - [x] use official value suggestion methods instead of direct api call - [x] removed language specified configuration - [ ] ~~added memoization for value suggestion to reduce number of calls made~~ - [x] removed core start from query suggestion function - [x] added tests for value suggestion - [x] added more test coverage for other general cases - [ ] ~~[[RFC] Monaco Code Editor provider registration #7594](#7594) made changes based on this RFC~~ - [x] remove grammar/.antlr auto generated files - [x] updated types in code completion and related files - [x] fixed many group value suggestion bugs and edge cases with more robust parser visitor - [x] fix group value NOT suggestion bugs - [ ] ~~added basic keyword syntax highlighting~~ --------- Signed-off-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
opensearch-trigger-bot bot
pushed a commit
that referenced
this issue
Aug 27, 2024
Update to DQL Autocomplete: #7391 Every file removal under `grammar/.antlr` can be ignored. - [x] use official value suggestion methods instead of direct api call - [x] removed language specified configuration - [ ] ~~added memoization for value suggestion to reduce number of calls made~~ - [x] removed core start from query suggestion function - [x] added tests for value suggestion - [x] added more test coverage for other general cases - [ ] ~~[[RFC] Monaco Code Editor provider registration #7594](#7594) made changes based on this RFC~~ - [x] remove grammar/.antlr auto generated files - [x] updated types in code completion and related files - [x] fixed many group value suggestion bugs and edge cases with more robust parser visitor - [x] fix group value NOT suggestion bugs - [ ] ~~added basic keyword syntax highlighting~~ --------- Signed-off-by: Paul Sebastian <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 741c0d6) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
ashwin-pc
pushed a commit
that referenced
this issue
Aug 28, 2024
Update to DQL Autocomplete: #7391 Every file removal under `grammar/.antlr` can be ignored. - [x] use official value suggestion methods instead of direct api call - [x] removed language specified configuration - [ ] ~~added memoization for value suggestion to reduce number of calls made~~ - [x] removed core start from query suggestion function - [x] added tests for value suggestion - [x] added more test coverage for other general cases - [ ] ~~[[RFC] Monaco Code Editor provider registration #7594](#7594) made changes based on this RFC~~ - [x] remove grammar/.antlr auto generated files - [x] updated types in code completion and related files - [x] fixed many group value suggestion bugs and edge cases with more robust parser visitor - [x] fix group value NOT suggestion bugs - [ ] ~~added basic keyword syntax highlighting~~ --------- (cherry picked from commit 741c0d6) Signed-off-by: Paul Sebastian <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
When the code editor from OSD Monaco is in use, there can be a
suggestionProvider
,signatureProvider
,hoverProvider
, and/orlanguageConfiguration
passed in. When done so, in theeditorWillMount
function, all of these providers will be registered. However, if the language within the props changes, these providers cannot be used for the new language.OpenSearch-Dashboards/src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx
Lines 130 to 155 in d7004dc
Describe the solution you'd like
Move this registration section within the
render()
function, like so:OpenSearch-Dashboards/src/plugins/opensearch_dashboards_react/public/code_editor/code_editor.tsx
Lines 154 to 184 in 2b1d01f
The Monaco Language
onLanguage
function (documentation here: https://microsoft.github.io/monaco-editor/typedoc/functions/languages.onLanguage.html) will make sure that these registrations will only trigger when a new language is encountered, which will come in through the props. <- This has also been tested and verified to only trigger in these circumstances.Functionally, if the monaco editor was being used somewhere where one of these providers was registered with a language, it would stay exactly the same with this change. This only adds extra functionality so that the providers can be used with this editor for any additional languages, if they change in the props.
Describe alternatives you've considered
Unmounting the CodeEditor, then remounting a new CodeEditor with a new language and its providers.
This solution seems to require extra computation every single time the language would need to change, which could add up since changing language isn't something that a user would expect eating up a lot of time. Additionally, it seems like the current way its implemented, the Monaco Language
onLanguage
function is redundant here, since only a single event would be emitted, as theeditorWillMount
function would only be triggered once.The text was updated successfully, but these errors were encountered: