microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.76k stars 29.47k forks source link

Syntax from extensions highlighting broken #158379

Closed samuelcolvin closed 2 years ago

samuelcolvin commented 2 years ago

Does this issue occur when all extensions are disabled?: NA - a change to vscode has broken multiple extensions

VS Code Version: 1.67.1 (Universal) Commit: da15b6fd3ef856477bf6f4fb29ba1b7af717770d Date: 2022-05-06T12:37:16.526Z Electron: 17.4.1 Chromium: 98.0.4758.141 Node.js: 16.13.0 V8: 9.8.177.13-electron.0 OS: Darwin arm64 21.4.0 OS Version: MacOS 12.3.1

See https://github.com/samuelcolvin/jinjahtml-vscode/issues/107 for more details and example code.

Steps to Reproduce:

  1. Install "Better Jinja" (or "Jinja") extension
  2. Load the code sample from https://github.com/samuelcolvin/jinjahtml-vscode/issues/107 and change the language to jinja-raw or jinja-html
  3. Highlighting is wrong

I'm the maintainer of "Better Jinja", I've spent a few hours digging through the textmate language definition and trying others, it seems some fairly recent change to VSCode has broken the extension.

Please could you point me to what change caused this, and how I can fix it.

vscodenpa commented 2 years ago

Thanks for creating this issue! It looks like you may be using an old version of VS Code, the latest stable release is 1.70.1. Please try upgrading to the latest version and checking whether this issue remains.

Happy Coding!

samuelcolvin commented 2 years ago

I've just tried with the following, same problem:

Version: 1.70.1 (Universal) Commit: 6d9b74a70ca9c7733b29f0456fd8195364076dda Date: 2022-08-10T06:09:06.916Z Electron: 18.3.5 Chromium: 100.0.4896.160 Node.js: 16.13.2 V8: 10.0.139.17-electron.0 OS: Darwin arm64 21.4.0

RedCMD commented 2 years ago

Are you complaining about vscodes bracket pair colourizer? it affects all languages not just yours

you can turn it off in your settings: editor.bracketPairColorization

samuelcolvin commented 2 years ago

No I'm not.

I'm asserting that the way vscode interprets tmlanguage files has changed in a recent release and is now broken.

Please read the issue and linked issue before replying, failing to do so nullifies my effort and that of others and thereby reduces the quality of future bug reports.

RedCMD commented 2 years ago

How is it broken? example of before/after please

in https://github.com/samuelcolvin/jinjahtml-vscode/issues/107 everyone is only talking about how braces are now highlighted which is a bultin vsocde feature since 1.6 image

I see no difference between 1.7 and 1.48.2 1.48.2: image 1.7: image

RedCMD commented 2 years ago

I do however notice an unclosed bracket in jinja-latex.tmLanguage.json image

and this is not how captures work (jinja-latex.tmLanguage.json and jinja.tmLanguage.json) image

It should be: image

samuelcolvin commented 2 years ago

Well it used to work, for both my extension and the other. Now it doesn't.

Could well be because of stricter parsing.

Maybe you could submit a PR to fix it?

RedCMD commented 2 years ago

because the broken scope names are white even after fixing captures, nothing changes image

I don't see any other issues or dead code everything works as expected

I still don't understand what isn't highlight correctly can you provide a before/after screenshot of what is broken?

I don't see any rule that even remotely targets the white text in the screenshot

samuelcolvin commented 2 years ago

See here https://github.com/samuelcolvin/jinjahtml-vscode/issues/107#issuecomment-1218119792. Sorry I'm on my phone so can't copy and paste the image easily.

Before the change both {{ and both }} would be highlighted.

RedCMD commented 2 years ago

If {{ }} is meant to be light blue that is a problem with the scope: meta.scope.jinja.variable it has no color mapped to it in the dark+ colour theme

should put variable at the front or seperate it from meta.scope.jinja "variable.meta.scope.jinja" or "meta.scope.jinja variable" https://macromates.com/manual/en/scope_selectors#ranking_matches

meta.scope.jinja.variable: image

variable.meta.scope.jinja image

The change to how scopes are mapped to themes was changed in 2017 https://code.visualstudio.com/blogs/2017/02/08/syntax-highlighting-optimizations#_textmate-themes https://code.visualstudio.com/blogs/2017/02/08/syntax-highlighting-optimizations#_parent-selectors

thqby commented 2 years ago

I also have an extension encountering this issue. [](){}, which has been marked as keywords, is still re-color by bracketPairColorization. image

But in the string, it will not. image

samuelcolvin commented 2 years ago

How do I see this information on code?

Also, is there a list of scope names and what should be used when?

thqby commented 2 years ago

Are you referring to Inspect Editor Tokens and Scopes command?

samuelcolvin commented 2 years ago

Yes I guess so, I'm sure I can find it.

Again, is they're a list of approved/recommended scope names?

Guess if I have the inspector I can work it out from other languages.

thqby commented 2 years ago

https://code.visualstudio.com/api/language-extensions/syntax-highlight-guide https://www.apeth.com/nonblog/stories/textmatebundle.html Hope these links will help you.

RedCMD commented 2 years ago

How do I see this information on code?

Keyboard shortcuts => editor.action.inspectTMScopes

Also, is there a list of scope names and what should be used when?

technically there is no convention as everything is decided by your theme but here is some general guidelines https://macromates.com/manual/en/language_grammars#naming_conventions you can of course open up your extensions folder and locate the theme to view its scopes (builtin extensions are in vscodes install location) image

here is a compiled list of all of vscodes dark+ theme scopenames https://github.com/RedCMD/TmLanguage-Syntax-Highlighter/blob/main/scope-names.tmLanguage.json image

alexdima commented 2 years ago

@RedCMD @thqby Thanks a lot for helping out! ❤️

@samuelcolvin The issue https://github.com/samuelcolvin/jinjahtml-vscode/issues/107 is caused by us enabling bracket pair colorization by default in 1.67 which shipped early May 2022. @Kaauw can get the previous behavior by configuring "editor.bracketPairColorization.enabled": false

thqby commented 2 years ago

@alexdima
This can solve the problem, but would it be better to colour tokens marked as bracket pairs? Instead of coloring all bracket pairs except the string as it is now.

alexdima commented 2 years ago

Brackets are ignored in tokens that are strings, comments or regexes. When registering a grammar, it is possible to define which TM scopes are strings, comments or regexes using bracketTypes documented here. There is also a relatively new property called unbalancedBracketScopes which is used for example here -- https://github.com/microsoft/vscode/blob/53e89be20381eb4cb7c4541b9f6a3d8b4502a94e/extensions/typescript-basics/package.json#L67 . @hediet Is this property described somewhere in our documentation?

samuelcolvin commented 2 years ago

what scope should be used for {{ in a jinja template?

e.g. in

Hello {{ first_name }},
alexdima commented 2 years ago

what scope should be used for {{ in a jinja template?

If you want for bracket matching to find the {/} inside, then you can use any scope which doesn't start with string., comment. or regexp..

If you want for bracket matching to ignore the {/} inside, then you can pick any scope e.g. template.boundary and then exclude that scope from bracket matching, by defining "unbalancedBracketScopes": ["template.boundary"]

samuelcolvin commented 2 years ago

okay thanks I'll have a try.

thqby commented 2 years ago

Brackets are ignored in tokens that are strings, comments or regexes. When registering a grammar, it is possible to define which TM scopes are strings, comments or regexes using bracketTypes documented here. There is also a relatively new property called unbalancedBracketScopes which is used for example here --

@alexdima This doesn't work when semantic token type is string . image

hediet commented 2 years ago

Semantic tokens are not considered for bracket matching/auto-closing, as they come in asynchronously.