Handle dynamic registration of semantic tokens capability - #60015
Conversation
Roslyn (the C# language server) advertises its capabilities via dynamic registration rather than statically, and only offers semantic tokens when the client advertises semanticTokens.dynamicRegistration. Advertise it, and handle the textDocument/semanticTokens (un)registration so the capability is stored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Thank you for the fix.
I suspect we need to refresh semantic tokens after the registration, similar to #60009 ?
If so, would be great to have a test and a refresh code; otherwise would be interesting to know what's different between two cases.
When a server registers textDocument/semanticTokens dynamically, already-open buffers kept tree-sitter-only highlighting until edited. Refresh semantic tokens after the registration so they re-query, mirroring zed-industries#60009 for code lens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good point, that should be refreshed too, I missed it. I've added the refresh after registration (the case is the same as #60009) plus a test that re-queries the open document. |
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
-
Both tests seem to pass if I omit prod changes from the latest commit — this is wrong.
Either my proposal is incorrect and we already refresh the semantic tokens on capability changes (curious to know, why there is such a difference?) or the test is wrong -
If I apply the test changes only, one of them hangs instead of panicking
First is crucial to fix properly, second is optional but good to have fixed nonetheless.
The test passed even without the registration refresh, because the refresh scheduled on document open was left pending (run_until_parked does not advance the debounce timer) and fired after registration. Drain it first by advancing the clock, and assert via a request counter instead of awaiting the request, so the test fails fast instead of hanging when no re-query happens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You're right. I missed that the editor requests semantic tokens with a ~50ms debounce, and the test never advanced the timer — so the refresh scheduled when the document opens stayed pending and fired after the registration. That's why a request happened even without the prod change (and why I added the clock advance to drain that initial refresh first, and switched to asserting via a request counter. Now the test fails fast (no hang) without the registration refresh and passes with it — verified both ways. Regarding your question the refresh is needed: with the registration refresh removed, the corrected test fails because the already-open document is never re-queried. So we don't already refresh on capability registration. So it looks like the same fix as in that PR #60009. |
Roslyn reports an extended semantic-token legend (`field`, `controlKeyword`, `recordClass`, `extensionMethod`, ...) beyond the standard LSP types. Without rules for them, those tokens are dropped and C# falls back to tree-sitter-only highlighting (e.g. a private field looks like a local variable). This adds `languages/csharp/semantic_token_rules.json` mapping the C#-specific types to theme styles. Standard types stay covered by Zed's built-in defaults, so only the Roslyn-specific ones are listed here (same approach the built-in rust/cpp/go/python rules use). Fixes #85. Moved here from zed-industries/zed#60027 per @MrSubidubi's suggestion. Needs zed-industries/zed#60015 (dynamic registration of `textDocument/semanticTokens`) to actually deliver the tokens — together they resolve #85. ## Showcase <details> <summary>Click to view showcase</summary> **Before** — Roslyn's C# token types aren't mapped, so fields/keywords fall back to tree-sitter (a field looks like a plain variable): <img width="979" height="1089" alt="Screenshot 2026-06-28 at 10 06 41" src="https://github.com/user-attachments/assets/26a79bc9-f7c6-4700-8b39-0505af75180c" /> **After** — fields, constants, control-flow keywords and records get semantic colors: <img width="934" height="1032" alt="Screenshot 2026-06-28 at 09 58 15" src="https://github.com/user-attachments/assets/def44b05-85fa-4b39-9ea7-74ceb667bf6d" /> </details> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ries#60015) In C# files nothing gets semantic highlighting — class fields and other identifiers are colored by tree-sitter only, so a private field looks the same as a plain local variable. The reason: Roslyn (the C# language server) doesn't declare `semanticTokensProvider` statically in its `initialize` response. It registers it **dynamically** (`client/registerCapability`), and only when the client advertises `textDocument.semanticTokens.dynamicRegistration = true`. Zed advertised `false` and didn't handle such a registration, so Roslyn never offered semantic tokens at all. (rust-analyzer/gopls are unaffected — they declare the capability statically.) This is the first of two PRs. This one makes Roslyn actually **send** semantic tokens. The companion PR (zed-industries#60027) maps Roslyn's C#-specific token types to theme styles — without it the tokens arrive but most are dropped, since their types aren't in Zed's default rules. ## Solution - Advertise `textDocument.semanticTokens.dynamicRegistration = true`. - Handle the `textDocument/semanticTokens` registration and unregistration so the capability is stored, following the existing arms for `documentLink`, diagnostics, etc. ## Testing - Added a test that dynamically registers and unregisters `textDocument/semanticTokens` and checks the stored capability appears and is cleared. - Verified manually against Roslyn on a C# project: Zed now sends `textDocument/semanticTokens/full` and gets tokens back; before this change there was no semantic-token traffic at all. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Support dynamic registration of the `textDocument/semanticTokens` capability. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In C# files nothing gets semantic highlighting — class fields and other identifiers are colored by tree-sitter only, so a private field looks the same as a plain local variable.
The reason: Roslyn (the C# language server) doesn't declare
semanticTokensProviderstatically in itsinitializeresponse. It registers it dynamically (client/registerCapability), and only when the client advertisestextDocument.semanticTokens.dynamicRegistration = true. Zed advertisedfalseand didn't handle such a registration, so Roslyn never offered semantic tokens at all. (rust-analyzer/gopls are unaffected — they declare the capability statically.)This is the first of two PRs. This one makes Roslyn actually send semantic tokens. The companion PR (#60027) maps Roslyn's C#-specific token types to theme styles — without it the tokens arrive but most are dropped, since their types aren't in Zed's default rules.
Solution
textDocument.semanticTokens.dynamicRegistration = true.textDocument/semanticTokensregistration and unregistration so the capability is stored, following the existing arms fordocumentLink, diagnostics, etc.Testing
textDocument/semanticTokensand checks the stored capability appears and is cleared.textDocument/semanticTokens/fulland gets tokens back; before this change there was no semantic-token traffic at all.Self-Review Checklist:
Release Notes:
textDocument/semanticTokenscapability.