New whitespace / layout / styling PR

[braver]

Replaces #168 after the codebase was split. Please refer to that PR for the epic ride towards the new good looking popups with code highlighting. It was pretty much impossible to rebase or merge this so I decided to re-implement.

Fixes #105 and fixes #82, pretty sure it also fixes #159

Please also note we should update our dependencies

Before:

After:

[tomv564]

About preserve_whitespace - is it not standard Markdown behaviour to ignore line breaks?
Mdpopups even provides a Python markdown extension markdown.extensions.nl2br to work around this, but the question is still if LSP should mess with language server's responses.

[braver]

@tomv564 I'm not sure. The \r\n\r\n's I was getting weren't being converted to paragraphs ("before"), \n\n does work as expected (note the extra space between paragraphs in "after"). So there does seem to be a need to clean it up before passing the content to mdpopups. I'm not sure what the other cases in the preserve_whitespace function are for, I don't think I encountered those cases.

[braver]

So..

contents = contents.strip(' \t\r\n')

I'm not sure what this accomplished. Trailing whitespace like this should be handled by the markdown conversion.

contents.replace('\r\n', '\n')

I'm not sure what the markdown engine expects or if it can be configured to deal with \r\n, but right now this fixes the content so that conversion to paragraphs actually works.

contents.replace('\t', '\u00A0' * 4)
contents.replace('  ', '\u00A0' * 2)

These mirror what mdopups does to preserve whitespace in pre elements (minihtml doesn't actually support pre). So, in code blocks we don't have to do this (therefore code blocks aren't passed through preserve_whitespace). Markdown should ignore and collapse whitespace like this. However, some of the content provided by servers isn't really markdown, so I think these two lines are here to attempt to preserve the layout of the original content.

Concluding:

1. I don't know what this accomplishes and it looks like it could actually backfire
2. This seems necessary for the markdown conversion to work properly
3. I don't think we should need to do these

[deathaxe]

1

contents = contents.strip(' \t\r\n')

As \n\n was replaced by \n \n multiple line breaks at the end of comments resulted in empty lines being displayed at the bottom of of a popup.

As this replacement is now replaced by proper paragraph spacing it should be obsolete.

2

Following @facelessuser's suggestion from the last PR the following snippet shows markdown to correctly convert paragraphs no matter which line ending was provided.

>>> mdpopups.md2html(view, markup="hello\r\nline2")
'<p>hello<br />line2</p>'

>>> mdpopups.md2html(view, markup="hello\r\n\r\nline2")
'<p>hello</p><p>line2</p>'

3 Conclusion

So I guess contents.replace('\r\n', '\n') is not needed, too.

The only lines required to ensure correct indention of ascii docstrings are

contents.replace('\t', '\u00A0' * 4)
contents.replace('  ', '\u00A0' * 2)

[braver]

Indeed, I can now remove most of the processing and it still looks good. 🤷‍♂️

The only lines required to ensure correct indention of ascii docstrings are

(emphasis mine)

I guess as the servers grow more mature they'll move away from ascii to markdown and eventually we will be able to remove all the processing?

[deathaxe]

Not at the moment.

pyls as an example forwards docstrings from jedi as is without any markdown conversion. Therefore removing whitespace replacement would break indention of all python docstrings. Don't see any sights for this behavior to change and I am even not sure whether it is always possible. We could file an issue at pyls to ask for changes, but ... ?

[braver]

We could file an issue at pyls to ask for changes

Well that's always good I guess. For now just keep an eye on the thing and make sure no extra cruft is added? I updated to comments to be clearer about why it's needed.

[tomv564]

Thanks for the rebase and the explanation of preserve_whitespace!

[deathaxe]

What a step backwards in whitespace handling 👎 Is it hurting to keep fixed white spaces soo much?