-
-
Notifications
You must be signed in to change notification settings - Fork 13
semantic highlighting works with classic editor #45
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general points for improvement, but the functionality looks good overall. Nothing seems up regarding tests, and running the classical demo locally shows semantic highlighting as expected!
Thank you. This is good to go from my side now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, new changes look good to go
@kaisalmen is this already part of |
@cdietrich yes it is. This PR is in and 47 (status from yesterday) and 48 are in. |
hmm somehow i still get the same error in my example. need to digg. problem found: is working now |
@cdietrich Superb! Btw, the langium example now allows you to launch the editor in both modes! |
The wrapper always uses the configuration service from
monaco-vscode-api
what basically makesupdateUserConfiguration
mandatory for configuration updates after start.In classic mode
editorOptions
that are editor specific work, but things likesemanticHighlighting
requires the user configuration update due to the activation of the vscode configuration service. This PR takes thesemanticHighlighting
from theeditorOptions
and applies them the the userConfiguration. This prevents to run into the same problem as #32 .This PR moves
userConfiguration
toEditorAppBaseConfig
which makes it available in both classic mode and vscodeApi mode. It is generally advised to use this mean to configure the app, but we leave the monaco-editor options in for BW-compatibility reasons.Overall aim to allow monarch + semanticHighlighting is achieved.