ziglang / vscode-zig

Zig language support for VSCode
https://marketplace.visualstudio.com/items?itemName=ziglang.vscode-zig
MIT License
534 stars 58 forks source link

Improve Semantic Token Type #213

Open StagramTG opened 3 months ago

StagramTG commented 3 months ago

Hi, I'm toying with theme improvement (cattpuccin) for Zig and I ran into few tokens that are :

So is there any chance that this will be improved in a near futur ? :)

Techatrix commented 3 months ago

In case you don't know this, The vscode-zig extension relies on the ZLS LSP which is responsible for providing the semantic tokens. So everything I am about to talk about is mostly unrelated to the vscode-zig extension.

Not existing att all (first image)

This has very recently been resolved in https://github.com/zigtools/zls/commit/2e5a2a5959c0ba645fd6bfbf93dc467c32458465. Function parameter should now get the semantic token type "parameter" even when zig.zls.semanticTokens is set to partial (which is the default value).

Wrongly applied (second one)

There is no standardized semantic token type for a constant. All pre-defined semantic token types can be found here. Instead a "variable" semantic token type can be emitted with the "readonly" modifier. As an example, look at how clangd does it:

In the past, I had looked into adding the "readonly" modifier to all constants in Zig but this caused some observable changes to the default VS Code themes:

Before After
VS-Code "Dark Modern" VS-Code "Dark Modern"
VS Code Dark Modern don't highlight constants VS Code Dark Modern highlight constants
VS-Code "Light Modern" VS-Code "Light Modern"
VS Code Light Modern don't highlight constants VS Code Dark Modern highlight constants

I have also tested Sublime Text and some colorschemes in neovim but they did not have any observable changes.

Even though I think that an LSP should not decide how to emit semantic token information based on how it affects various editors, I personally find this change worse than the status quo. Zig code is usually heavy on const variables. With the addition of function parameters and capture values which are also constants, the code becomes littered with "readonly" tokens. But this is just my personal option, the code in the images above is a bit cherry-picked by me to highlight this behavior.

One possible solution I am considering is to lie to the editor/client and give non-const variables the "readonly" modifier. This is how it would look like:

Comparison

status quo Screenshot from 2024-06-25 23-29-34 highlight constants Screenshot from 2024-06-25 23-29-49 highlight variables Screenshot from 2024-06-25 23-29-41

This would avoid the "everything becomes blue" problem while still allowing themes to make use of the "readonly" modifier.

I have created a branch of ZLS that implements a config option the experiment with different behaviour for the "readonly" modifier. It can be configured by adding the following to your VS Code settings:

  "zig.zls.path": "/path/to/zls/zig-out/bin/zls",
  "zig.zls.additionalOptions": {
    // `never` means that nothing will get the "readonly" modifier.
    // `mutable_is_readonly` means that non-const variables with get the "readonly" modifier.
    // `constant_is_readonly` means that all constants including function parameters and capture values will get the "readonly" modifier.
    "zig.zls.semanticTokensReadonlyBehaviour": "constant_is_readonly",
  },

It would also be possible to keep the status quo and instead emit a custom non-standardized semantic token modifier like is_mutable to avoid it being used by the default themes of VS Code but still make this information to custom themes. Adding custom semantic token information goes far beyond just highlighting constants. Ideally we would want to use tokens that match actual Zig language constructs like how rust-analyser does it. This would enable color theme authors to create themes that is specifically tailored to Zig. Related Issue https://github.com/zigtools/zls/issues/202

StagramTG commented 3 months ago

Hello @Techatrix and thanks for your answer that is really detailed. I wasn't sure of the LSP role on this side and thank you for all these details that help my understand ! Now, I want to avoid hacky solution since it will surely change in a near futur and this issue don't prevent me writing my code in any way. The last part of your answer seems to be the way to go, in fact a variety of languages use custom token and enable custom theming for them (like rust as you mentioned but Microsoft do the same with C# extensively). I will keep an eye on the zls repo.

Keep up the good work and thank you again ;)