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

Improve documentation popup in neovim. Fix typo. #1017

Merged
merged 2 commits into from
Jan 9, 2021
Merged

Improve documentation popup in neovim. Fix typo. #1017

merged 2 commits into from
Jan 9, 2021

Conversation

subnut
Copy link
Contributor

@subnut subnut commented Jan 4, 2021

No description provided.

@prabirshrestha
Copy link
Owner

Can you include before/after screenshots? I will test it during the next few days.

subnut added 2 commits January 5, 2021 10:50
If the available space in either side of the completion popup is less
than 3/4 of the required width, then we shall place the documentation
popup below (or above, if necessary) of the completion popup.

Previously, the documentation was just squashed into whatever space was
available. It made documentation undreadable in some cases.
@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

Before

gifcast_210105110507

After

gifcast_210105110609

@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

Extreme case

Before

gifcast_210105105343

After

gifcast_210105110821

@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

Explanation

gifcast_210105110208

@prabirshrestha
Copy link
Owner

If you have 20 items where would the poppup show?

Another idea I have thought was to remove the snippet so that it behaves similar to VsCode.

image

In your case it would look like this.

import        [k]
ImportError   [k]
ImportWarning [k]
print         [f]

Since the pumwidth is now small the left side of it can be used for documentation instead.

The only issue would is how it would look for languages that have function overloads but I'm hoping documentation would still solve this issue.

Your PR is still definitely better experience.

@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

If you have 20 items where would the poppup show?

Here's how the positioning is preferred -

  1. Right of pum
  2. Left of pum
  3. Bottom of pum
  4. Top of pum

Another idea I have thought was to remove the snippet so that it behaves similar to VsCode

This is what I actually wanted! It would be great if this could be implemented soon! I would absolutely love it 💟
BTW, there should be an option of enabling it back, right? So, if someone wanted, he/she could bring back the snippet

@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

In your case it would look like this.

import        [k]
ImportError   [k]
ImportWarning [k]
print         [f]

I would like it to be a little more verbose -

import        [pyls] keyword
ImportError   [pyls] keyword
ImportWarning [pyls] keyword
print         [pyls] function

@subnut
Copy link
Contributor Author

subnut commented Jan 5, 2021

I noticed something -

With palantir/python-language-server

2021-01-05_11-56

With pappasam/jedi-language-server

2021-01-05_11-57

I am confused....

Why different things in different servers??

@prabirshrestha
Copy link
Owner

It really depends on language servers.

autocomplete and popup are the most fragile one so it is quite risky to change. currently working on other items so won't happen soon.
when i do work on it i do want to simply popup and autocomplete. this one requires a lot more time so just need to find a longer dedicated time to hack on it.

vital.vim popup is ready to be merged in vim-lsp. vim-jp/vital.vim#748 Might be that is the first thing we want to add. and then similar to #1019 have let g:lsp_experimental_popup_ui = 1 so that we can start reworking on it. Since the flag will be disabled by default no one will see it and we are done we can remove the flag make it the default behavior.

I was hoping some of these gets fixed vim/vim#7555 vim/vim#7553 before I invest more in vim popups/neovim floats.

For example I want to add the bulb support in vim-lsp.
image

@prabirshrestha prabirshrestha merged commit 03ca21d into prabirshrestha:master Jan 9, 2021
@prabirshrestha
Copy link
Owner

Merged. Thanks.

@subnut would you be interested in porting popup from vital.vim to vim-lsp? vim-jp/vital.vim#748

Currently I'm working on getting other areas in vim-lsp so I won't be able to work on it anytime soon.

@subnut subnut deleted the subnut branch January 10, 2021 02:54
@subnut
Copy link
Contributor Author

subnut commented Jan 10, 2021

@subnut would you be interested in porting popup from vital.vim to vim-lsp? vim-jp/vital.vim#748

I am sorry I can't... I have exams coming up, and I am going to be very busy until the end of June.


Off topic: Are you of Indian origin?

@prabirshrestha
Copy link
Owner

@subnut Feel free to try #1038 and provide thoughts. It should be lot faster and I removed some of the info from popup.

I did notice a small bug. I'm currently typing std at line number 8 which means the popup should never overflow on that particular line.

when popup shows at the top.
image

when popup shows at the bottom
image

when popup shows at the top right. notice i simulated long line by adding /* ************** */ comments but only /* is visible.
image

when popup shows at bottom right.
image

Currently neovim experience does seem better than vim in-terms of overlap as vim overlaps the complete menu and we can't see all the visible complete items. For now it would be better to fix the neovim only experience and in the future when we do move to a generic popup we can port neovim's logic instead of vim's popup logic.

image

Feel free to send fixes after exam and best of luck for your exams!

I'm originally from Nepal.

@prabirshrestha
Copy link
Owner

@subnut Can you please try this branch. #1052. It is rewrite of the documentation popup and which unifies vim and neovim popups. Want to make sure we don't regress once we check it in.

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