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

Explore providing secondary labels and errors/notes using textDocument/hover instead #662

Closed
Xanewok opened this issue Jan 17, 2018 · 10 comments

Comments

@Xanewok
Copy link
Member

Xanewok commented Jan 17, 2018

One obvious use case is for mismatched types error now: #550. Since we don't include the information from secondary diagnostic (expected <type>, found <another>) in the published IDE diagnostics, users are not informed why exactly is there an error and how to fix it.

iirc @nrc also suggested that we may additionally include the secondary diagnostics info as a response to textDocument/hover.

@nrc
Copy link
Member

nrc commented Jan 17, 2018

I prefer using hover for the secondary diagnostic info - the problem with merging is that VSCode does no formatting, line-breaking, or overflow handling, so the info disappears off the side of the window and is unreadable (although iirc, it looks OK on hover, it's just the problem window which is annoying). By doing the hover stuff ourselves, we should also be able to handle multiple spans nicely.

@alexheretic
Copy link
Member

I started looking at this yesterday before you raised this, great minds huh? #664

@algesten
Copy link
Contributor

@alexheretic did you make any headway on this? @Xanewok created this issue after me trying to find some work to do on IRC :)

But if you're making good progress, I'll look elsewhere for stuff to do. Maybe you can raise a PR to show progress?

@alexheretic
Copy link
Member

Yep #664 includes full messages & thanks to @Xanewok also generates additional secondary diagnostic highlights.

I think its in a state worth merging now, check out the pr and see what you think.

@algesten
Copy link
Contributor

Cool. Thanks!

@Xanewok
Copy link
Member Author

Xanewok commented Jan 21, 2018

@algesten hey, sorry it turned out like this - didn't want to specifically mention you not to make you feel pressured or obliged to do it!
In the future I'll try to mention who expressed their intention to tackle the issue, however if it's not the case, feel free to also say that you're interested and I'm sure people will also be willing to help 😄

@algesten
Copy link
Contributor

@Xanewok no worries. There are other things I can do. :)

@Xanewok
Copy link
Member Author

Xanewok commented Jan 21, 2018

For now we present this all in a single diagnostics message (#664 has more shots how it currently looks like), but it'd still be good to see how it looks like when providing secondary diagnostics on a textDocument/hover, instead of merging this in one message.

One benefit is that hover supports Markdown and so we could provide faimiliar rustc formatting (bolding, code backticks).

However, one thing to keep in mind are terminal editor users. If only diagnostic messages are displayed inline in those, It might not be a good idea to take secondary span labels away from the message itself and move it to hover and only do that for notes/hints.

@autozimu wrt the above, how do you think the secondary diagnostic should look like in Vim and similar?

@Xanewok Xanewok changed the title Explore merging primary+secondary compiler diagnostics before publishing to IDE Explore providing secondary labels and errors/notes using textDocument/hover instead Jan 21, 2018
@autozimu
Copy link
Member

The extra formatted message would be really helpful.

But is it possible to have those messages in both published diagnostics and hover response?

For current vim/neovim, when a user navigate to a specific line, the cached diagnostic will be displayed (though extra work to be done under autozimu/LanguageClient-neovim#224), but currently hover would require an explicly action from user and takes a round trip to language server.

@nrc
Copy link
Member

nrc commented Jan 22, 2018

But is it possible to have those messages in both published diagnostics and hover response?

I don't think this is possible, since the client would display the 'extended' bit of the error twice when hovering.

@nrc nrc closed this as completed Jun 21, 2018
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

5 participants