bazelbuild/vscode-bazel

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

nicolasstucki opened this issue · 8 comments

The following rule matches those keywords but do not result in any highlighting.

"match": "\\b(?<!\\.)(?:(and|or|not|in)(?# 1)|(for|if|else)(?# 2))(?!\\s*:)\\b|(\\*|\\+|-|%|//|/)(?# 3)|(!=|==|>=|<=|<|>)(?# 4)",

Screenshot 2024-05-17 at 11 36 48

These are currently tagged as

"name": "keyword.operator.logical.starlark"

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.

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...

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

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.

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

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.

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

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