search

wellle / tmux-complete.vim

Vim plugin for insert mode completion of words in adjacent tmux panes
MIT License
515 stars 21 forks source link

System call should be done in the python process to avoid blocking Neovim #73

Closed thalesmello closed 7 years ago

thalesmello commented 7 years ago

This PR replaces the Vim system call for one directly in Python. This is important because vim#call end up blocking vim instance. It becomes a problem if the person has a higher limit of words to be used during completion, as Vim starts to freeze while typing. This pull request fixes that.

wellle commented 7 years ago

Thanks! I'll try to check it out soon.

thalesmello commented 7 years ago

I will ask you to wait a little bit more to test this pull request. I suspect my fork kills the deoplete process after a while. I still have no idea why this is happening. I will give you a heads up after dome investigation.

thalesmello commented 7 years ago

@wellle So, the culprit was apperently a print statement that I had in my code. I've been using my modified branch for the past few days and the problem didn't happen anymore.

Would you be kind to test it as well and give me a heads up in case you find any issue?

justinmk commented 7 years ago

Printing to stdout will manifest as a bad message in the stdout/stdin RPC channel (if a socket wasn't used). What kind of error did you see in that case? We may need to make such malformed messages more obvious (by reporting an error in nvim or something).

thalesmello commented 7 years ago

@justinmk Take a look at this thread, particularly my comment near the bottom of the thread.

https://github.com/Shougo/deoplete.nvim/issues/353

@Shougo helped me figure out the problem was tmux-complete, particularly my fork which had the print statement.

Shougo commented 7 years ago

@Shougo helped me figure out the problem was tmux-complete, particularly my fork which had the print statement.

:-)

thalesmello commented 7 years ago

@wellle any position on this pull request?

wellle commented 7 years ago

I'll try to have a look at it soon. Thank you!

wellle commented 7 years ago

@thalesmello: I just tried it and it is fantastic! 👏

Sorry for taking so long on this. The diff looks good and the changes make sense. And the result is just so much more usable after the changes. I'll definitely keep using that combo now, thank you for your work on this!

One tiny thing: Would you be so kind and rebase your commits on top of master? I'll merge it immediately afterwards!

@Shougo @justinmk @blueyed: Thank you too!

blueyed commented 7 years ago

@wellle May I suggest to just use GitHub's squash-merge UI, and edit the commit message to remove the second one ("Remove print statement"), i.e. emptying the commit's body?

thalesmello commented 7 years ago

@wellle There you go

wellle commented 7 years ago

@thalesmello: Great, thanks again!

@blueyed: Interesting, I'll keep that in mind for the future 👍