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

Question: Are DiagnosticTags deprecated in favor of semantic tokens? #2024

Open
jfly opened this issue Sep 20, 2024 · 2 comments
Open

Question: Are DiagnosticTags deprecated in favor of semantic tokens? #2024

jfly opened this issue Sep 20, 2024 · 2 comments

Comments

@jfly
Copy link

jfly commented Sep 20, 2024

I've seen a some conversations that imply that semantic tokens obsolete DiagnosticTag:

But the discussion over on microsoft/vscode#92157 has a comment from @alexdima that says:

I am not in favor of adding competing API that gives two distinct ways to achieve the same effect.

So I'm confused. @HighCommander4, maybe you know the answer? Why does clangd need an extension to the Language Server Protocol to do this? Is it related to marking large regions of code as unused?

@HighCommander4
Copy link

HighCommander4 commented Sep 20, 2024

So I'm confused. @HighCommander4, maybe you know the answer? Why does clangd need an extension to the Language Server Protocol to do this? Is it related to marking large regions of code as unused?

Inactive regions are a feature for dealing with conditional preprocessor branches in C/C++ code, for example:

#ifdef WINDOWS
// windows-specific declarations
#else
// declarations specific to other platforms
#endif

If the active build configuration is Windows, the region between the #else and the #endif is inactive (not compiled in this configuration), and we want to visually identify that in the editor in some way.

Diagnostics don't seem appropriate here, as a region being inactive does not indicate anything wrong with the code, or any action that should potentially be taken by the programmer (unlike, say, unused code, where the programmer may want to remove it).

We decided to go with our own protocol rather than using semantic tokens for the following reasons:

  • Semantic tokens did not make it easy for the editor to apply a whole-line style to the region (e.g. grey background color). We hacked it together in vscode with middleware, but it was messy and having a dedicated protocol made it much cleaner.
  • Semantic tokens would overwrite client-side highlighting tokens (e.g. TextMate tokens in VSCode), but it was actually useful to apply client-side highlighting to inactive regions, because it made them more readable to have the individual tokens within an inactive region colored differently.

@jfly
Copy link
Author

jfly commented Sep 20, 2024

Thanks for the quick feedback!

Diagnostics don't seem appropriate here, as a region being inactive does not indicate anything wrong with the code, or any action that should potentially be taken by the programmer (unlike, say, unused code, where the programmer may want to remove it).

I think this is common confusion about the spec. As I understand it (and this is really just me parroting @erictraut here), diagnostic hints are not necessarily something actionable. This confusion seems to partially be due to an unfortunate choice of words in the spec: "unnecessary" implies something is wrong, whereas (IMO), it really should be "unused". I filed a separate issue about this: #2025

I have no idea if this actually would address the issues you raised about implementing the client side of this.

EDIT: Nevermind, after chatting with Neovim, I'm convinced this is just underspecified in the spec. I've filed #2026 to try to address this.

@jfly jfly changed the title Question: Are DiagnosticTags deprecated in favor of Semantic Tokens? Question: Are DiagnosticTags deprecated in favor of semantic tokens? Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants