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

Use nvim.async_call in non-main threads #26

Merged
merged 1 commit into from
Nov 6, 2018

Conversation

wookayin
Copy link
Contributor

@wookayin wookayin commented Nov 6, 2018

Due to a change from neovim python-client 0.3.0, it is now required
that nvim API calls from other threads must use nvim.async_call().

This commit wraps all such API calls from the background thread
(via self._wrap_async) to make use of nvim.async_call().

This PR attempts to fix #25.
Tested with neovim python-client 0.2.6 and 0.3.0.

Due to a change from neovim python-client 0.3.0, it is now required
that nvim API calls from other threads must use nvim.async_call().

This commit wraps all such API calls from the background thread
(via self._wrap_async) to make use of nvim.async_call().

See #25 for details.
@wookayin wookayin changed the title Use nvim.async_call in non-main threads (#25) Use nvim.async_call in non-main threads Nov 6, 2018
@wookayin
Copy link
Contributor Author

wookayin commented Nov 6, 2018

This is an initial attempt to fix #25. Please feel free to poke me if you see anything needs update. Some thoughts:

  • Maybe proper unit test is required? (not sure)
  • Should we prevent creating wrapper function instance every time?

UPD: Test is failing, will look into it.

@numirias
Copy link
Owner

numirias commented Nov 6, 2018

Thanks for working on that!

Regarding the test failure: There are some race conditions in plugin integration tests (the ones that run a headless neovim instance) which I didn't find a clean fix for yet. The failing test you get may be one of them and you can probably safely ignore it.

Unfortunately I won't be able to verify the patch myself until tomorrow. But since the current issue makes Semshi unusable with 0.3.0, I'd prefer to merge ASAP. So let me know if you're confident it's working as intended and I'm happy to merge.

@wookayin
Copy link
Contributor Author

wookayin commented Nov 6, 2018

I can confidently confirm that this should work with combination of py-neovim {0.2.6, 0.3.0} and NVIM {0.2.3, 0.3.1}, but whether to merge or wait a bit more is up to you.

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.

Broken with neovim python-client 0.3.0: request from non-main thread
2 participants