TypeError when inheriting from sublime_aio.ViewCommand

[rwols]

I'm experimenting with sublime_aio.ViewCommand.

But it looks like existing, old command classes that take an edit object as the first argument are failing to run. For instance:

Traceback (most recent call last):
  File "/home/raoul/.config/sublime-text-3/Lib/python314/sublime_aio.py", line 493, in run_
    call_coroutine(self.run(**args) if args else self.run())
                   ~~~~~~~~^^^^^^^^
TypeError: LspHoverCommand.run() missing 1 required positional argument: 'edit'

The signature of LspHoverCommand.run is:

    def run(
        self,
        edit: sublime.Edit,
        only_diagnostics: bool = False,
        point: int | None = None,
        event: dict | None = None
    ) -> None:
        ...

Could the ViewCommand class ensure that old command classes remain compatible?

[rwols]

In the linked PR, sublimelsp/LSP#3004, you can try it out for yourself. Hover over a symbol and see the TypeError occur. I'm not quite sure whether the problem is really the edit parameter or not.

[deathaxe]

sublime_aio/src/sublime_aio.py

Lines 478 to 479 in 30782ef

	It is like a `TextCommand` but doesn't provide an `edit` token, because
	it wouldn't be valid anymore, when `async def run()` is invoked.

It doesn't provide support for an edit token on purpose to not create the illusion it would work.

Therefore this is called ViewCommand but not TextCommand.

[rwols]

Ok. But then there is no backwards-compatible upgrade path possible. Or is there?

[deathaxe]

There is no backward compatibility as there is no asyncio compliant TextCommand. Such would require core API to support asyncio.

[rwols]

I agree that async def run() should not receive the edit token as the first argument, as that edit token is no longer valid when the coroutine body runs.

And I can understand why the blocking version def run() would also not get an edit token, for API symmetry.

But not providing the edit token in the def run() variant really does break backwards-compatibility for packages that depend on some base class command that starts inheriting from ViewCommand.

There are quite a bunch of LSP-* helper packages that inherit from a base LspTextCommand, so it cannot inherit from sublime_aio.ViewCommand as it then breaks an old contract.

Would it really be so bad to pass the edit token in the old def run() variant? New code is going to use the async version straight away, anyway. Old code would remain compatible.

[deathaxe]

ViewCommand and friends don't define a synchronous def run() method. They override the one from sublime_plugin.TextCommand. Any such method in an inherit command would either be ignored or more likely cause call_coroutine to throw an exception. They can't therefore "continue" to use or provide synchronous run().

A switch to sublime_aio.Command is therefore always a breaking change, requiring all consumers to switch to async def run().

This step can/must then include removal of passing/expecting an edit token.

EDIT: An synchronous run being executed is a bug, caused by how coroutine is constructed before calling it.

sublime_aio explicitly wants to enforce usage of async def coroutines and NOT mix up with legacy ways of scheduling ordinary synchronous functions in event loop.

The dogma is to use async/await exclusively.

[rwols]

A switch to sublime_aio.Command is therefore always a breaking change, requiring all consumers to switch to async def run().

Can't it use inspect.iscoroutinefunction to check whether self.run is a coroutine function or not?

If it's not, then just invoke self.run with the edit token. If it is, run it with call_coroutine... ?

NOT mix up with legacy ways of scheduling ordinary synchronous functions in event loop.

I agree. The regular run(), if defined, can just be invoked in the main thread. I don't get the problem.

I want to use sublime_aio.Window / sublime_aio.View, but since synchronous versions of run() are "banned", it's now hard to utilize them.

[deathaxe]

This is explicitly not desired design. Not supporting it is a design decision for sanity reasons.