neovim / pynvim

Python client and plugin host for Nvim
http://pynvim.readthedocs.io/en/latest/
Apache License 2.0
1.53k stars 118 forks source link

Cannot execute a vim command from a new asyncio task #342

Open tbodt opened 6 years ago

tbodt commented 6 years ago

To reproduce: Start Neovim with NVIM_PYTHON_LOG_FILE set, and run these commands:

py3 async def foo(): vim.out_write('testing')
py3 import asyncio, vim; asyncio.ensure_future(foo())

Then look in the python log file (_script suffix) and see:

2018-06-30 20:20:29,362 [ERROR @ base_events.py:default_exception_handler:1266] 31555 - Task exception was never retrieved
future: <Task finished coro=<foo() done, defined at <string>:1> exception=AttributeError("'NoneType' object has no attribute 'switch'",)>
Traceback (most recent call last):
  File "<string>", line 1, in foo
  File "/usr/local/lib/python3.6/site-packages/neovim/api/nvim.py", line 378, in out_write
    return self.request('nvim_out_write', msg, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/neovim/api/nvim.py", line 170, in request
    res = self._session.request(name, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/neovim/msgpack_rpc/session.py", line 91, in request
    v = self._yielding_request(method, args)
  File "/usr/local/lib/python3.6/site-packages/neovim/msgpack_rpc/session.py", line 160, in _yielding_request
    return parent.switch()
AttributeError: 'NoneType' object has no attribute 'switch'
tbodt commented 6 years ago

I forgot to type a description before creating the issue, so I went back and edited the description after creating it.

blueyed commented 6 years ago

Just tried it, and it works for me using Python 3.6.6 and pynvim master. It needs \n to actually write it though.

Also vim.command("echom 1") worked.

blueyed commented 6 years ago

However, only the first command gets run. With the following only 1 will be displayed:

:py3 async def foo(): vim.command('echom 1'); vim.command('echom 2')
:py3 import asyncio; asyncio.ensure_future(foo())
blueyed commented 6 years ago

Yeah, your issue after all.. - so confirming ;)

blueyed commented 6 years ago

https://github.com/neovim/python-client/blob/fbf0c843284ee33eca2dc4d62aec178809b0463d/neovim/msgpack_rpc/session.py#L63-L69

blueyed commented 6 years ago

Using async_=True works around this.

blueyed commented 6 years ago

Is that something that nvim.request should handle? Or should/could there be a decorator or context manager that you could then use with your async def function?

bfredl commented 6 years ago

nvim-managed greenlets and asyncio coroutines are like water and oil, they are incompatible event/async primitives that simply don't mix: any attempt to "seamlessly" integrate them might fail in the first request stack/event handing situation that wasn't explicitly considered.

What we could add is (1) a greenlet-callable function, that runs an asyncio coro/future until completion and then resumes the greenlet and (2) an awaitable function that suspends a coroutine and runs a closure as a greenlet until it completes and then resumes the coroutine. Though this will more or less be callback hell and thus have none of the benefits of either the stackful greenlet approach or the explicitly yieldable approach of asyncio coroutines.

Rather what I would imagine useful is to reimplement the python client using only asyncio and native coroutines and no greenlets, I suspect that will be a lot simpler than the exisiting multi-layer client: it will more or less be a library providing a nvim asyncio protocol for its native transports, and request awaitables for coroutines which asyncio otherwise manages by itself.

tbodt commented 6 years ago

I would really love to have all these strange event loops dropped and replaced with asyncio/trollius, though wouldn't there be a backwards compatibility problem?

bfredl commented 6 years ago

though wouldn't there be a backwards compatibility problem?

Yes. It would need be a separate project from this. I wouldn't even think "fork", rather new codebase which can happen to borrow code from this where it makes sense. Starting from scratch with no regards of vim compatibility (which necessitates a sync-like interface backed by greenlets or similar) would give a lot more freedom for both simplicity and expressivity. For instance call_atomic could be exposed as a function converting multiple request awaitables to a single awaitable, while with this codebase to use call_atomic you must throw all existing abstractions above the raw API out through the window.

Shougo commented 5 years ago

I need the fix for denite.nvim split processes feature.

https://github.com/Shougo/denite.nvim/issues/509