[RDY] Neovim Python & Synchronous +PlugInstall #227
Conversation
|
Thank you. My suggestion is that we don't explicitly say anything about this change since it's a temporary workaround to a known issue. A subset of Neovim users who have python support enabled will benefit from this change, while the others will still find that vim-plug doesn't work like Vundle but they'll notice that there is an open issue. It's clearly a better situation than now anyway. What is tricky is that there's no guarantee that |
| echomsg 'vim-plug: pip(3) install neovim' | ||
| echohl None | ||
| endif | ||
| endif |
There was a problem hiding this comment.
vim +PlugUpdate will cause any subsequent install/update to become synchronous. I'd prefer to remove s: variables like I did in #104 (comment).
There was a problem hiding this comment.
@junegunn Right you are, will patch that once I've made the fix for the seg fault. I think it was just a matter of locking. Might have over zealously removed some.
|
@junegunn When you say don't explicitly say anything do you mean also remove my warning? Or I'll leave it? If you ask me, most users of +PlugInstall/Update will want synchronous behaviour because most use it as part of script/docker etc... |
| \ map(split(split(pyv)[0], '\.'), 'str2nr(v:val)'), [2, 6]) | ||
| endif | ||
| if (s:py || s:ruby) && !s:nvim && s:update.threads > 1 | ||
|
|
||
| if (python || ruby) && (!s:nvim || (has('nvim') && has('starting'))) && s:update.threads > 1 |
There was a problem hiding this comment.
Let's move the second condition to line 747, the definition of python. It doesn't apply to ruby.
I meant removing warning as well, but either is fine. Your call.
True, but we have to assume that there can be exceptional use cases. |
|
Hmmm, found new small bug. On neovim only, update gets interrupted by ultisnips: Error detected while processing function UltiSnips#CursorMoved..provider#python#Call:
line 12:
Invalid channel "1" |
|
@junegunn Think I've gotten all the problems. Currently no warning printed to users, simply uses python installer silently. Limitations still existing:
|
| @@ -747,6 +744,10 @@ function! s:update_impl(pull, force, args) abort | |||
| echohl None | |||
| endif | |||
|
|
|||
| let python = (has('python') || has('python3')) | |||
| \ && (!s:nvim || (has('nvim') && has('vim_starting'))) && !s:is_win && !has('win32unix') | |||
| let ruby = has('ruby') && !has('nvim') && (v:version >= 703 || v:version == 702 && has('patch374')) | |||
There was a problem hiding this comment.
How about we check has('nvim') only once when defining s:nvim and see s:nvim instead in other places? I think we can reorganize the expressions as follows to make them slightly less confusing
let python = (has('python') || has('python3')) && !s:is_win && !has('win32unix')
\ && (!s:nvim || has('vim_starting'))
let ruby = has('ruby') && (v:version >= 703 || v:version == 702 && has('patch374'))
\ && !s:nvim- has python & not windows & (neovim installer n/a or vim is starting)
- We can just strip out vim_starting part when jobwait is ready
- has ruby & vim version not too old & neovim installer n/a
There was a problem hiding this comment.
@junegunn Agreed. Less messy now.
|
I commented on the code. Just some minor nitpicking. Other than those, looks great. You can squash the commits so I can merge. Thanks! |
* Buffer updates now managed by __main__ loop with buf_q. * Synchronous neovim install temporarily provided by python installer. * Known issues: * No ctrl-c/interrupt support on nvim. * Graphical bug: neovim/pynvim#103
|
@junegunn Changes made, overall a nice improvement. Hope nvim users notice. |
starcraftman
changed the title
|
@starcraftman Looks like travis build is getting delayed. I'll merge when it's done :) |
Use Python installer on Neovim during `vim_starting` (#104)
|
Merged, many thanks as always! |
__main__loop with buf_q. Entirely for neovim support.Known issues:
Travis Python 3 build segfaults.@junegunn Was interested in this to respond to #104 . This PR is work in progress still need to fix segfault. Open to comments.
Open Questions: