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

[vscode] register grammars only when all languages are registered #6966

Merged
merged 4 commits into from
Jan 28, 2020

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Jan 27, 2020

What it does

How to test

  • Use Developer: Inspect Tokens command to see TM token metadata.
    • I know that hover cannot be closed please open another issue. I don't think it is critical.
  • Check that in light theme for embedded languages highlighting is correct, for instance for shell language.
    • Also look at devtools logs, there should not be errors from markdown VS Code extension about missing embedded languages.

Review checklist

Reminder for reviewers

@akosyakov akosyakov added monaco issues related to monaco vscode issues related to VSCode compatibility labels Jan 27, 2020
@akosyakov akosyakov requested review from vince-fugnitto and svenefftinge and removed request for vince-fugnitto January 27, 2020 07:59
@akosyakov
Copy link
Member Author

@corneliusludmann Could you check please whether it resolves the issue for you?

@corneliusludmann
Copy link
Contributor

@corneliusludmann Could you check please whether it resolves the issue for you?

Unfortunately not. I opened this branch in Gitpod, yarn && yarn --cwd examples/browser start ../.. --hostname=0.0.0.0 and changed the README.md + the theme:

image

Latest lines of the log:

oot INFO [e281462a-0cb6-4011-a5d4-12e705e851d1] Start of 63 plugins took: 784.1 ms
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-npm/extension/dist/main)
root INFO [e281462a-0cb6-4011-a5d4-12e705e851d1] Sync of 0 plugins took: 187.4 ms
root INFO [e281462a-0cb6-4011-a5d4-12e705e851d1] Load contributions of 0 plugins took: 0.1 ms
root INFO [e281462a-0cb6-4011-a5d4-12e705e851d1] Start of 0 plugins took: 97.2 ms
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-typescript-language-features/extension/dist/extension)
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-jake/extension/dist/main)
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-gulp/extension/dist/main)
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-grunt/extension/dist/main)
root INFO [nsfw-watcher: 12120] Started watching: /workspace/theia/README.md
root WARN Invalid embedded language found at scope meta.embedded.block.php: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.vs_net: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.dosbatch: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.coffee: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.diff: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.objc: <<0>>
root WARN Invalid embedded language found at scope meta.embedded.block.scala: <<0>>
root INFO [hosted-plugin: 12138] PLUGIN_HOST(12138): PluginManagerExtImpl/loadPlugin(/workspace/theia/plugins/vscode-builtin-markdown-language-features/extension/dist/extension)
root ERROR TypeError: Cannot read property 'startLineNumber' of null
    at TextModel.validateRange (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/vs/editor/editor.main.js:65306:55)
    at TextModel._applyEdits (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/vs/editor/editor.main.js:65513:47)
    at TextModel.applyEdits (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/vs/editor/editor.main.js:65504:29)
    at MonacoEditorModel.push.../../packages/monaco/lib/browser/monaco-editor-model.js.MonacoEditorModel.applyEdits (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/28.bundle.js:1340:31)
    at MonacoEditorModel.<anonymous> (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/28.bundle.js:1431:50)
    at step (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/28.bundle.js:951:23)
    at Object.next (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/28.bundle.js:932:53)
    at fulfilled (https://3000-b8501179-900d-4879-ba18-80b9b77823f6.ws-eu01.gitpod.io/28.bundle.js:923:58)

Let me know if you need more info …

@akosyakov
Copy link
Member Author

@corneliusludmann please try again, I've updated markdown extension before and thought it is irrelevant, but it turned out that a fix and upgrade are required

package.json Outdated Show resolved Hide resolved
@corneliusludmann
Copy link
Contributor

@corneliusludmann please try again, I've updated markdown extension before and thought it is irrelevant, but it turned out that a fix and upgrade are required

Still no luck with text rendering. I even did a hard reload of the tab …

image

This is the proper markdown plugin version, @akosyakov? Just to make sure I have the latest changes running …

@akosyakov
Copy link
Member Author

This is the proper markdown plugin version, @akosyakov? Just to make sure I have the latest changes running …

yes, strange, looking again

@akosyakov
Copy link
Member Author

akosyakov commented Jan 27, 2020

ah, i have an idea, a race between language activations by the monaco and plugin extensions, so monaco activates the shell script without grammars installed... - nope, something else, shell script does not get activated at all, only coloring is installed

@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from 578d7ec to e92f3b6 Compare January 27, 2020 12:29
@akosyakov akosyakov changed the title [vscode] register grammars only when all languages are registered WIP [vscode] register grammars only when all languages are registered Jan 27, 2020
@AlexTugarev
Copy link
Contributor

Screen Shot 2020-01-28 at 04 38 52

I guess the issue is still there, right?

@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from e92f3b6 to 7870338 Compare January 28, 2020 03:54
@akosyakov
Copy link
Member Author

@AlexTugarev still debugging, now it is reproducible always on fresh start and after reload

@akosyakov
Copy link
Member Author

akosyakov commented Jan 28, 2020

vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken... preparing a commit to normalize colors before passing to vscode textmate

akosyakov added a commit that referenced this pull request Jan 28, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this pull request Jan 28, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from a63c1bb to 2330d25 Compare January 28, 2020 12:27
akosyakov added a commit that referenced this pull request Jan 28, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from 2330d25 to 3c3f619 Compare January 28, 2020 12:35
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
akosyakov added a commit that referenced this pull request Jan 28, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from 3c3f619 to 511a6e1 Compare January 28, 2020 12:47
@akosyakov akosyakov changed the title WIP [vscode] register grammars only when all languages are registered [vscode] register grammars only when all languages are registered Jan 28, 2020
@akosyakov
Copy link
Member Author

@corneliusludmann @AlexTugarev could you try again?

…istered

otherwise not yet registered languages are not considered as embedded languages for grammars

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from 511a6e1 to 1783dab Compare January 28, 2020 12:56
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the akosyakov/theme-markdown-text-of-6907 branch from 1783dab to e7c71e1 Compare January 28, 2020 13:20
Copy link
Contributor

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

LGTM! I've tested with light theme and couldn't see any of the known issues. 👍

@akosyakov akosyakov merged commit f5209b3 into master Jan 28, 2020
akosyakov added a commit that referenced this pull request Jan 28, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov deleted the akosyakov/theme-markdown-text-of-6907 branch January 28, 2020 15:51
akosyakov added a commit to akosyakov/theia that referenced this pull request Feb 24, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
JesterOrNot pushed a commit to JesterOrNot/theia that referenced this pull request Mar 12, 2020
vscode textmate tokenization indexes all kind of colors, but monaco textmate tokenization normalizes them to 6 place based hex form, so it works till a user is using normalized form in json files, if not coloring is broken

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
monaco issues related to monaco vscode issues related to VSCode compatibility
Projects
None yet
4 participants