search

davidhalter / jedi-vim

Using the jedi autocompletion library for VIM.
MIT License
5.27k stars 370 forks source link

Don't check for preview in completeopt for adding doc to completion #1001

Closed novadev94 closed 4 years ago

novadev94 commented 4 years ago

As title. It's extremely helpful for users who want to use popup in Vim or floating window via some plugin (like https://github.com/ncm2/float-preview.nvim) in Neovim.

davidhalter commented 4 years ago

@blueyed What do you think?

I remember this being mostly a performance change (to avoid loading all the docstrings, which sometimes involves a lot of type inference/opened files).

This is still kind of a performance issue. It has become quite a bit better in Jedi 0.16.0 for the big libraries (numpy, scipy, pandas, matplotlib, tensorflow), but only after the first time using it. The first time might take up to 15s! So I'm a bit conflicted about this :/. (That said I get why @NovaDev94 wants to change it).

blueyed commented 4 years ago

@NovaDev94 I've not checked https://github.com/ncm2/float-preview.nvim, but couldn't it set preview temporarily?

Otherwise having a (more universal) option for this might be feasible, but would either requiring some auto-detection, or manual setup.

I for myself really do not like having all the overhead when it gets not used then anyway.

blueyed commented 4 years ago

I assume this still uses the setlocal omnifunc=jedi#completions? We could have a wrapper there that would add "info" always, e.g. jedi#completions_with_info.

davidhalter commented 4 years ago

I'm definitely against additional complexity. If you're against execution if it's not used we should just close. The amount of people that are going to use this is probably 1. So either we merge or close.

novadev94 commented 4 years ago

@blueyed I'm not sure if it's possible on the float-preview side, it works by hooking into CompleteChanged and display info from v:event.completed_item. And since jedi-vim didn't populate that data, there's nothing to show.

@davidhalter I wasn't fully aware of the performance degradation since it was completely fine on my machine. But as you said, it negatively affects many users and only a small amount of users can benefit from it - a niche use case. So I think it's fair to call it a "close", I can find other alternatives (like deoplete-jedi) or maintain a fork instead.

blueyed commented 4 years ago

I think it would be ok for completions to have a kwarg with_info, so that you could have a custom omnifunc yourself. It would default to None, using the current method then, but could also be set to True/False explicitly.