[PATCH] D77702: [clangd] Use token modifiers and more token types for semanticTokens

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks for your thoughts!

In D77702#1972495 <https://reviews.llvm.org/D77702#1972495>, @sammccall wrote:

> Some context for me: I did also start some work on supporting modifiers, and after some experiments with VSCode concluded it was too early (no support in any shipped themes, I saw some hard-to-explain interactions with the default syntax highlighting...).
>  My plan was to revisit once this was in the standard and VSCode had some out-of-the-box support. If there's a goal to implement already in other editors (Theia?) that's a good reason to move forward sooner, but we should be wary of getting too far ahead of VSCode I think, as they're the ones who will mostly decide how the standard mostly gets interpreted.


VSCode Insiders today allows themes and user settings to customize styles for token types -- and, if desired, for combinations of token types and token modifiers -- using the `"editor.tokenColorCustomizationsExperimental"` settings entry, as documented here <https://github.com/microsoft/vscode/wiki/Semantic-Highlighting-Overview#token-styling>. For example:

  "editor.tokenColorCustomizationsExperimental": {
    // Applies to tokens which have the "function" token type and both the "member" and "static" modifiers
    "function.member.static": {
      "foreground": "#123456",
      "fontStyle": "underline"
    }
  }

My intention in this patch is to preserve the ability for users to have e.g. static and non-static methods styled differently, by using things like the above (whether via a theme or their user settings).

In terms of a timeline, my hope is that support for enough modifiers to be able to express our existing token kinds, can be added no later than the removal of the old API (which I understand from another patch is planned for clangd 12?).

(Since you mentioned Theia: I believe its plans are to move towards greater reuse of VSCode components. So, while it had its own implementation of the old semantic highlighting API, for semanticTokens it will likely be using VSCode's implementation, and therefore have the same capabilities and behaviours as VSCode.)

> Some of the most useful modifiers mentioned in the spec (e.g. readonly and definition) are certainly going to apply to a wide range of kinds. And I imagine these will often be orthogonal in presentation too (e.g. underline all definitions, color indicates what it is).

Having modifiers be orthogonal in presentation is certainly a nice goal, but I don't think it's realistic to expect that clients will be able to accomplish this for all modifiers. Simply put, there are more potentially interesting modifiers than there are orthogonal ways to style text in your typical editor (e.g. color, bold, italic, underline, etc.). Even if that weren't the case, tying modifiers to orthogonal aspects of text styling will not necessarily produce the best-looking highlighting.

Notably, the customization ability described above allows associating styles with modifiers both in an orthogonal way (e.g. using `"*.static"` to apply a common style to all static entities) and in a non-orthogonal way (e.g. using `"function.static"` to apply a style specifically to static functions). As a server, I think we should leave it open to clients to use this larger degree of customizability / freedom if they choose to.

> would like our design to reflect this internally (e.g. modifiers as boolean flags in HighlightingToken>) rather than having modifiers+kind encode some internal enum.

That makes a lot of sense, and I was actually thinking of moving in that direction in follow-up patches as well :)

I just wanted to start with a minimal change to avoid regressing the ability to express some of the existing distinctions that appear in our current API.

> 2: The "predefined" modifiers/kinds in the semanticToken spec are a bit of a dilemma.
>  [...]

I think languages are different enough that, while some common concepts may be shared, ultimately each language will end up having its own set of token types and modifiers. It seems to me, based on comments like this one <https://github.com/microsoft/vscode-languageserver-node/blob/91827d97d01fab56ddb8b1e334cfeede6b084e7b/protocol/src/protocol.semanticTokens.proposed.ts#L12>, that the protocol is designed with this in mind.

I do think it's important to agree on a set of token types and modifiers within a language community (so e.g. among C++ language servers and clients). However, I think we (clangd) have enough of a "de facto standard" / front-runner status in the C++ community that we can take the lead on adding new ones? (That's my impression, anyways.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77702/new/

https://reviews.llvm.org/D77702