Syntax highlighting color of keywords should remain constant

[jrom99]

Environment data

• Version: 1.82.2
• Commit: abd2f3db4bdb28f9e95536dfa84d8479f1eb312d
• Date: 2023-09-14T05:51:20.981Z
• Electron: 25.8.1
• ElectronBuildId: 23779380
• Chromium: 114.0.5735.289
• Node.js: 18.15.0
• V8: 11.4.183.29-electron.0
• OS: Linux x64 6.2.0-32-generic (Ubuntu 23.04)
• Language Server version: 2023.9.10 (pyright c70adefc)
• Python version: 3.11.4 64-bit

Code Snippet

1 in []
1 in list()
1 in set()
1 in dict()
1 in {}

Expected behavior

Keywords like in and operators like + should use default color even if custom definitions exist, but still allow linking as implemented in #4467

Actual behavior

Keyword use function color when custom definition exist, and normal color otherwise. I'm not sure if operators colors change as well, since my vision is not good, but they also should follow the same behavior.

Logs

pylance.log

[Molkree]

I'm not sure if operators colors change as well

They do change color, this is with GitHub Light theme:

class Bar:
    def __add__(self, other: object) -> None:
        ...


x = Bar()
_ = x + 1
_ = 5 + 1

[bschnurr]

I believe this is as designed, we wanted to show a different color to indicate when operators were overloaded

[jrom99]

This is not really helpful considering all examples in my screenshot are of default objects, so no overloading was actually done. Even then, I think it hurts readability, as the color schema loses its primary purpose while providing little benefit, supposing linking is still allowed.

[debonte]

@heejaechang, what do you think about this? Should we not use the overridden semantic token modifier in these scenarios? I guess one could argue that this is a decision for the theme author -- they chose to honor the overridden modifier. However, it seems we're at least inconsistent with it (first example above) and I don't think we use overridden in other scenarios (ex. non-dunder class methods). Maybe overridden should only used for overrides in user code?

[heejaechang]

sorry for late reply. currently, we only consider types from builtin as builtin types. I think that is causing issue since things like dicts/collections are comming from typing. we should consider types from typing as builtin types as well. then the issues the original user is seeing will go away.

that said, I think if one doesn't care about seeing which types override operators, he should use theme that doesn't show that. we mark tokens that information for people who cares about it. it is up to the theme and users whether they utilize that info or not.

completely removing that info will affect everyone who wants that info. semantic token provider is supposed to provide categorization of tokens, and it is up to theme to pick which info they will utilize.

[jrom99]

Except that I'm using the default dark theme, the same as many users. It is not helpful to me in that I rely on the current color scheme to separate functions and keywords.

[heejaechang]

unfortunately, only way to do so will be you overriding the theme's color if the theme is using the overriden modifier by default.

you will need something like this in your setting.json file.

"editor.semanticTokenColorCustomizations": {
    "rules": {
      "keyword.overridden": { "foreground": "#0000FF" }
    }
  },

the color, you can use Developer: Inspect Editor Tokens and Scopes command to find out color of the regular keyword for the theme you are using.

[jrom99]

This is not helpful since different token kinds are affected. It hurts readability having the colors be overriden with the function one. I might as well just set a "yellow" theme color. And what about overrides that are not functions such as __setattr__, __getattr__ and __enter__?

[heejaechang]

you can override any color as you want. use the Developer: Inspect Editor Tokens and Scopes to find token names and use that to customize the theme to your liking.

[jrom99]

At this point this would be the same as manually overriding the entire theme just to get the UX that has better readability. I like having the ability to link to the overrides, but the change in color hurts a lot more than it helps.

[heejaechang]

I'm not sure that's true. You probably need at most 3 tokens overridden: keyword.overridden, operator.overridden and class.overridden (there might be one or two more, but these are the ones I can think of off the top of my head).

Also, you don't even need to do that all at once; just start with keyword, class and operator and add more if you find other cases.

[KacieKK]

This issue has been fixed in prerelease version 2024.5.100, which we've just released. You can find the changelog here: CHANGELOG.md