Language-server-protocol: Support semantic highlighting

[hackwaly]

Like WebStorm and VS does: eg. Symbol is a type or parameter or namespace or unresolved ...

Textmate based grammars are hard to do this. Since we did support Diagnostics, why not support semantic highlighting?

[dbaeumer]

Fully agree and we already discussed it since we would like to have it in VS Code as well.

[daviwil]

I'd love to have this for the PowerShell extension also. We provide the ability to create "dynamic keywords" for the purpose of writing domain-specific languages. Semantic highlighting would allow us to colorize those keywords in VS Code even though they aren't part of the PowerShell language spec.

/cc @BrucePay

[smarr]

I was looking for that as well.

What's exactly the meaning of the current 'highlighting' for read/write/text (see document highlights)?

I implemented that part but didn't see any for of visual feedback in the editor.

[cdietrich]

DocumentHighlight is for "mark occurrences"

[smarr]

@cdietrich sorry for hijacking the topic, but this is very unclear to me. How is document highlights used to realize a 'mark occurrences" feature? How does the read/write/text distinction fit in, and why does it only expect a single result in return? For mark occurrences, I'd expect to be able to return a collection, no?

Would be great if that could be clarified also in protocol.md.

[cdietrich]

@smarr yes you are right. this makes no sense. i asume the return type should be an array.
if i have a look at vscode i can find

export interface DocumentHighlightProvider {
        provideDocumentHighlights(model: editor.IReadOnlyModel, position: Position, token: CancellationToken): DocumentHighlight[] | Thenable<DocumentHighlight[]>;
    }

thus looks like a bug in the protocol

[smarr]

@cdietrich thanks, I'll open a separate issue.

[svenefftinge]

Please find a proposal in PR #124

[jpike88]

I’m guessing this is dead... anyone got any updates?

[svenefftinge]

I am working on a new proposal for semantic coloring.

[rcjsuen]

There is a proposal in the microsoft/vscode-languageserver-node repository now that I suggest others take a look at when they get a chance.

@dbaeumer Suggest to close #513 in favour of this or the other way around.

[axelson]

VSCode 1.43 now supports semantic highlighting via the language server for TypeScript: https://code.visualstudio.com/updates/v1_43#_typescript-semantic-highlighting

I think it's through proposed updates for the language server protocol 3.16?

[kaby76]

Can someone provide a link to the proposed 3.16 LSP spec, which includes "semantic highlighting"? I cannot find it in the usual location. I've read the wiki page on Semantic Highlighting Overview for VSCode, and while I am pleased with the "General Tokens Provider API" because it fits nicely into the current VSIDE (aka VS2019) ITagger<> and IClassificationFormatMap framework, I am also concerned over the Token Classification portion. I remind folks that symbols in programming languages like Antlr, Bison, and other parser generator languages, do not fit into these categories at all. Also, it is worth noting that grammars can contain target-language code blocks, so it is not a homogenous language used. I am hoping that the provider just return an integer of an equivalence class that the client then maps into the client UI format via a user specified mapping. The client does not need to know the meaning of the equivalence class of the symbol, i.e., whether it is "variable", "type", "enum", "non terminal" or "terminal". However, the user must understand what it is. For Antlr LSP, I already provide a judicious mapping of the equivalence class of the symbol into a SymbolKind--an integer--for Document Symbols, disregarding the meaning of the SymbolKind (e.g., "type"), and use that for CFG/static semantics colorization via a custom message. (Note, in VSIDE, SymbolKind in the LSP client that MS implemented also takes on additional services, like "go to def" and "find all refs", so it was done with care, experimentally, so things like comments do not get populated in drop-down boxes.) In VSIDE, scrolling only requests tagging for a small range of text that is going to be displayed. It is quite fast and works well. I never implemented a TextMate color tagger because Antlr syntax cannot be described using regular expressions.

[rcjsuen]

Can someone provide a link to the proposed 3.16 LSP spec, which includes "semantic highlighting"?

I don't think it's been written up yet. There is this TypeScript file from the microsoft/vscode-languageserver-node repository that you can do some reading up on.

[ell1e]

Interesting, isn't it in practical use already? Would be useful to allow other editor/IDEs and LSPs to catch up if that is the case, preferably without everyone just guessing / making up the specs based on the code by themselves.

[Razzeee]

It's in use by some extensions, but it's still in proposal as far as I know. The lsp implementation will even needs to get some updates if you can trust https://github.com/microsoft/vscode/issues/95168
So it's not done it seems.

[dbaeumer]

That is correct. The API is not final yet hence it is still in proposed state (and will be there until we ship 3.16)

[etc0de]

Ok. But forgive me if this is a stupid question, but wouldn't it be better to get some early feedback on how it could look like before investing all the effort into a complete implementation in VS Code? It sounds like now would be a good time to collect some feedback while the implementation isn't fully set in stone yet

[rcjsuen]

@etc0de Some of us have implemented it and there was a bit of back and forth on the VS Code API (https://github.com/microsoft/vscode/issues/86415) which does influence the LSP API. I didn’t have much issues implementing this feature but Dockerfiles aren’t exactly the most complicated language semantics-wise. :)

Of course, I do agree in that it would be great if there were more implementations so there can be more feedback about the feature.

[etc0de]

Well the thing is, all you linked is implementation code but I don't even know how the actual protocol looks like like right now. I just found this in one of your links, but this is about vs code settings: https://github.com/microsoft/vscode/wiki/Semantic-Highlighting-Overview

Hasn't anybody sat down and done a quick writing down of the actual JSON things sent back and forth?

I feel like this is what feedback would be useful on right now, because I'm not talking end user feedback (this is the LSP repo after all, not the VS code repo) but other language server authors' feedback whether the protocol even works for them and their implementations. This seems to be unclear right now, most of the links floating around here look like they're already for end users to test or complete plugins for use as-is. I don't care about the VS code API, I want to know if the actual underlying protocol is reasonable from the language server side.

It just feels like VS Code is already locking in their implementation when not many have really seen the underlying protocol yet, that seems risky to me.

Also, as a side note, has anybody considered deprecating documenthighlight and renaming it to expressionhighlight, occurrencehighlight, or something different in a newer protocol draft? The "document" part really makes it sound like it semantically highlights the entire document, so I feel like the naming here is somewhat unfortunate. Edit: I guess it's named like that for consistency with DocumentSymbol etc. Then what about DocumentOccurrenceHighlight or something, or is that too long? Oh well, maybe renaming it isn't ideal... it was just an idea

[rcjsuen]

Well the thing is, all you linked is implementation code but I don't even know how the actual protocol looks like like right now.

@etc0de The TypeScript file I linked to is mostly interfaces (and not necessarily implementation code) but it is certainly fair to say that it is not exactly very reader-friendly.

Hasn't anybody sat down and done a quick writing down of the actual JSON things sent back and forth?

In terms of the payload of the integer array, that is explained in the API documentation for the provideDocumentSemanticTokens function in VS Code which was linked to from the aforementioned TypeScript file. But as per the above, I agree that it is certainly not super obvious and it would be nice if it was presented in a more readable format.

I feel like this is what feedback would be useful on right now, because I'm not talking end user feedback (this is the LSP repo after all, not the VS code repo) but other language server authors' feedback whether the protocol even works for them and their implementations. This seems to be unclear right now, most of the links floating around here look like they're already for end users to test or complete plugins for use as-is. I don't care about the VS code API, I want to know if the actual underlying protocol is reasonable from the language server side.

Then I suggest you take a look at the comments from the last few months in https://github.com/microsoft/vscode/issues/86415. Comments like https://github.com/microsoft/vscode/issues/86415#issuecomment-573934661, https://github.com/microsoft/vscode/issues/86415#issuecomment-587184889, and https://github.com/microsoft/vscode/issues/86415#issuecomment-596143316 are all questions posed by from the perspective of a language server author. Although the questions were posed in the VS Code repository, because the proposed LSP API of semantic highlighting practically maps 1-to-1 to the VS Code API the questions are still of relevance.

Do I kind fo wish those questions were linked to from or posted in this issue instead? Sure.

It just feels like VS Code is already locking in their implementation when not many have really seen the underlying protocol yet, that seems risky to me.

I can see that.

Also, as a side note, has anybody considered deprecating documenthighlight and renaming it to expressionhighlight, occurrencehighlight, or something different in a newer protocol draft? The "document" part really makes it sound like it semantically highlights the entire document, so I feel like the naming here is somewhat unfortunate. Edit: I guess it's named like that for consistency with DocumentSymbol etc. Then what about DocumentOccurrenceHighlight or something, or is that too long? Oh well, maybe renaming it isn't ideal... it was just an idea

This is probably the first I've heard of such a request. I have never thought of it that way but from how you are describing it I can see how it can be confusing now that I look at things that way.

[etc0de]

The TypeScript file I linked to is mostly interfaces (and not necessarily implementation code)

I figured, but I honestly can't tell to what JSON they map. What does a SemanticTokensBuilder with a push do? What is a provideDocumentSemanticTokensEdits? entry in JSON? Is that a type, a name of some complex object member, a boolean, ...? It is honestly quite unreadable if you're not familiar with typescript.

IMHO it's a bad precedent to just assume anyone using this standard is familiar with typescript, I think it's also bad to drop an implementation as a spec draft. But maybe that's just me :woman_shrugging: I wish I could give you feedback, I'm certainly interested, but in the current format I'm afraid that is hard to do

Then I suggest you take a look at the comments from the last few months

The discussion is interesting, but it doesn't really solve that the "draft" itself in it's current form isn't really in a universally readable form. It's basically a closed off playground for typescript devs only, apparently...? I don't know, I do kind of find this approach bewildering. I really would suggest this approach is changed.

Edit: to be fair, the protocol.semanticTokens.proposed.ts is a bit more readable now that I looked over it. It was mostly the semantic-tokens-sample/vscode.proposed.d.t that threw me off. Still, I think using typescript code for a spec draft is far from ideal.

[rcjsuen]

The discussion is interesting, but it doesn't really solve that the "draft" itself in it's current form isn't really in a universally readable form. It's basically a closed off playground for typescript devs only, apparently...? I don't know, I do kind of find this approach bewildering. I really would suggest this approach is changed.

The other proposed API at the moment, the textDocument/callHierarchy request does at least have a Markdown file written up about it and I can recall other instances in the past where a similar Markdown file was written. I am not sure why one wasn't written this time.

[Razzeee]

For the sake of completeness, the F# plugin Ionide also already implements it and also shipped it. @Krzysztof-Cieslak

[Krzysztof-Cieslak]

We created custom LSP endpoint and wrapped stuff on VSCode side, it’s not really using proposed API in LSP spec

[axelson]

I have a question that I haven't seen an answer to. Can the semantic highlighting be additive with the existing textmate grammars? Or is it completely one or the other?

[rcjsuen]

Can the semantic highlighting be additive with the existing textmate grammars? Or is it completely one or the other?

I don’t believe this has been spec’d out. I encountered this in VS Code (https://github.com/microsoft/vscode-languageserver-node/issues/570 ) and they do a merge of sorts there.

[Krzysztof-Cieslak]

Semantic highlighting in VSCode is additive. VSCode first runs normal text mate highlighting and then improves it when semantic results are available.

But I’d imagine it may differ from client to client, it seems like a client implementation detail.

[kjeremy]

I implemented the LSP portion of semantic highlighting for the rust-analyzer server and it was pretty straightforward.

The other proposed API at the moment, the textDocument/callHierarchy request does at least have a Markdown file written up about it and I can recall other instances in the past where a similar Markdown file was written. I am not sure why one wasn't written this time.

FYI that file does not match the protocol. I was hoping to update it if https://github.com/microsoft/vscode-languageserver-node/pull/614 looks right.

[paulyoung]

I think @etc0de has brought up some very good points and I too would like to see a draft of the spec in a similar form to what would eventually appear on the website.

[paulyoung]

I think @etc0de has brought up some very good points and I too would like to see a draft of the spec in a similar form to what would eventually appear on the website.

[rcjsuen]

The other proposed API at the moment, the textDocument/callHierarchy request does at least have a Markdown file written up about it and I can recall other instances in the past where a similar Markdown file was written. I am not sure why one wasn't written this time.

FYI that file does not match the protocol. I was hoping to update it if microsoft/vscode-languageserver-node#614 looks right.

:( Well, that's unfortunate. Thanks for pointing this out, @kjeremy!

[axelson]

@rcjsuen @Krzysztof-Cieslak thanks for the info and link. It would be great to see the additive (or not) behavior formally spec'd out (perhaps in a new .md that has been proposed). As a Language Server author I like the idea of semantic highlighting being additive since it will make it easier to adopt since initially I could just add support for the few tokens that have recursive definitions (or are otherwise hard to define without using negative lookbehinds or other complex tools).

[HighCommander4]

FYI that file does not match the protocol. I was hoping to update it if microsoft/vscode-languageserver-node#614 looks right.

:( Well, that's unfortunate. Thanks for pointing this out, @kjeremy!

I have pointed this out a while ago :)

[rcjsuen]

@rcjsuen @Krzysztof-Cieslak thanks for the info and link. It would be great to see the additive (or not) behavior formally spec'd out (perhaps in a new .md that has been proposed).

I'm all in favour of more things being explicitly stated. 👍

[etc0de]

For what it's worth, I left minor other points here: https://github.com/microsoft/vscode/issues/86415#issuecomment-619350479

It would be great to see the additive (or not) behavior formally spec'd out

For that I would like to request that textmate supports isn't a requirement in a client: my suggestion would be to make the spec say "if the client supports other sources of highlight formatting options like textmate, it should add them in additively unless the LSP server specifies option XYZ to indicate an intended exclusive handling of semantic highlighting" or something like that.

The reason for textmate being optional: I pondered writing a small mini-IDE for my language, and if I do I plan to just support LSP-backed highlighting only because that is all I personally need but I'd want to at least possibly allow other LSPs to be plugged in too. It'd be nice if I then wouldn't necessarily be violating the specs just because of a more spartanic not-textmate-supporting editor implementation.

The reason for LSP server being able to override textmate with a protocol option: I am all for editors that have "universal" LSP plugins where you don't need to provide a client-side plugin to support a language. However, these editors might also have fallback support for textmate highlighting that is then possibly going to be outdated, and without a per-client plugin it might be difficult to tell the editor to not use that at all unless it's in the protocol. So if I design my LSP server-side to highlight everything exhaustively, it'd be nice to be able to explicitly state "please don't use textmate additively unless the end user overrides this, the result will likely be less correct anyway".

[etc0de]

Another thing, does anyone know if this viewport-style local querying as suggested here is possible as a client? https://github.com/microsoft/vscode/issues/86415#issuecomment-573934661 It looks like it is with the ranges, but I just wanted to ask again since this looks really important to get ok performance in large files. Sorry if this was already answered, there were a lot of comments on this;

(Less important: I wonder, has anyone tested the performance of not using semantic token edits and just requerying the viewport on typing? I find the suggestion of not adding in the edits to simplify the protocol at least possibly worth exploring, even though I assume in the end one probably wants the edits for minimal typing lag)

[rcjsuen]

I have created a proposed.semanticTokens.md gist to hopefully make the reading of the TypeScript definition file a little bit easier.

I hope this will be of use to the wider community though I do kind of worry about possibly causing confusion due to it not being official. Hopefully the disclaimer at the top is good but I am happy to delete the Gist if people think it will do more harm than good (by possibly hurting the understanding of the semantic tokens API requests because it is not a 1-to-1 mapping of the TypeScript file).

[rcjsuen]

Another thing, does anyone know if this viewport-style local querying as suggested here is possible as a client? microsoft/vscode#86415 (comment) It looks like it is with the ranges, but I just wanted to ask again since this looks really important to get ok performance in large files. Sorry if this was already answered, there were a lot of comments on this;

@etc0de The two comments here (https://github.com/microsoft/vscode/issues/92789#issuecomment-608145650 and https://github.com/microsoft/vscode/issues/92789#issuecomment-608146554) seem to suggest it is working in terms of the VS Code API but I'm not sure if anyone's implemented textDocument/semanticTokens/range from the LSP end yet.

[puremourning]

Thanks for dong this @rcjsuen, it's much appreciated.

I would suggest that the overwhelmingly most important section is the referenced code comments: https://github.com/microsoft/vscode-extension-samples/blob/5ae1f7787122812dcc84e37427ca90af5ee09f14/semantic-tokens-sample/vscode.proposed.d.ts#L71 and https://github.com/microsoft/vscode-extension-samples/blob/5ae1f7787122812dcc84e37427ca90af5ee09f14/semantic-tokens-sample/vscode.proposed.d.ts#L131

This is the API that server and client authors need to understand, review and accept/debate. It's complex and arguably non-obvious and making it work with all the various clients is the goal (assuming of course the LSP stated goal of solving the matrix problem is still a goal).

For example, it wasn't obvious to me why delta encoding is used for the positions? This seems to introduce a (unnecessary?) complexity to the protocol which favours one particular implementation of client highlighting. Presumably its purpose is to minimise the amount of data transferred on the "wire" doing edits? If this leads to complex encoding/decoding, is it really the right trade-off ? I mean, are there numbers behind it?

[axelson]

For that I would like to request that textmate supports isn't a requirement in a client: my suggestion would be to make the spec say "if the client supports other sources of highlight formatting options like textmate, it should add them in additively unless the LSP server specifies option XYZ to indicate an intended exclusive handling of semantic highlighting" or something like that.

The reason for textmate being optional: I pondered writing a small mini-IDE for my language, and if I do I plan to just support LSP-backed highlighting only because that is all I personally need but I'd want to at least possibly allow other LSPs to be plugged in too. It'd be nice if I then wouldn't necessarily be violating the specs just because of a more spartanic not-textmate-supporting editor implementation.

That's a great point! I'm agreed on both points (not being textmate specific) as well as a method to indicate that the server supports fully tokenizing the document. Although in the second case, I think the client should be free to provide their own overrides of the server's tokens, a primary reason for this would be if an end-user wants to customize the tokenization in some way.

[puremourning]

Forgive my ignorance, but where does TextMate appear in the spec ? I realise that the spec uses TextMate snippet syntax (sort of) but reading @rcjsuen markdown, I don't see any TextMate specifics in there. Perhaps I'm missing something obvious.

[axelson]

I don't think TextMate appears in the spec, I was the only one that brought it up.

[puremourning]

Oh does VSCode use TextMate internally then? Is that why it came up ?

[rcjsuen]

Oh does VSCode use TextMate internally then? Is that why it came up ?

Correct, VS Code will render with TextMate first and then apply semantic highlighting (additively) on top after it gets the response back from the language server.

TextMate is not mentioned in the VS Code API or the LSP API as far as I know.

[rcjsuen]

I've tried to summarize the above points regarding what the client and server should support and declare.

1. The specification should not make any claims about the client needing to support TextMate, Tree-sitter, or any other grammar. LSP clients should be free to rely completely on the language server for its syntax highlighting needs without also needing to support syntax highlighting via a grammar.

2. Clients should declare a) if they support additive/merging support of semantic tokens on top of whatever internal grammar it has already and b) if they support discarding the grammar from A and replacing it completely with the semantic tokens information from the server.

   • A implies that it is capable of taking the results from its internal grammar (if it exists) and merging them with the tokens from the language server. (Note: VS Code does this automatically right now with no opt-in/opt-out option.)
   • B implies that it is capable of ignoring the grammar and simply applying everything from the language server instead. This would address https://github.com/microsoft/vscode-languageserver-node/issues/570. (Note: As indicated by the bug, VS Code does not support this use case right now.)

3. Servers should declare a) whether they support full document calculations and/or b) partial document calculations.

   • A implies that the server knows everything and will return everything if asked. Anything it does not return is intentional (and likely done to fix grammar limitations which may make an additive end result less correct).
   • B implies the server is capable of not returning everything if asked and thus the result it returns to the client should (probably?) be merged with the grammar on the client, assuming one has been defined and set there.

It's a little verbose but I think it's better to be more explicit about everything. What does everyone else think?

[etc0de]

Clients should declare a) if they support additive/merging support of semantic tokens on top of whatever internal grammar it has already and b) if they support discarding the grammar from A and replacing it completely with the semantic tokens information from the server.

IMHO 2B should just be mandatory for clients that support the new semantic highlighting protocol. (2A can be optional, of course.) I don't see why it would be difficult for anyone to implement, and not having it can obviously mess up the result as seen in https://github.com/microsoft/vscode-languageserver-node/issues/570 .

Servers should declare a) whether they support full document calculations and/or b) partial document calculations.

Why not just replace 3. with a server->client command to just not do any additive grammars on top unless the user explicitly overrode it? I can't think of a scenario where actually knowing why is particularly relevant, especially since I can't really see why it'd be done outside of the expectation additive grammars worsen the result anyway, in which case it makes no sense to default to additively apply them for whatever reason.

Edit: basically the protocol of what is actually sent could be, client: "(if client supports & does this) btw I will use an additive grammars on top of LSP semantic highlighting, protest if that is bad", server: "(if additive is bad) please don't do that additive thing unless the user really made you do it, thanks". (Whether clients actually allow setting such an override I find personally unimportant. I don't think it is likely to be particularly needed, but I don't want to take away any IDE's choice to offer it to the user)

[etc0de]

Sorry for the comment spam, but this remark bubbled up in my head again:

For example, it wasn't obvious to me why delta encoding is used for the positions? This seems to introduce a (unnecessary?) complexity to the protocol which favours one particular implementation of client highlighting

I'm probably missing something, but I agree and I had this other idea:

If these edits are to move things around fast after insert, can't there be a simple token-insert-at operation (and token-delete-at) for the LSP server which will shift all the follow-up tokens in index respectively in the client's token stream memory? For larger changes resulting (e.g. quotation typed with multiline string tokens) things would need to be fully retransmitted anyway, and the file position of the tokens could just be adjusted by the editor/client locally. (After all, it's obvious inserting a space will move every token after it on that line in terms of column position by +1, right?)

Or am I missing something? I'm really sorry if that proposal was already discussed. I read the typescript wrong, obviously it is already an insert-at and/or delete-at. But why the complicated decoding then?

[rcjsuen]

For example, it wasn't obvious to me why delta encoding is used for the positions? This seems to introduce a (unnecessary?) complexity to the protocol which favours one particular implementation of client highlighting. Presumably its purpose is to minimise the amount of data transferred on the "wire" doing edits? If this leads to complex encoding/decoding, is it really the right trade-off ? I mean, are there numbers behind it?

@alexdima @dbaeumer Can one of you help weigh in here?

Servers should declare a) whether they support full document calculations and/or b) partial document calculations.

Why not just replace 3. with a server->client command to just not do any additive grammars on top unless the user explicitly overrode it?

I don't think a command makes sense as it feels odd to me for a server to send an explicit command to the client solely for the purpose of toggling something on and off. I am not sure how likely it is for a server to have confidence it is "perfect" for some X% of the time but then to also occasionally need to send a request/notification over to the client to toggle itself because now it is the Y% of the time when it's not "perfect".

How about enhancing the SemanticTokens interface so that it has a boolean field to instruct the client whether an additive merge should be applied or not? The exact naming of this field is of course up to debate.

export interface SemanticTokens {
    /* Copy/pasted from the original interface... */
    resultId?: String;
    /* Copy/pasted from the original interface... */
    data: number[];
    /*
     * The semantic tokens data should be applied on top of the
     * syntax highlighting that the client already has.
     */
    mergeRequired: boolean;
}

[etc0de]

A field seems fine, sure. mergeRequired however sounds like a client needs to support merging which I think so far everyone agreed should be optional. What about mergeDisabled, or isExhaustive (= true means must not merge), or unmergeable, or something like that?

[kaby76]

I have created a proposed.semanticTokens.md gist ..

This looks good from my perspective (VS2019/Antlr). It's similar to what I've implemented via custom messaging and ITaggers's in VS2019. It'll be an easy change for me to switch the code over to the proposed semantic highlighting API if and when the LSP APIs are updated (with emphasis on the if).

This may be a stupid request, but I would like to see the purpose of LSP semantic highlighting elucidated to the point where I, a moron, can understand. When I first implemented my LSP server, I was a little surprised that the text in the VS2019 textview was completely monochrome. After all, a document symbols request assigns a type ("variable", "class", "method", etc) to a range of characters throughout the document. It could have been used for colorization of the text where the client defines apriori the display style for the type. Since it's clear the information in the semantic highlighting call is a superset of the information in a document symbols call, one could ask why are they are separate calls. What I'm looking for is a short description of what the client is going to use it for, something like this:

_LSP semantic highlighting is for noting to the client a list of type and range of characters in the text a static display style in a text view._

_LSP ‘textDocument/documentSymbol’ is for noting to the client a list of type and range of characters in the text whether the client can allow “go to def”, “find all refs”, … (???) for that text in a text view. (Note that there are separate API calls "go to def", "find all refs" to provide further information.)_

_LSP ‘textDocument/documentHighlight’ is for noting to the client a list of type and range of characters in the text so that the client can apply a dynamic highlighting style to the text, i.e., when you click on a symbol in the text._

I have still no idea what LSP ‘textDocument/documentColor’ is for, but now obviously not for “semantic highlighting”.

You can wordsmith this to whatever you feel, but my point is that it's absent from the LSP spec at each call. I imagine the response will be "Oh, we can't do that because it boxes in the versatility of LSP.' But, the fact that someone would ask "deprecating documenthighlight and renaming it to expressionhighlight, occurrencehighlight, or something different" is an indication of something wrong.

Respectfully, Ken Domino

[etc0de]

If I may give a concrete idea for SemanticTokensEdit:

Why not something like edit: {rangeStart: 0, rangeLen: 3, property: "line", valueShift: 1} to move tokens 0,1,2 down by one line, instead of: edit: { start: 0, deleteCount: 1, data: [3] } as given in the spec. valueShift would be relative(!) value applied to the given "line" property. There could be an alternative value field applying an absolute value that also allows strings to e.g. set the "tokenType" in bulk.

Wouldn't this cover most practically relevant edits in a really short JSON string still?

we have decided to avoid allocating an object for each token and we represent tokens from a file as an array of integers

As a note to this, while for storage of the file as a whole this format might make sense that doesn't necessarily make it a good match for edit transfers.

Edit: I made a separate ticket for this now: https://github.com/microsoft/language-server-protocol/issues/979

[HighCommander4]

_LSP ‘textDocument/documentSymbol’ is for noting to the client a list of type and range of characters in the text whether the client can allow “go to def”, “find all refs”, … (???) for that text in a text view. (Note that there are separate API calls "go to def", "find all refs" to provide further information.)_

[...]

I have still no idea what LSP ‘textDocument/documentColor’ is for, but now obviously not for “semantic highlighting”.

My understanding is:

• textDocument/documentColor is for cases where colors are part of the language's semantics itself (e.g. in CSS, colors can be the values of properties), and allows the client to show a color indicator (e.g. a little square filled with the color in question) next to places in the language syntax where a reference to a color appears.

  • textDocument/documentSymbol is mainly for being able to show an "outline" of the document listing the symbols defined in the document, including any hierarchical relationship between them (e.g. a method is inside a class), and navigate to the element in the source from the outline. The information sent by the server for each symbol includes a range which is entire range of the symbol's definition (e.g. for a class, the entire class definition), and a selectionRange which is just the name of the symbol. While the selectionRange could technically be used to apply semantic highlighting, that would only cover the definitions of symbols, and not references to them, so it wouldn't be very useful.

I do agree that the purpose of requests isn't always clear from the spec, and I've sometimes had to look at example usages in clients or servers to understand the intent. Perhaps illustrative examples of intended editor features could be provided in the spec, without constraining what the requests can be used for (as they are just examples).

[kjeremy]

I do agree that the purpose of requests isn't always clear from the spec, and I've sometimes had to look at example usages in clients or servers to understand the intent. Perhaps illustrative examples of intended editor features could be provided in the spec, without constraining what the requests can be used for (as they are just examples).

This is as close as I've found for something like that:
https://code.visualstudio.com/api/language-extensions/programmatic-language-features

[kaby76]

I do agree that the purpose of requests isn't always clear from the spec, and I've sometimes had to look at example usages in clients or servers to understand the intent. Perhaps illustrative examples of intended editor features could be provided in the spec, without constraining what the requests can be used for (as they are just examples).

This is as close as I've found for something like that:
https://code.visualstudio.com/api/language-extensions/programmatic-language-features

Folks, maybe the spec should have definitions of the UI terms along with the purpose of the API method, starting with "semantic highlighting"? I never understood what this "hierarchy" was all about in LSP for document symbols, so I just return a SymbolInformation[]. But I now read that it's for "outlining". Is that as in the "outlining" controls that somehow _magically_ appeared in the left margin of a textview, e.g., the squares with + or -, with a vertical line that extends down, that show and hide code when toggled _even though the spec says I specified a FLAT hierarchy using SymbolInformation[]_? Or, maybe it's "outline" as in a tree displayed in the Solution Explorer? I'm taking this on blind faith that "semantic highlighting" is going to colorize my text. I hope you see my point.

[dbaeumer]

Wow, lots of comments :-).

@rcjsuen THANKS a lot for writing up the markdown. I will have a look beginning of next week. We are in endgame right now.

[rcjsuen]

Clients should declare a) if they support additive/merging support of semantic tokens on top of whatever internal grammar it has already and b) if they support discarding the grammar from A and replacing it completely with the semantic tokens information from the server.

IMHO 2B should just be mandatory for clients that support the new semantic highlighting protocol. (2A can be optional, of course.) I don't see why it would be difficult for anyone to implement, and not having it can obviously mess up the result as seen in microsoft/vscode-languageserver-node#570 .

On the topic of grammars vs semantic highlighting, I've encountered another issue in VS Code that's somewhat related to this topic due to how it chooses to trigger code completion (https://github.com/microsoft/vscode/issues/95924#issuecomment-620181834).

[dbaeumer]

I commented in #979 about the format and why we choose it. And there was a lot of measurements going on to end up with that format.

Here a short recap of why we ended up there:

1. encoding the delta should be easy (no knowledge of what is delta encoded). In the current implementation the delta is simply the delta of two number arrays. In a previous implementation we ask to create a real semantic delta for tokens but the first tester ask to implement it (TypeScript) basically failed since it got to complex.
2. delta encoding is used to save space on the wire and to reduce object allocation on the client.

3. simple edits (new characters on the line, new lines) should result in a small delta.

4. & 3. resulted in encoding a line relative to the previous line.

Additional thinks I read from the comments we should add:

• client capability to signal if the client already does some sort of tokenization and whether it merges server tokens with client tokens. I would do something like:

        /**
         * Capabilities specific to the `textDocument/semanticTokens`
         *
         * @since 3.16.0 - Proposed state
         */
        semanticTokens?: {

                        // .....

            /**
             * Whether the client has its own tokenizer (e.g. TextMate)
             */
            tokenizer?: {
                /**
                 * Which strategy the client uses if both client and server tokenize.
                 */
                strategy: 'merge' | 'replace';

                /**
                 * If strategy is `merge` which token wins if both client and server
                 * provide a token for a range.
                 */
                conflict?: 'clientWins' | 'serverWins';
            }
                }

I am not sure if we would still need mergeRequired. What is the use case of letting the server decide this on every response.

[etc0de]

@dbaeumer but wouldn't that complexity only exist when needing arbitrary document deltas? However, doesn't all of that go away when only looking at what the user can reasonably change? (insert of X tokens at a single point, deletion of X tokens at a single point) To me it seems like sending the changes resulting from that is actually fairly easy, unless I'm missing something. What's the scenario where this is so complex?

Sorry if I'm dragging this out unnecessarily, but I would really be interested in understanding what exactly you tested and why, if my questions aren't too much trouble. I understand arbitrary diff'ing is hard, but I just don't get why this would ever be needed here

Edit: I'm also just noticing, if the encoding helps people write a generic differ why not suggest it as internal conversion for those who need help with this? That still seems a bit odd why it'd need to be in the protocol just for that reason

[dbaeumer]

Regarding

I'm also just noticing, if the encoding helps people write a generic differ why not suggest it as internal conversion for those who need help with this?

We discussed this but decided against it. We think that it is easier to understand if the delta encoding happens on the request level. It gives more flexibility. What we do look in parallel is to support a different content encoding on the wire (message pack versus JSON).

Changing code at one place can change semantic tokens at many place in the same file even in other files. So for implementors (from the feedback) it seemed the easiest to always travers the full file (or a predefined given range) and create the semantic tokens for it. We think the easiest diffing is then happening afterwards using the old and new result, which BTW is optional in the protocol. If memory is an issue a builder can be used to compute the diff on the fly.

I also want to point out that we worked on this for months now being on our iteration plan since November 2019 (https://github.com/microsoft/vscode/issues/83930) with quite some feedback and discussions form LSP and VS Code implementers.

[puremourning]

I also want to point out that we worked on this for months now being on our iteration plan since November 2019 (microsoft/vscode#83930) with quite some feedback and discussions form LSP and VS Code implementers.

Where was this discussion held ? I would have thought this forum (the LSP repo) PRs/Issues to be the obvious place to have the discussion.... which i guess is what we're doing. I'd be surprised if anyone participating in this discussion here is _not_ a LS or LSP client implementor.

We need to remember that there is more than 1 client of LSP, VSCode is not the only one that people care about.

[etc0de]

and discussions form LSP and VS Code implementers.

Were all of them using JavaScript or TypeScript? I can understand it is easier to encode it like this for these languages where JSON objects are kind of "native", but I think all others this is likely a nuisance rather than a help, especially in languages like C++/Rust/Go/... with fast native types that work differently.

I also think the protocol shouldn't be trying to somehow teach me an internal format, it sounds like all of this should go into the appendix for those people who need help with implementing this efficiently. I think the protocol should focus on clarity on what happens with each protocol event (which I think the encoding right now works highly against) as long as this doesn't impact transfer speed too much (which from the sort of edits/shifts that will happen with real world typing input I don't think will be a problem, but that is why I asked for numbers and what exact scenarios you tested!). Beyond that, the protocol should IMHO leave the implementation details to the authors.

Summed up, to me it just feels a mix of purposes for me, things that don't belong together, maybe even a hack to make it easier for one group of people (JS/TS users) while sacrificing universality and ease of understanding of others. I don't think that is the right way to go, personally. But I'll happily bow if the majority disagrees with me, I just wanted to put my voice out there.

(FWIW I implemented an LSP server in the past and I plan to implement one again soon. So I also count myself as an implementer)

Edit: just as an additional note:

We think the easiest diffing is then happening afterwards using the old and new result, which BTW is optional in the protocol.

My assumption is that supporting partial edits and according events is not optional in practice for large files if any acceptable performance is to be reached. I think most LSP server-side implementers want to support that use case. So I don't think arguing that people can just not implement this protocol part is useful here, personally.

[dbaeumer]

@puremourning sorry for that. Because of all the feedback we got in other issue I somehow assumed that this is know more broadly. Is there anything in the current proposal that makes it hard or impossible for you to implement that client side?

[puremourning]

@dbaeumer well, in the case of vim and/or neovim (i guess) the whole text-properties thing means that highlighting applied moves with the text and doesn't require being constantly updated. so a lot of this delta stuff might actually be worse for client performance (as it might be redundant).

But if i'm completely honest, it requires a lot more thinking than i've actually done on it. I would say that the protocol in its current form doesn't appear to lend itself to be easily implemented in any client for Vim (or vim-a-likes). But that's more of a gut feeling than a technical analysis.

My original comment was intended to understand the reasoning, which you've explained. I would tend to agree in a philosophical sense that designing a protocol around what's good for one particular client and it's particular quirks (such as garbage collection) is probably not the ideal situation, but i also completely understand why that would be, and can see that you've applied some practical engineering which makes sense.

I would be interested in hearing the thoughts of @chemzqm and @prabirshrestha (who i understand are the authors other prominent vim clients). In particular i understand that @prabirshrestha may have already implemented this proposal

[etc0de]

so a lot of this delta stuff might actually be worse for client performance (as it might be redundant).

That is a curious point, and I also wondered if it might be easiest to just send insert-token-at events - even my simplified proposal still implies manual explicit range shifts, albeit without the encoding. It's indeed a bit weird the protocol currently assumes token ids need to be shifted around in larger ranges in the first place. But it's hard to judge the actual perf details without trying it out (and sadly my code base isn't ready right now, that is quite unfortunate)

[kjeremy]

FWIW in rust-analyzer we do not support token edits, we just support full document and range and that works well enough.

[etc0de]

@kjeremy interesting, but wouldn't that force you to resend the entire token range after any insert in the middle of the document fully with all the token attributes etc? That sounds like it might develop into a problem when typing at the very top of a long file, unless the editor/LSP client is smart enough to discard these areas quietly and refetch them only for the viewport and then gradually in the background for the invisible further down areas. But if the assumption is LSP clients do that, why do the token edits/deltas even exist?

[dbaeumer]

@puremourning I am a little bit lost now since the proposal basically boils down to more or less the same as in the early PR you linked. At end end a server reports a token type (class function) and set of token modifiers (const, async, ...) for ranges in a document. Are you saying that this concept can't be applied to VIM?

[etc0de]

I don't want to dominate this discussion, and hoped there would be more input other than mine up to now (for any viepwoint really). But since nobody else jumped in, I've had time to reflect:

So it seems to me like right now, there are two ways to get good performance on a large file: 1. implement ranges server-side, and hope the LSP client will be smart enough to not instantly requery the entire file after an insert but rather in some gradual way (e.g. viewport-based + otherwise bit by bit refetching), 2. implement semantic edits server-side, and hope the LSP client will use it for really minimal deltas. I assume approach two gives even better performance, but not significantly so.

Therefore, this feels slightly duplicate to me. This is why I think it is problematic:

1. Somebody writing a client could not implement smart ranges handling (or not use ranges at all) and could expect token edits deltas to be supported server-side if any good performance is desired,

2. And somebody writing a server could skip token edits given they might not provide a huge boost in something like C++, where the integer encoding might not be a good fit to internals, and assume the client will be smart with ranges if needed.

Now both did "their part" to facilitate good performance, and it's still bad in the end result. That sounds undesirable to me, and it does also confuse me why both approaches are even needed for such a similar activity.

To solve this, I think one of the solutions should be marked as "mandatory when expecting reasonable performance with a common implementation," while the other marked as "icing on the cake" and maybe even moved out into some super-optional extension. I don't really care which one is preferred, but it might make sense to decide that first. Because IMHO the moved out one doesn't really need to be that understandable anyway, if people can just reasonably skip it.

I would however suggest that SemanticTokenEdits feels like a very specific approach that seems to only mainly benefit implementors that use the actual JSON objects for storage, so basically scripting languages that heavily use more generic key->value maps and generic list similar to JSON's object and array. That probably does not apply to the majority of implementors, so it does feel more alien to me than the ranges. But that is obviously just my personal opinion. I read things wrong here, after rereading it is obvious to me the "more alien" encoding is used both for ranges and token edits. So from that angle it doesn't really matter which method is the preferred one, but as noted below, not making deltas ever desync could be way more bug prone than simple ranges.

My apologies if I misunderstood how ranges and token edits work and I drew wrong conclusions, in that case please disregard my entire comment. (Will happily go back and edit all of this to strikethrough if that turns out to be the case)

[axelson]

I'm in agreement with @etc0de that with the current proposal could easily run into performance problems with certain client/server combos. I'm not sure that the textDocument/semanticTokens/edits request is worth the implementation effort, it seems very likely to not be fully adopted by all servers, and it seems potentially error-prone to generate and maintain (e.g. it could get out of sync easily).

Another issue that maybe I'm not understanding, but it seems like only giving theme creators a single set of tokens, with some modifiers applied might be limiting. I would expect that an approach with more nesting would be more useful because the theme could be used across more languages and more easily customized by end-user's. An example of the nesting that I'm talking about is: "string.quoted.double.interpolated.elixir" which lets the theme/user distinguish between strings with single quotes and those with double quotes.

Also is it possible to add module to the list of default token types? It would be helpful for languages that don't have classes.

[etc0de]

Also is it possible to add module to the list of default token types?

I think the idea is that the token types don't match the exact keyword name but the "spirit" of what the item is. E.g. class would then be used for any sort of multi-instantiable base type (like a prototype object in javascipt, or I assume a module in elixir if that behaves similarly?). Or if a module is just a grouping of functions under a name without an instantiated OOP/object metaphor, namespace would probably be a good match. Or is the module you think of not similar to any of these two?

Edit: also please note you can derive names. So you can send a module token type that is derived from (= colored like) a class type.

[axelson]

Yes, in Elixir a module is most similar to a class (a grouping of functions, but without any state). Are there any examples of how deriving token types works? I didn't see an example of that in the proposal.

[etc0de]

Documentation is still a bit poor at the moment, but fwiw here is the relevant note of the token types not being fixed but extensible e.g. via client capabilities: https://github.com/microsoft/vscode-languageserver-node/blob/e5d7ad881b1a51b486e3f0e4aa0fbc25dad2be58/protocol/src/protocol.semanticTokens.proposed.ts#L12 and I think the idea from a LSP server side is to specify new tokens in SemanticTokensLegend in the "tokenTypes" key offered by the server, although I'm not sure how exactly to specify what base token type each derives from.

@dbaeumer would you happen to have a small JSON example of how specifying a new server-specific "module" token type that maps to a "class" base type in the protocol would look like?

[etc0de]

Fwiw, I just reread the specs and realized the delta integer encoding is also used for ranges, or initially sent tokens in whatever case, not just the delta edits. I was mostly stumped because I didn't understand why bother for short deltas with this complicated encoding, but for longer initial data ranges I have to agree that the data savings might be worth it.

So with that, I am really only left wondering why there is both ranges and delta edits (since the latter seem barely worth as a separate almost-doing-the-same-job thing) and I'm not really opposed to the short integer encoding as a whole anymore, apart from the weird deltaLines. I can see why streaming larger ranges this might be worth it in integer encoding, as ugly as it looks like. Sorry that it took me so long to understand that part correctly.

I think deltaLines & the whole delta edit thing might be worth to just not have in there. I don't think having otherwise the short integer encoding but absolute line numbers would increase the initial send a lot, and it seems to me the delta edits are way more complicated than resending of plain ranges with possibly little gain.

[kjeremy]

@etc0de while this doesn't answer your question the encoding stuff clicked for me when I saw this implementation: https://github.com/microsoft/vscode-languageserver-node/blob/50a56c01328699a3357cfb3726e8e2a05f67f30d/server/src/semanticTokens.proposed.ts#L45-L149

The implementation we use (full document and range only) is here: https://github.com/rust-analyzer/rust-analyzer/blob/b65d6471ca809b567b6045f987ae1bdee2849423/crates/rust-analyzer/src/semantic_tokens.rs#L88-L128

[dbaeumer]

Sorry for the long silence but I was heads down with implementation stuff in LSP.

I think there is still confusion about the idea behind all of this. So I will try to summarize it:

• Semantic tokens are described in the following way: a range in a document, a token type and a set of token modifiers. Conceptually the ranges are absolute (although currently not encoded that way)
• the only request that a server needs to implement is to get semantic tokens for the full document.

Everything else are optimizations to either speed up semantic coloring in the UI or to minimize the payload on the wire.

• range based: this request is to speed up coloring if a user opens a large file. The server can quickly provide the colors for the visible range. However a client still needs to get the semantic tokens for the full file to avoid color flickering when scrolling and to be able to color the minimap.
• delta encoding: to minimize the payload on the wire.

We left the combination (range, delta encoded) out since a range based result is usually small, only requested once and therefore the saving is minimal.

To make things more clear I propose the following renames:

textdocument/semanticTokens/full: request to get all semantic tokens of a text document.
textdocument/semanticTokens/range: request to get all semantic tokens of a specific range.
textdocument/semanticTokens/full/delta: to get the delta for a full document.
textdocument/semanticTokens/range/delta: we could spec it but I guess given the above most servers will not bother implementing this.

I am also willing to spec a format using absolute positions in case a server doesn't want to implement the delta requests. However my fear is that server will start with this not thinking about the payload size and afterwards complain about things being slow. But in any case I will foresee different formats which the client can announce in case we want to extend this in the future.

So my question is: is there a big desire for a absolute position format. The relative encoded format can easily be converted into a absolute one. It is simply adding numbers :-) while iterating over the array of numbers.

[etc0de]

So my question is: is there a big desire for a absolute position format.

I think using absolute numbers only makes sense if the delta variant is just removed. If at all the protocol needs less choices, not more. A client could even request less than the viewport on a large screen to gradually update even the visible parts, so honestly I'm a bit surprised the delta format is that important for performance especially with the already shortened integer encoding. And apart from the delta's problems with being hard to understand, and duplicating what ranges do and risking client/server combinations having poor performance, I agree with the previous note that implementing it without ever desyncing might also be hard and result in buggy servers. Is it really worth it?

However a client still needs to get the semantic tokens for the full file to avoid color flickering when scrolling and to be able to color the minimap.

This could also be done in gradual range chunks in the background which would also bring down payload, and while not completely eliminating any possibility of flickering it would likely remove it in many cases. It doesn't seem to be necessarily a given that delta is needed for this, if that was the reason it was added. And a minimap updating with a slight delay doesn't seem like the end of the world to me, but that's just my opinion, obviously

[puremourning]

@dbaeumer thanks for the (first?) clear explanation about how this all works :)

i think the 'delta encoding within one array of ints' is probably fine on reflection; as you say it's trivial to implement (though it still feels like a strange optimisation to me - do you have numbers on the payload differences for real documents from your testing ?).

On a somewhat different point, what was the motivation for the "delta" request to be client-to-server rather than a server-to-client notification (e.g in response to document change notifications) ? What is the expected client behaviour ... to send a delta request after each document change ?

[kjeremy]

To make things more clear I propose the following renames:

textdocument/semanticTokens/full: request to get all semantic tokens of a text document.
textdocument/semanticTokens/range: request to get all semantic tokens of a specific range.
textdocument/semanticTokens/full/delta: to get the delta for a full document.
textdocument/semanticTokens/range/delta: we could spec it but I guess given the above most servers will not bother implementing this.

While not opposed to the renames (except that it means I have to change some code :)) they seem to go against the typical convention.

[axelson]

I am also willing to spec a format using absolute positions in case a server doesn't want to implement the delta requests. However my fear is that server will start with this not thinking about the payload size and afterwards complain about things being slow. But in any case I will foresee different formats which the client can announce in case we want to extend this in the future.

I don't think we need absolute positions. I think the integer-based encoding makes sense for minimizing the payload on the wire, and since all the needed information is available in a single request which it should be relatively straightforward to implement.

[dbaeumer]

@kjeremy can you elaborate why you think this. I am open for a better proposal. So far we didn't have the problem of a full and range request of the same type.

@puremourning LSP whenever possible uses a pull model (they only exception are diagnostics) driven by the client. The reasons are as follows:

• for a server to effectively compute these notification it needs to have a basic understanding of the UI model which I think we should never sync to the server. Consider the case that a change if file A changes semantic coloring in file B. Should a server always send an event for file B even if the file is not visible in an editor. It might be expensive to find out if B is affected.
• that servers don't need to compute the same result over and over again we started to introduce the resultId concept. A resultId is basically a HTTP eTag.

What is the expected client behaviour ... to send a delta request after each document change ?

No the client will pull for the delta assuming that the server will provide a delta request.

[puremourning]

Thanks again. Makes sense for the client to determine which buffers it wants tokens for, but...

What is the expected client behaviour ... to send a delta request after each document change ?

No the client will pull for the delta assuming that the server will provide a delta request.

Sorry i didn't follow this... When would you expect the client to poll (pull) the textdocument/semanticTokens/full/delta ? Is the expectation that clients just do it periodically ? Or after all changes in the text document (e.g. at same time as didChange notification?)

If i understood correctly, the general flow would be:

• The user opens a text document (textDocument/didOpen)
• The client requests textdocument/semanticTokens/full and gets a some tokens. Rosy.
• The user enters insert mode and starts typing some characters
• Client sends textDocument/didChange to server (maybe server issues diagnostics, etc.)
• Does the client _also_ send the textdocument/semanticTokens/full/delta alongside each didChange notification ?

FWIW - What I was expecting is something more like "register for tokens for file" (returning a snapshot of the full file tokens, followed by updates) until "unregister for tokens for file". So it's "Pull" as a query snapshot+updates paradigm with i guess a "cancel" mechanism. Feels more real-time that way to me, rather than the client polling for updates.

I realise that's a fairly meaningful departure in approach, and there probably isn't precedent for it in the existing protocol, but this is a new feature, and i think the success of this feature will entirely depend on the snappiness/promptness of UI updates. Many of us will have experienced unacceptable lag/"highlight popping" in existing semantic highlighting systems, so i'd certainly be curious which approach yields better feel in various clients. Hard to know without implementing both of course, and nobody wants that! :)

[dbaeumer]

@puremourning regarding your questions:

Does the client also send the textdocument/semanticTokens/full/delta alongside each didChange notification ?

Yes, but may be with a less intense frequency. We are also looking into adding batching support to make this more performant for servers (https://github.com/microsoft/language-server-protocol/issues/988)

FWIW - What I was expecting is something more like "register for tokens for file" (returning a snapshot of the full file tokens, followed by updates) until "unregister for tokens for file". So it's "Pull" as a query snapshot+updates paradigm with i guess a "cancel" mechanism. Feels more real-time that way to me, rather than the client polling for updates.

The problem with this approach is that the server doesn't know when to stop pushing. If the file is not visible anymore there is no need for the server to re-compute semantic tokens. Please note that the open/close calls are no indication for visibility. A document can be visible in the UI without the client every sending a open event. Open/close events are a ownership indication.

[puremourning]

The problem with this approach is that the server doesn't know when to stop pushing.

I was suggesting that the client "un-registered" for the updates with another message, e.g."

• -> textDocument/semanticTokens/subscribe (document/range)
• <- snapshot
• <- update
• <- update
• -> textDocument/semanticTokes/unsubscribe (document/range, or perhaps sub ID)

[etc0de]

I don't think we need absolute positions. I think the integer-based encoding makes sense

Just to explain this again since apparently multiple people were confused, my related suggestion was to use absolute positions only and otherwise stick with integer coding, but scrap the delta stuff entirely. I agree offering absolute position as some sort of option is kind of pointless since all my suggestions aim to reduce the current protocol complexity, not increase it further.

On the same note I think the push model might make sense to make the delta stuff work better, but I think it might still be better to just remove it entirely and just use ranges only.

[kjeremy]

@kjeremy can you elaborate why you think this. I am open for a better proposal. So far we didn't have the problem of a full and range request of the same type.

Only that this would be the first case with three or more levels of specification textDocument -> semanticTokens -> full/range. It's just something I view as inconsistent (or maybe just new?) but I don't have a better proposal.

[rcjsuen]

@kjeremy can you elaborate why you think this. I am open for a better proposal. So far we didn't have the problem of a full and range request of the same type.

@dbaeumer @kjeremy What about the formatting requests?

• textDocument/formatting
• textDocument/rangeFormatting

[matklad]

Just want to report that we at rust-analyzer are pretty happy with the wire format. Relative positions and edits make total sense (albeit only after "In addition we specified that data structure in a way that computing the delta is done on a number array without any knowledge of the data's semantic" comment), I don't think we can improve this further, short of swapping JSON for protobuf.

[etc0de]

For what it's worth, I am happy to be ignored if the conclusion is I'm the minority thinking the delta edits mostly just double the ranges and are more trouble than worth adding. It is at the end of the day more like an annoyance, not a total showstopper. However, I think a note saying which of ranges and deltas should be preferred by both sides when expecting good perf to avoid the "performance deadlock" I described above would be very useful if both mechanisms are kept.

[Pyromuffin]

Maybe I missed it, but what is the plan for the situation referenced

for a server to effectively compute these notification it needs to have a basic understanding of the UI model which I think we should never sync to the server. Consider the case that a change if file A changes semantic coloring in file B. Should a server always send an event for file B even if the file is not visible in an editor. It might be expensive to find out if B is affected.

Currently using the vscode proposed features, when editing a header file, i get stale semantic tokens in any file that includes it. I'm not sure if I should do something on the server to tell the client to re-request tokens for a file or if vscode should be re-requesting tokens regularly. The client doesn't have any knowledge of the actual semantics of the language, so i wouldn't expect it to know when a file needs to be refreshed.

[dbaeumer]

Regarding https://github.com/microsoft/language-server-protocol/issues/18#issuecomment-635986774

I agree, but today I would make the textDocument/formatting/range textDocument/formatting/full and textDocument/formatting/onType.

Especially around semantic tokens I think it does even make more sense to have an extra level since the requests are not independent of each other.

[dbaeumer]

Regarding https://github.com/microsoft/language-server-protocol/issues/18#issuecomment-635861405

We really try to stay away from syncing UI state over to the server or to ask to implement a subscribe / unsubscribe model. Major reason is that this that this gets complicated to implement in a remote scenario or if we make servers multi tenant. To allow for this evolution path we decided to whenever possible use a pull model over the push model since a client can easily recover from network failures. In the semantic token case if the network goes down the client and simply ask for all semantic tokens again.

[dbaeumer]

Regarding using only ranges in https://github.com/microsoft/language-server-protocol/issues/18#issuecomment-635866688

If it is about one request with a range that can spawn the whole document then a server has no way to indicate if it implements a special range request or not.

Typically the major amount of time spent to compute semantic tokens is to run the type binding phase in the compiler. If not optimized (e.g. by using a type pull model) libraries usually compute the type bindings for the whole file. This being said a range request is very likely equal expensive (minus the payload on the wire) than computing the tokens for a whole file. Having to end point allows the server to tell the client that it only supports whole documents and no range since there will actually be no win.

[dbaeumer]

Regarding https://github.com/microsoft/language-server-protocol/issues/18#issuecomment-643693626

The idea is that the client pulls for semantic tokens for all visible files. The one not having the focus should be pulled with less frequency. Assuming that the server implements the delta mechanism a pull for a file that has no semantic color changes will not produce any additional payload. This model was chosen to not sync any UI state. Otherwise the server might do an expensive impact analysis about which files need to refresh semantic tokens although non of them are visible in the UI.

[dbaeumer]

I did a pass over the proposed implementation to allign it with suggestion from this issue:

• enhanced client capabilities https://github.com/microsoft/vscode-languageserver-node/blob/master/protocol/src/common/protocol.semanticTokens.proposed.ts#L166
• renames to reflect the [[full, range], delta] pattern better

[etc0de]

Having to end point allows the server to tell the client that it only supports whole documents and no range since there will actually be no win.

Sorry, I have trouble reading that. Can you try to reword it? However, here is my response to what I think I understood:

Just my thought process cleared up: Clients could range query in roughly viewport-sized chunks (or even smaller) and gradually like that for the entire file, with a higher priority if the user actively scrolls or types somewhere. While this has high bandwidth cost in the long run, it should be pretty cheap both computationally and in data sent in any given small time frame. So where's the significant win in delta'ing?

Response to what I think you wrote: If your argument was that servers would recompute the entire file for any range request making it as slow as fetching the entire document, why couldn't servers just look at the modified timestamp and/or edit events sent and otherwise cache it? It seems like that would be required for deltas as well to be efficient, after all the server needs to somehow be able to tell that the document updated to compute a delta too (if it doesn't want to recompute the entire thing all over again and again even when nothing changed). So I don't really get your argument here

[dbaeumer]

From our experience colorizing a range can be as expensive as coloring the whole file. This is due to the fact that to colorize the file the type bindings need to be resolved. Some tools have implemented a pull model for that (e.g. TypeScript, which is faster on range) but other don't (e.g. Java). The ones that don't are better off to always compute the color information for the whole file than being as for the range first and then for the whole file. These servers can not provide a range request which would ask the client to always ask for the whole file.

Regarding caching: we should not make caching a prerequisite. It should be up to servers to decide when to cache. We even investigate into batching to make it easier for servers to bring project states up to date, run design time builds or type binding phases (https://github.com/microsoft/language-server-protocol/issues/988).

For the delta: this is designed in a way that the server doesn't need to keep an AST or resolved bindings. It only needs to keep the last number array which is a lot smaller. And to detect that nothing has changed the server can use the version numbers send with document notifications.

As you can see with the new proposed protocol there is now a format and capabilities to indicate what requests the client will sent.

[etc0de]

For the delta: this is designed in a way that the server doesn't need to keep an AST or resolved bindings. It only needs to keep the last number array which is a lot smaller

Sorry, I have trouble following this: @ AST: this is never needed to be kept for caching, neither for ranges nor deltas. What needs to be kept is the tokens in BOTH cases (for the delta to make a diff, for ranges to resend a cached range), so no difference here. (And the server could use your "number array" internally for both, so no difference here either.) Tracking changes on disk with version numbers and document notifications also is the same with diff and ranges, again no difference. So I'm really still quite baffled why delta is supposedly so useful. I get the benefits on transfer savings of course, I'm just not sure that alone is worth it. But I'm repeating myself, I should stop here.

Edt: to clarify the above into a more concrete suggestion, I feel like all the gains you attribute to deltas can be achieved similarly by writing "LSP servers should keep the tokens cached in an efficient internal format to make multiple range queries on an unchanged document fast." While I guess the delta format is a way of somehow forcing server writers to put this in, that seems to me like an unnecessarily over-engineered workaround to avoid just writing this suggested sentence into the spec. Although obviously the bandwidth gains remain, I don't want to explain those away.

Edit 2: and I deeply apologize for repeating myself so much. I am trying my best to make my points understandable to hopefully avoid this going on forever

[dbaeumer]

I agree that the server can keep whatever it feels best to produce the delta. This must not necessarily be the number[].

As I already outline a couple of times the delta is optional. A server will still function correctly if it is not providing it. We don't force servers to implement the delta. But in the light of more and more servers being used in a remote scenario having the delta is a useful addition. And with the latest changes I did you can even implement another format with another set of requests.

[kjeremy]

And to detect that nothing has changed the server can use the version numbers send with document notifications.

Is this really true? I don't think this is reliable in a language that supports imports.

[dbaeumer]

@kjeremy for the content of a file it is enough. If you have inter file dependencies and you know that semantic coloring (or any other feature) invalidates file A if file B changes then your server needs to model that dependency tree. But there timestamps don't help either.

[axelson]

As you can see with the new proposed protocol there is now a format and capabilities to indicate what requests the client will sent.

Is there a link where the updated protocol can be reviewed? Is it still primarily in the typescript server?

[dbaeumer]

@axelson the proposed version is here in terms of implementation. I am in the process of writing the markdown. https://github.com/microsoft/vscode-languageserver-node/blob/master/protocol/src/common/protocol.semanticTokens.proposed.ts#L1

[dbaeumer]

And here is a first version of the spec. No word polish and no spell check :-)

https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_semanticTokens

[kjeremy]

@dbaeumer When will we need to move our LSP server implementation over to this one from the current semantic tokens implementation supported in vscode?

[dbaeumer]

@kjeremy Are you relying on the implementation in the next version of the LSP libs ?

[kjeremy]

@dbaeumer Our server-side implementation is in rust and based on https://github.com/gluon-lang/lsp-types which will need to be updated. The client-side opts in via https://github.com/rust-analyzer/rust-analyzer/blob/master/editors/code/src/client.ts#L151

[etc0de]

@dbaeumer I really think a remark that clients are expected to cache locally with ranges would be quite important unless you want to make deltas de-facto required for servers. Right now, I can't see such a remark, it should probably go into "Implementation considerations." (Unless you don't think it should be optional @ deltas, but you did sound like you wanted to keep it in but as optional.)

If you don't write that in, some clients will just not do such caching and then servers will be required to implement deltas to guarantee good performance or users will blame it on them, making deltas basically a must.

Edit: also, right now the general concepts part is mixed with the pretty specific integer encoding in one block, I find that this is suboptimal for readability.

Suggestion in detail to split up text:

I think the General Concepts section would benefit from the integer encoding part split into a separate section placed separately. In detail, I am suggesting to split at the following start/end points:

Split start at:

On the capability level types and modifiers are defined using strings. However the real encoding happens using numbers to save bandwidth. [SPLIT HERE]

I would start to cut out parts for a new section after this, named Integer Encoding for Tokens, which contains the text up to BEFORE this paragraph:

Split end at:

[SPLIT HERE] The protocol defines an additional token format capability to allow future extensions of the format. The only format that is currently specified is relative expressing that the tokens are described using relative positions.

So the new General Concepts starting section would go like:

[... General Concepts as before ...]
On the capability level types and modifiers are defined using strings. However the real encoding happens using numbers to save bandwidth, see Integer Encoding (link to section).
The protocol also defines an additional token format capability to allow future extensions of the format. The only format that is currently specified is relative expressing that the tokens are described using relative positions.
[... General Concepts ending parts as before...]

... with the Integer Encoding for Tokens section separated out to below based on above cut out starting with:

The server therefore needs to let the client know which numbers it is using for which types and modifiers. They do so using a legend, which is defined as follows: [...remaining Integer Encoding concepts up to split end...]

This new section could be additionally prefixed with an introducing sentence like: This section describes the integer encoding used for data transfer.

[kjeremy]

@dbaeumer

I'm trying to figure out the delta behavior and have some questions:

1. Does the server fill in the resultId for the range request? That seems odd to me (I'm not sure what I would do with that information).

2. For the delta I am holding onto all the tokens for the file and computing a diff against that and returning it like how SemanticTokensBuilder works. Does the resultId returned from the delta request represent the state of ALL the tokens in the file?

I guess ultimately I'm trying to figure out what I need to hold onto to compute the deltas. I am assuming that each deltas really asks for the delta between "now" and the previous delta or full request. It does not appear to be spec'd that way however and it could be that a client asks for a delta between "now" and many revisions ago.

[puremourning]

@dbaeumer i'm reading through the initial draft spec (thanks for writing it up!). I have a handful of comments/questions/clarifications. What's the best way to provide feedback? would you like comments on a PR or something like that?

[dbaeumer]

@puremourning I think the comments are best provided as a PR.

[dbaeumer]

@kjeremy you are correct a delta for range makes no sense. I am pretty sure VS Code will basically drop a previous result ID.

For the delta is should be always reported against the last result independent whether this was a full or a delta response. So in an easy implementation the id is simply incremented. I have clarified this in the spec.

[dbaeumer]

@etc0de thanks for the suggestions. I made them in the 3.16 version of the spec.

[MarFren]

Regarding #18 (comment)

The idea is that the client pulls for semantic tokens for all visible files. The one not having the focus should be pulled with less frequency. Assuming that the server implements the delta mechanism a pull for a file that has no semantic color changes will not produce any additional payload. This model was chosen to not sync any UI state. Otherwise the server might do an expensive impact analysis about which files need to refresh semantic tokens although non of them are visible in the UI.

If I understand it correctly, the basic idea is to let the client pull semantic tokens for all open files on a regular basis, the one having the focus with high frequency, the other files with low frequency (but automatically). That would allow for updating the semantic highlighting in all open files, even if file A has the focus and a change here does induce a change in highlighting in another file B - am I correct? If so, the current reference implementation in VSCode Insiders (1,49.0-insiders) does not implement automated pulls of semantic tokens of open files, right? Wouldn't it be good to describe the expected pull behavior in the 3.16 specs of LSP?

[kaby76]

Now that my LSP server is more stable, I decided to update the extension for VSCode and give the semantic highlighting feature a spin. On the server, I decided to first try out SemanticTokensOptions { range = false, full = true}. However, I see the following:

[Error - 8:40:58 PM] Request textDocument/semanticTokens failed.
  Message: No method by the name 'textDocument/semanticTokens' is found.
  Code: -32601

Well, yeah, that's right. There is no "textDocument/semanticTokens" message because it only mentions in the spec "textDocument/semanticTokens/range", "textDocument/semanticTokens/full", and "textDocument/semanticTokens/full/delta", but no "textDocument/semanticTokens". Is this spec up to date? I can debug VSCode, figure out what is it expecting, and program to the implementation, but it would be nice to know what's going on.

[DanTup]

@kaby76 I think the request is constructed by the vscode-languageclient package so the issue might be that the extension is not using the latest version of that package (7.0.0-next.9 is the one I've been using to test in the Dart extension and it seems to work well - though I've only implemented /full so far).

[dbaeumer]

@MarFren adding this as a recommendation to the spec makes sense. A PR is welcome. However the LSP spec never enforces this. If a client decides to not do this it should still be fine.

[DanTup]

@dbaeumer in the spec for semanticToken/range it says the result is:

result: SemanticTokens | null where SemanticTokensDelta

It seems like the line is incomplete (or the last part shouldn't be there).

If the return value is SemanticTokens, is the lineDelta from the range that was requested, or from the start of the document? (I couldn't find this mentioned anywhere).

[kaby76]

@DanTup That worked. I had to also set up a few other dependencies

    "vscode-jsonrpc": "^6.0.0-next.5",
    "vscode-languageclient": "^7.0.0-next.9",
    "vscode-languageserver": "^7.0.0-next.7",
    "vscode-languageserver-protocol": "^3.16.0-next.7",
    "vscode-languageserver-types": "^3.16.0-next.3"

use the "Insiders" VSCode, and change an import because LanguageClient was moved around:

import * as vscodelc from 'vscode-languageclient/node';

But, "textDocument/semanticTokens/full" starting to work. Computing the start line/col deltas a little challenging. And VSCode does not work the same as the LSP client in VS2019 with edits.

[DanTup]

I had to make the /node import changes, though I don't think I needed to use insiders (nor add the other dependencies).

Computing the start line/col deltas a little challenging

I've been refactoring my server work to collect the tokens using absolute data initially (line/cols, enum types) and then at the end do the conversion to the LSP format. This made it much simpler than when I was also doing things like splitting up multiline/nested tokens at the same time. The final conversion is now relatively simple:

var lastLine = 0;
var lastColumn = 0;

_tokens.sort(
  (t1, t2) => t1.line == t2.line
      ? t1.column.compareTo(t2.column)
      : t1.line.compareTo(t2.line),
);

for (final token in _tokens) {
  var relativeLine = token.line - lastLine;
  var relativeColumn = relativeLine == 0
      ? token.column - lastColumn
      : token.column;

  encodedTokens.addAll([
    relativeLine,
    relativeColumn,
    token.length,
    semanticTokenLegend.indexForType(token.type),
    semanticTokenLegend.bitmaskForModifiers(token.modifiers) ?? 0
  ]);

  lastLine = token.line;
  lastColumn = token.column;
}

[kaby76]

@DanTup Yes, my code looks more or less just like your code after I noticed initially that only the first symbol was being colored. I tried out some hardwired values, then understood what had to be done (i.e., sort + compute diffs).

[dbaeumer]

I will close the issue since SC is now part of the upcoming 3.16 spec.