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

Add option to condense completion menu items (would address #581) #582

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

berggeist
Copy link
Contributor

This will add a new option condensedCompletionMenu, which allows to condense (or minimize) the text (width) of the completion menu items:
The default behavior for the completion menu items does not only display the word that will be completed but also additional details, like completion kind and detailed information, provided by the language LSP server. Typically, especially for typed languages for functions their call parameters with all their types are included. This makes the display of the completion menu items surprisingly overwhelming 😄
When this option new condensedCompletionMenu is enabled only the (key-) word and its kind is part of the completion menu item. The additional provided details from the LSP server are moved to the info popup, which is shown as soon as a completion item is selected.
This enhancement would address the issue:
Improve complete-popup on vertical screens #581 #581 (comment)

Here is a screenshot of the default completion menu:
2024-12-29_condensedComplMenu-false

and here if this new option has been enabled:
2024-12-29_condensedComplMenu-true png

Instead of sprinkling the necessary modifications all over the source code, I decided to collect them in one place, which maybe would be less efficient but would improve the readability of this enhancement.

Caveat: For completion LazyDoc the detailed information which is moved to the info popup will be automatically overwritten with the lazily delivered documentation from the LSP server.

#581

@Shane-XB-Qian
Copy link
Contributor

i like your idea, but:

  1. your example for golang seems good, but clangd its content in 'menu' seems was 'return type', so moving that to info popup seems odd.
  2. to lazy doc servers, how reliable you made it work?

@berggeist
Copy link
Contributor Author

berggeist commented Dec 30, 2024

Hi & thank you for your review!

Why did I make the changes like I did?
First, I wanted to keep the changes as simple as possible in order to not damage existing source code.
Then, I wanted to make the completion menu item, as narrow as possible, in order to give much more display area to the info popup.

How does Vim display the completion items:
Each completion item, handed over to Vim, has this attributes:

  • word (which contains typically only the short e.g. function name)
  • abbr (which is typically more more verbose and contains e.g. the complete function signature)
  • kind (typically just one character)
  • menu (which is handled very differently in each LSP server, e.g. clangd returns the return type of a function, whereas gopls returns the function signature, without the function name. Other LSP servers might leave this attribute even empty)
  • info (some more documentation)

Vim concatenates the following attributes into the completion menu and displays the info attribute in the info-popup which is displayed when an item gets selected:

   +--- "word | abbr"
   |                 +--- "kind" 
   |                 |  +--- "menu"
   |                 |  |
   v                 v  v

  aaaaaaaaaaaaaaaaaa k mmmmmmmmmmmmmmmmmm

Unfortunately different LSP servers provide you with different data in their completion responses.

Therefore I decided to

  • take the smaller text of the word or the abbrev, which is typically more WYSIWYG (what you see is what you get).
    In this case the abbrev will be moved to the info-popup.
  • leave the kind untouched, as it is only one character short.
  • move the menu to the info popup.

Let's look into an example using the clangd LSP server:

Here is the completion menu with the new option condensedCompletionMenu disabled:
2024-12-30_clangd_condensedComplMenu-false

and here with the new option condensedCompletionMenu enabled:
2024-12-30_clangd_condensedComplMenu-true-2

We can even compare the look of the completion menu to the default hover documentation popup:
2024-12-30_clangd_hover-doc

As you can see the completion menu with the new option condensedCompletionMenu enabled resembles much more the look of the hover documentation popup. Especially the type of the return value. Unfortunately the clangd LSP server does not (yet) provide the structured parameter info within its completion responses (nor does it provide the doxygen doc-strings).

@Shane-XB-Qian
Copy link
Contributor

Shane-XB-Qian commented Dec 30, 2024 via email

@berggeist
Copy link
Contributor Author

i knew your idea, but my concern:

Let's address your concerns:

  1. d.word maybe empty, so you compared abbr's len with word's len, maybe wrong?

d.word must not be empty at the location, where my pull request applies, otherwise the existing code before my patch is doing something wrong.

  1. something cfg e.g 'filterCompletionDuplicates' if on, i am worried you changing those value made the result not reflect the truth.

This was also a concern I worried myself. At the end, all I'm doing, is replacing the attribute d.abbr by d.word and d.word is exactly what gets inserted into your code after you accept the completion item. So far with my pull request, you get far more WYSIWG (what you see is what you get) as before. E.g. before applying my pull request a LSP server would provide "zzTop" in the attribute d.word and "Bee Gees" in the attribute d.abbr with the consequence that "Bee Gees" is displayed in the completion menu whereas "zzTop" get inserted into your code. After applying my pull request "zzTop" is displayed in the completion menu and will be inserted into your code.

  1. still about lazy doc servers, how you deal with them?

I kept the handling of lazy doc servers as is. That means, if I recognize in line 328

 if !lspserver.completionLazyDoc && !empty(infoText)

that the LSP server is providing its doc lazily, I do not touch the info-attribute at all. If it worked well up to now than it will work as well after applying my pull request. With the consequence that if e.g. the attribute d.menu is being emptied before it will not be shown at all, as it is not moved to the info-pop where it would be overwritten by the lazily supplied doc anyway. Most LSP servers I tried out do show the data provided in d.menu also in the (lazily provided) info doc.
But if you are worrying about you could just disable this option anyway. Which is the default, i.e. it is per default disabled in order to preserve the existing behavior.

@Shane-XB-Qian
Copy link
Contributor

Shane-XB-Qian commented Dec 30, 2024 via email

@berggeist
Copy link
Contributor Author

berggeist commented Dec 31, 2024

well, who knows, at least, if d.word is empty, then d.abbr should not get from it.

See also my remark from above:
No ,d.word cannot be empty. This is ensured by the specification of the language server protocol that mandates that at least CompletionItem.label must be not empty. And the fallback (which might be in my eyes not straightforward regarding the handling of fuzzy-matching, see also #584 ) of this code (beginning with line 197) ensures this:

    if item->has_key('textEdit') &&
	lspOpts.completionMatcherValue != opt.COMPLETIONMATCHER_FUZZY
...
    elseif item->has_key('insertText')
...
    else
      d.word = item.label
    endif

but you changed d.menu to '' empty too, not only switched d.word and d.abbr

Intentionally my modifications are inserted after the deduplication. Why: Because for e.g. the java language server jdtls provides in its completion list also all overloaded methods and I did not want to remove them. As for now you might see for overloaded methods multiple entries with the same d.word but as soon as you select any of them the info-popup will reveal their full method signature, so no information gets lost.

maybe let !lspserver.completionLazyDoc helped the d.word and d.abbr too, not only d.menu?

The worst thing that could happen is that the lazily provided documentation by the LSP server will override the moved information, i.e. d.abbr and d.menu, but this cannot be fixed anyhow. If someone don't want this behavior, he just has to leave this option to its default value, i.e. disabled.

gcanat added a commit to gcanat/lsp that referenced this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants