Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Semantic Token Type #213

Open
StagramTG opened this issue Jun 25, 2024 · 2 comments
Open

Improve Semantic Token Type #213

StagramTG opened this issue Jun 25, 2024 · 2 comments
Assignees
Labels
lsp Issue with language server integration

Comments

@StagramTG
Copy link

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

  • Not existing att all (first image)
  • Wrongly applied (second one)
    image
    First image: Token don't exists
    image
    Second image: Mark as Variable instead of constant

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

@Vexu Vexu added the lsp Issue with language server integration label Jun 25, 2024
@Techatrix
Copy link
Collaborator

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 zigtools/zls@2e5a2a5. 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 zigtools/zls#202

@StagramTG
Copy link
Author

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp Issue with language server integration
Projects
None yet
Development

No branches or pull requests

3 participants