Add a "group" field to Diagnostic#681
Conversation
|
Relevant comment: microsoft/language-server-protocol#166 (comment). This seems like a small enough change, but I really hope that other LSP clients will only ignore extra data and not crash/misbehave on receiving custom diagnostics. @sebastiencs could you please also try this with other LSP clients and see if it's not causing any troubles? That'd be very much appreciated! |
|
@Xanewok Alright I will test with some editors and come back here |
|
I just made the PR on @Xanewok I have tested with vim/neovim, VS Code, Sublime Text and emacs. |
Xanewok
left a comment
There was a problem hiding this comment.
Let's land this together with languageserver-types update
src/actions/post_build.rs
Outdated
| v.clear(); | ||
| } | ||
|
|
||
| let mut group = 0; |
There was a problem hiding this comment.
style nit: could we use for (group, msg) in messages.iter().enumerate() instead? Seems cleaner than manually updating the group counter
0fcff80 to
ca7d41a
Compare
|
@Xanewok Done, I've updated |
|
Awesome, thanks! I'll merge this now, but in the meantime it'd be good to document any extensions we're currently using (such as this one). @sebastiencs would you mind also updating the https://github.com/rust-lang-nursery/rls/blob/master/contributing.md#extensions-to-the-language-server-protocol section, mentioning the change here? |
|
Alright I will do that :) |
With #664, clients now receive secondary labels as a single diagnostic.
The issue is that clients don't know to which error belong all the secondary diagnostics.
Adding a
groupfield solves this problem.I intentionally made the minimum and simple changes to the code. Clients are free to ignore the field
I worked a bit with my editor to display the labels and it renders like this:
If this is accepted I will make a PR to languageserver-types
cc @Xanewok @alexheretic