bazelbuild / vscode-bazel

Bazel support for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=BazelBuild.vscode-bazel
Apache License 2.0
231 stars 76 forks source link

Syntax highlighting of `and`/`or`/`not`/`in` in Starlark #396

Closed nicolasstucki closed 1 month ago

nicolasstucki commented 1 month ago

The following rule matches those keywords but do not result in any highlighting. https://github.com/bazelbuild/vscode-bazel/blob/e5c95e4567a80c82ada48031c9fde5561fc7c957/syntaxes/starlark.tmLanguage.json#L397

Screenshot 2024-05-17 at 11 36 48

These are currently tagged as https://github.com/bazelbuild/vscode-bazel/blob/e5c95e4567a80c82ada48031c9fde5561fc7c957/syntaxes/starlark.tmLanguage.json#L400

nicolasstucki commented 1 month ago

If we change the tag name to the following we get the same highlighting as in a Python file.

- "name": "keyword.operator.logical.starlark" 
+ "name": "keyword.language.logical.starlark"
Screenshot 2024-05-17 at 16 11 38

I am not sure if language is the same category used by the Python extension.

vogelsgesang commented 1 month ago

According to the documentation at https://macromates.com/manual/en/language_grammars (which is referenced by the VSCode docs), keyword.operator.* seems to be the right way to go. I wonder where the keyword.language.* is defined...

vogelsgesang commented 1 month ago

Also VSCode has a lot of mentions of keyword.operator.logical but not a single mention of keyword.language

nicolasstucki commented 1 month ago

I see. My patch probably ended up falling back on keyword and therefore used the same highlighting as in keyword.operator.logical.python. The issue here seems to be that the highlighting of those operators as keywords is special-cased for Python with keyword.operator.logical.python and hence we do not get the same effect with keyword.operator.logical.starlark.

vogelsgesang commented 1 month ago

I wonder if we should use keyword.operator.logical.python.starlark. That way we would inherit the styles from Python by default, but theme authors would still have the option to provide Starlark specific styles, if they wish to do so

nicolasstucki commented 1 month ago

keyword.operator.logical.python.starlark is an interesting approach. If we take this approach would we also change all others .starlark to .python.starlark to inherit all existing highlighings? This would imply changing around 100 names.

vogelsgesang commented 1 month ago

Personally, I would just rename all of them, for consistency. But others might have different opinions... Could you create a PR? And then let's see if anybody dislikes the renaming during the review

nicolasstucki commented 1 month ago

Yes @vogelsgesang, I can create a PR with this change.