Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Semantic Highlighting #85

Closed
chergert opened this issue Nov 3, 2016 · 13 comments
Closed

Semantic Highlighting #85

chergert opened this issue Nov 3, 2016 · 13 comments

Comments

@chergert
Copy link

chergert commented Nov 3, 2016

Unfortunately, the Language Server Protocol does not yet include API for proper Semantic Highlighting. In Builder, I landed a hack to work around this that has a small caveat.

I've abused the textDocument/documentSymbol API to get a list of symbols in a file and then fuzzy match on them when highlighting the source document. As you might imagine, this limits what we can semantically highlight to symbols that exist within the current rust document.

However, if we were to have an API that allowed us to get an index of all symbols within the project, we could build our index for the highlighter from that, giving us something close to being correct.

Basically all I need from the API is the name and kind property from the textDocument/documentSymbols endpoint so that I can select the proper highlight style in from our style scheme. I think this might be doable simply by implementing the workspace/symbol endpoint and returning everything for an empty query.

Thoughts?

@chergert
Copy link
Author

chergert commented Nov 3, 2016

I also imagine that to avoid calling this too much, we can put some tricks in our highlighter engines to detect line changes that affect new symbols being created. (look for struct, fun, let, etc etc).

@bruno-medeiros
Copy link
Contributor

Word of advice, I'm not sure it would be worth to implement Semantic Highlighting in the client in such a hacky and cumbersome way, you might be better off waiting for native support for it, either in rls extension and (eventually, hopefully) a new LSP addition.

I imagine the best way to implement this would be for the LS to send some sort of notification back as soon as it gets text changes (similar to PublishDiagnostics , but only for opened documents).

@chergert
Copy link
Author

chergert commented Nov 4, 2016

Agreed. Since we already notify rls via textDocument/didChange as to buffer changes, it would be nice if rls kept track of the invalidated regions. However, it was my understanding that Rust does not currently have an incremental compiler, which makes tracking AST changes to a single tree rather cumbersome and costly. Without the ability to keep the invalidation region bounded we would have to update highlighting from the change position to EOF.

@nrc
Copy link
Member

nrc commented Nov 6, 2016

I think this is mostly a dup of #49

@nrc
Copy link
Member

nrc commented Nov 6, 2016

@chergert
Copy link
Author

chergert commented Nov 6, 2016

I haven't thought through how to "abuse" that API, but I think its targeted more at the use-case of "my insert cursor has moved, what related things should get a background highlight".

@nrc
Copy link
Member

nrc commented Oct 30, 2017

AFAIK, there is no good way to implement this and the LSP is unlikely to ever add facilities for us to do so.

@nrc nrc closed this as completed Oct 30, 2017
@torpak
Copy link

torpak commented Oct 30, 2017

Then maybe it was the wrong move to follow microsofts lead, if such basic functionality is ruled out there.

@Xanewok
Copy link
Member

Xanewok commented Oct 30, 2017

There is a pending proposal PR for LSP 4.0: microsoft/language-server-protocol#124

@HighCommander4
Copy link

@nrc I'm curious why you say "the LSP is unlikely to ever add facilities for us to do so". It seems to me like it's being worked on (see previous comment).

@nrc
Copy link
Member

nrc commented Nov 13, 2017

The last time I talked to the VSCode team (which was a little while ago, so things may have changed) they felt that the editor components couldn't be made to work fast enough if they were tied it to the LSP in this way, and therefore any kind of LS-backed colouring wouldn't work (iirc).

@HighCommander4
Copy link

HighCommander4 commented Nov 13, 2017

Interesting, thanks for the info.

That said, I don't see any technical reason to be blocked on the official LSP protocol, or on the VSCode team, for trying this. If rls and rls-vscode agree on a protocol extension for semantic coloring (could be the one proposed in microsoft/language-server-protocol#124, or something else), they can implement it without requiring anything from LSP or VSCode itself.

@HighCommander4
Copy link

HighCommander4 commented Jan 5, 2018

For example, cquery (a C++ language server) has implemented semantic highlighting as an LSP protocol extension, with the cooperation of its VSCode client plugin, and it performs well enough.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants