Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnostic hover doing "word-wrap: break-word;" which makes them hard to read #22239

Closed
joshbw opened this issue Mar 8, 2017 · 7 comments
Closed
Assignees
Labels
editor-contrib Editor collection of extras languages-diagnostics Source problems reporting under-discussion Issue is under discussion for relevance, priority, approach

Comments

@joshbw
Copy link

joshbw commented Mar 8, 2017

  • VSCode Version: 1.10.1 & Insider for Mar 8 2017
  • OS Version: Win 10 RS1

Steps to Reproduce:

  1. Install an extension with fairly long diagnostic hovers (an example included below)
  2. trigger the diagnostic hover
  3. notice that the wrapping of text is breaking words, which makes reading the hover content much more difficult

I'm not entirely certain what to recommend to address this. For the majority of text it is better to just have the word drop down a line rather than be broken, but for long URLs and such breaking a word is preferable. I don't know if you can selectively break words by wrapping them in a span with css "word-wrap: break-word;", but if you can a strategy might be to preprocess the hover text and wrap URLs/UNC paths in said span (I don't know what else other than URLs and paths would be long enough that they need to break-word to fit in the hover box). Alternatively, and this is my preference, you could support links within the hover, so that link text rather than the full URL can be shown. That both makes it easier for people to access the URL (they could just click on it, instead of copying it out of the hover box and pasting in a browser) and provides a mechanism to show shorter text that doesn't need to be broken up

If you want a diagnostic that reproduces this scenario you can grab and install vscode-devskim-0.1.0.vsix from https://github.com/Microsoft/DevSkim-VSCode-Plugin. Once installed, create a new .cpp file and type gets(str); into it - DevSkim will catch the use of gets and add a diagnostic with text long enough that it triggers the undesired wrap conditions

@jrieken
Copy link
Member

jrieken commented Mar 9, 2017

@joshbw can you attach a screen shot/capture please?

We have some special behaviour here because often diagnostics messages are 'ascii-formatted' like below and try to preserve that. Unsure if other word-wrap tricks can be used

some.code()
     ^^^^^
     unknown something, /some/file.fooLang,12,7

@jrieken jrieken added the editor-contrib Editor collection of extras label Mar 9, 2017
@joshbw
Copy link
Author

joshbw commented Mar 9, 2017

wrap

you can see that condition and a couple other words are broken in the middle

@joaomoreno
Copy link
Member

joaomoreno commented Apr 6, 2017

There is a constraint here. We use monospace font for diagnostics just so we can support the ^^^^ scenario, despite most text in this container is plain English, by far. Also despite the fact that we could render such information in much more user friendly way. Anyway, monospace fonts increase the width of each character and thus of each line, increasing the probability of wrapping.

At the same time, people sometimes place reallyLongMethodNamesOrFunctionsThatAreBig in these, making wrap by words look really ugly.

So the only way to solve this is to continue with monospace and have character wrapping.


Going forward. Deprecate the current way diagnostics are rendered and make them work just like the regular hover: a Markdown string which defaults to a non-monospace font but still lets language feature providers open up monospace code blocks with triple back quotes.

@joaomoreno joaomoreno removed their assignment Apr 6, 2017
@joshbw
Copy link
Author

joshbw commented Apr 6, 2017

make them work just like the regular hover: a Markdown string

That would be great - I'd love to have access to MD in a diagnostic

@jrieken jrieken added the under-discussion Issue is under discussion for relevance, priority, approach label Apr 21, 2017
@smarter
Copy link

smarter commented Jun 2, 2017

Going forward. Deprecate the current way diagnostics are rendered and make them work just like the regular hover: a Markdown string which defaults to a non-monospace font but still lets language feature providers open up monospace code blocks with triple back quotes.

+1 to that, just had a user of our language server implementation report this and I guess there's nothing we can do on our side: scala/scala3#2665

@smarter
Copy link

smarter commented Jun 2, 2017

Opened microsoft/language-server-protocol#250 to track this on the LSP side.

@jrieken jrieken added the languages-diagnostics Source problems reporting label Sep 11, 2018
@jrieken
Copy link
Member

jrieken commented Sep 11, 2018

closing this in favour of #54272

@jrieken jrieken closed this as completed Sep 11, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-contrib Editor collection of extras languages-diagnostics Source problems reporting under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

4 participants