Whitespace in popups could be better

[braver]

Perhaps this is a bit too minor/visual for this stage of the package, but I'm a designer and it's bugging me 😁

Text sits too tight too the edge of the popup. At the same time there is an inexplicably large amount of space between the code fragment at the top and the documentation below. It looks worse if there isn't much documentation (e.g. it just says "function").

It would look better if the text aligns and margins are consistent all around.

I also noticed the phantoms are missing the little pointer nubbin that the build system phantoms have. This seems to be a function of using the mdpopups library. Are these things we can/should tune here, or should I try to land some enhancements in mdpopups?

[tomv564]

Agree absolutely, would love for a PR to fix the left margin and paragraph / code block spacing.
The HTML output may need to change as well?
Perhaps can also apply the sans-serif font ( issue #82 )?

Fixing the pointer on the phantoms is probably a question of taking the right bits of the large style sheet here: https://github.com/twolfson/sublime-files/blob/master/Packages/Default/exec.py#L395

[braver]

If @rmccue isn't already upto something with this, I could look into this some time next week.

[rwols]

There's also the issue that we can't browse through other overloads like in VSCode.

[braver]

@rwols could you point me to a demonstration of that?

[rwols]

I'm working on that right now, here's a prototype screenshot:

I just dump all overloads to the popup box, but ideally, the "active" overload should be displayed, and the up and down arrow keys allow you to browse through the other overloads. The active parameter should be bold. This is how it's done in intelliSense in Visual Studio.

Anyway, this is somewhat of a separate issue but I thought I'd let you know of what I'm up to so you can keep it in mind for when you might change the layout.

[braver]

@tomv564 How do you feel about the mdpopups dependency? I think if you want to do stuff like @rwols proposes it might not be as flexible as you'd like. I'd prefer having direct access to the HTML that comes out, but then again, I manipulate HTML and CSS all day every day and I can see how someone might not enjoy that 😉 What I'd like to attempt is to set up some basic elements we can re-use and tune it to the purposes at hand. It's not like it will take that much to style up a basic popup.

[rwols]

My implementation actually doesn't use mdpopups but plain-old HTML+CSS, please wait for the PR!

[braver]

Well, there you go :)

[tomv564]

Agreed, if we're building UI then using markdown is not ideal. But mdpopups supports plain HTML and also provides us with syntax highlighting and user overrides. Are we really discussing replacing all that with code maintained in LSP?

[rwols]

The syntax highlighting from mdpopups should definitely be reused, that's super useful.

[braver]

The user overrides are out-of-the-box from Sublime itself, it's no extra effort for us. Edit: not sure what mdpopups offers here, but syntax themes have full control over styling, and IIRC users can override that. Not sure why anyone would want to, but it's there.

The syntax highlight would be cool and useful. It doesn't necessarily work though if you're working with super small snippets.

[rwols]

Related: #111

[deathaxe]

but syntax themes have full control over styling

mdpopups provides the same kind of flexibility and access to all html/css elements as well.

It's not about styling the mdpopups but syntax highlighting. mdpopups provide a <div class="highlight"> which you can put simple source code into. mdpopups will syntax highlight it the same way the editor does. I find this feature very useful and would like to see the code signatures of all the popups to be highlighted correctly.

LSP does not yet make use of the features mdpopups offers. You don't need to put pure text to the popup to let its markdown engine render it. You can also put normal html code with your own css. This what GitGutter uses for diff popup.

With mdpopups.syntax_highlight(view, source_content, **popup_kwargs) it can be used to highlight a piece of source code and re-use it in ordinary html. By using mdpopups you can ensure the basic elements are styled correctly using its default.css.

But on the other hand I find the syntax style being used by PackageDev to display information about the settings, well enough too. One headline (the signature), one line with a brief description followed by the docstring and maybe the list of arguments. I originally created that popup style for my CNC Sinumerik 840D package to display variable/function tooltips. Same use-case as for LSP.

The only missing thing what I'd like to see LSP to do is highlighting the active argument in the signature, if the cursor is within the argument list.

like def function( arg1, arg2, arg3)

[deathaxe]

Are we really discussing replacing all that with code maintained in LSP?

With or without mdpopups. I think popup rendering needs a separate package as - i don't know in which format the different language servers provide their docstrings - each language might need a little different handling some day to provide a good experience. I already discussed something like that in the Jedi - Autocompletion package. Even in python, you have different styles of docstring. RST, markdown, google, numpy, ... All of them need different parsing to create a commonly styled output. I guess it even becomes worth with other languages. Therefore a kind of plugin infrastructure to handle different languages might be required.

language-server -> response -> popup render plugin -> output

where popup render plugin is an abstract interface for the real rendering plugin provided for a certain language/syntax. Maybe with some kind of format guessing - some day.

That's not so easy.

[tomv564]

To summarise the original issues and the new issues discussed in this thread

• Hover: Left margins inconsistent between code blocks and paragraphs: PR NEEDED
• Hover: Vertical spacing is inconsistent: Enhancement: Keep whitespace displayed in popups #112 by @deathaxe
• Hover: Vertical spacing inconsistent with editor line spacing: (really hard?)
• Diagnostic: Phantom pointer nubbin missing: DONE by @braver 🎉
• Signature Help: Browsing through overloads: Browse function overloads with the up/down arrow keys #111 by @rwols
• Signature Help: Highlight active argument: Browse function overloads with the up/down arrow keys #111 by @rwols

If I understand your suggestions in this thread, then we can more or less agree on standardising popups with:

• Structure & Layout: HTML & CSS (through mdpopups if the below cases apply)
• Code blocks: Use mdpopups (<div> or highlight call if necessary)
• Server-provided documentation fields: Use mdpopups to render markdown/html.

Issues that this package probably shouldn't fix:

• Other docstring formats: The protocol dictates only Markdown is allowed.
• Standardising content in hover popups (there is no guaranteed structure in Hover.contents, language servers could be sending the function name after the documentation if they wanted to)

[deathaxe]

Hover: Vertical spacing inconsistent with editor line spacing

Doesn't really matter in my opinion.

protocol dictates only Markdown

This is good to know, but the issue is markdown can be nearly everything. But this enables us to file an issue at python-language-server to create correct markdown for each kind of docstring. 😆 😈

The rest is 👍