Dart-Code / Dart-Code

Dart and Flutter support for VS Code
https://dartcode.org/
MIT License
1.49k stars 306 forks source link

Use the analysis server for semantic highlighting #2202

Closed simolus3 closed 3 years ago

simolus3 commented 4 years ago

On the insiders channel, VS Code now provides apis for semantic token highlighting. This would allow Dart Code to enhance the static grammar-based highlighter with highlighting information from the analyzer.

In particular, the extension could use it to:

To my knowledge, LSP doesn't support syntax highlighting (yet). So I think this would have to be implemented using the DAS protocol and a contributor for now. I've started some work on this and I hope I can open a PR next week.

Current issues

DanTup commented 4 years ago

@simolus3 I've uploaded a build of Dart-Code that uses the preview LSP client that includes semantic tokens:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.14.0-dev.semantic-tokens

You can download the vsix and install using the Extensions: Install from VSIX command in VS Code.

I have (temporarily) disabled the built-in syntax for that version to aid testing. This means files will be uncoloured until the server provides highlights (though when we ship for real, the original Dart syntax will be included to avoid a delay in providing some colours while the server is doing initial analysis etc.).

An easy test that it's working in Dart files is to create a variable starting with an uppercase letter - this was incorrectly coloured as a type in the original grammar.

Screenshot 2020-08-27 at 14 07 22

The latest server code is here:

https://github.com/DanTup/sdk/tree/lsp-3-16-semantic-tokens

It fixes multiline tokens, but not yet overlapping ones. For Dart, I might create a custom highlight_computer for LSP since the mapping is quite complicated (esp. for multiline), though for plugins we'd either need to continue to map, or introduce an LSP-specific tokens API for plugins (we don't currently have any LSP-specific plugin APIs, but I suspect we'll want to start at some point).

I'm undecided about when to start trying to land things in the SDK repo yet. I was planning to wait until the LSP spec is final, but I may consider putting it behind a flag or something to avoid building up a huge review.

simolus3 commented 4 years ago

Thanks! I can see that this works for Dart files, but I'm having no luck with my plugin. Is there a way to view the LSP communication between the extension and the server? Highlights generated by my plugin show up in the instrumentation log, but I'm not seeing anything in the editor. It might just be that I just generate invalid tokens, but seeing the LSP messages would help to diagnose this.

but not yet overlapping ones.

What I did in my original PR was to split tokens when they overlap or nest. So if I had tokens like this:

"This is a $string literal"
\-------------------------/       string
           \-----/                variable

I'd split them into

"This is a $string literal"
\---------/        \------/       string
           \-----/                variable

They still display as intended without overlapping. Basically, I sort regions by ascending offset and then by descending length. I can then iterate through them, and keep track of the current active span. If the next token intersects with that span, I split the original token around the new one. As far as I remember, this worked pretty well in the client back then. Maybe the server can do this transformation too? I'm happy to port it to the server if you think it's necessary. But yeah, for Dart it might be more efficient to write another highlights computer that doesn't generate overlapping tokens at all.

DanTup commented 4 years ago

Is there a way to view the LSP communication between the extension and the server?

You can use the Dart: Capture Logs to capture the traffic (or enable the dart.analysisServerLogfile setting), but what's in the instrumenation log should be the same thing - it's just logged by the server on the way out.

What I did in my original PR was to split tokens when they overlap or nest.

I did push fixes for overlapping tokens - but I realise now I didn't handle this properly. The examples I had (like imports and annotations) had nested tokens that filled the whole of the "outer" token. I hadn't gotten to string interpolation yet, I guess that would've turned up that I did it badly. I'm going to restructure how it works a little, as the current multi-line handling is really messy and I have some ideas to simplify it.

I do think we should try to support mapping the existing regions, whether we have a new computer for Dart and/or a new API for plugins, for compatibility.

simolus3 commented 4 years ago

Just tried to type around a bit and suddenly it worked :tada:

grafik

I was afraid that latency and the conflict between the notification and pull-based model might cause problems, but I didn't notice anything so far. The inconsistencies are probably because of my plugin, so this looks really awesome so far.

DanTup commented 4 years ago

Neat!

I've started tidying the server implementation up a little (https://github.com/DanTup/sdk/commit/ef362ea6f011f65b1fa1525ffbc11664aa76139d), though nothing is merged yet (I'm not sure of the timeline of the next LSP spec version - it has quite a lot of open issues on GH suggesting it might not be soon so I'm unsure about trying to land the spec updates in the SDK just yet).

I'd like to add range support (this would allow the editor to get tokens faster for the visible code rather than need the whole file), though I don't know if it would end up in us doing more work (eg. if we're re-computing the entire file for each range request) so I need to do a little testing.

DanTup commented 4 years ago

@simolus3 still haven't forgotten this, but it's still unfinalized in LSP so I've been holding off merging or doing any more. There are also a few things I've asked about, like supporting multiline/nested tokens that haven't really been resolved yet.

DanTup commented 3 years ago

@simolus3 LSP v3.16 is due to be finalised imminently (hurrah!). I have started preparing to land changes in the server:

https://dart-review.googlesource.com/c/sdk/+/175722/

Once LSP 3.16 is final (and the next version of VS Code) I'll make a new preview build of Dart-Code with the new client that will work with the server changes.

I've included a basic plugin test in that CL, though if there are cases you think it doesn't cover, please let me know (or feel free to contribute changes).

simolus3 commented 3 years ago

Awesome, thanks for letting me know! I took a look at your CL and I think your test cases cover what I'm interested in.

DanTup commented 3 years ago

Fixed by https://github.com/dart-lang/sdk/commit/cb2ede57b7c91fb61c0cb05971cc1823faee2086.

In order to get the fix you'll need Dart-Code v3.18.0 (a preview release should be available later today/tomorrow), be using LSP Preview, and have a Dart SDK from after yesterday (the most recent nightly works).

@simolus3 once the Dart-Code preview is available later, please give this a try with your plugin and let me know if you hit any issues. Thanks!

semantic_tokens
DanTup commented 3 years ago

Preview build is here: https://github.com/Dart-Code/Dart-Code/releases/tag/v3.18.0-alpha.1

ericdallo commented 3 years ago

Cool @DanTup, I'll test it with lsp-dart since already have support for semantic highlighting, thank you!

DanTup commented 3 years ago

FYI in case you see this - there was a bug in where overlapping tokens might not be processed correctly (due to an unstable sort) that could result in some tokens being dropped (the blue imports):

Screenshot 2020-12-16 at 16 50 25

I've got a fix at https://dart-review.googlesource.com/c/sdk/+/176446/ though it won't be in the nightly until tomorrows (assuming I get it landed today).

DanTup commented 3 years ago

FYI - I've disabled Semantic Tokens in the server while I get a fix for a race condition in the code that handles splitting multiline tokens. Hoping to get a fix soon.

DanTup commented 3 years ago

Due to the issue above, I'm reverting the change to support LSP v3.16 from the upcoming release and will instead aim to ship it in the next one. You can still use the preview build for it (and I can build. anew preview build after the release that re-enables it), though you'll want a version of the SDK with all the fixes to avoid the issue mentioned above.

DanTup commented 3 years ago

The issues in the server were fixed and landed, and the Dart-Code change to re-enabled them landed in a11cba2d.