Make command classes backwards compatible with existing class hierarchies

[rwols]

This is both a PR but also a proposal to have the functionality as I mentioned in this comment. It makes the changes in sublimelsp/LSP#3004 work. Meaning, it allows existing packages to inherit from sublime_aio.ViewCommand safely.

If the run method is not a coroutine function, it is simply invoked as-is, and in the case of a view command the edit_token is passed as first argument as well.

[deathaxe]

I am aware of this being possible, but as mentioned in the other PR this is not supported by intent.

Synchronous code is to use synchronous functions and async code is to use async def.

[deathaxe]

Anything else ends in unpredictable chaos.

[FichteFoll]

Imo LSP should create a new LspViewCommand class that subclasses sublime_aio.ViewCommand and is provided as the "new" API in a non-breaking way. I agree with deathaxe here that adding synchronous fallback behavior for sublime_aio is not desirable.

If you really want to not change your interface for Lsp plugins, you should create your own wrapper class with the desired functionality.

[rwols]

What is the actual, technical, problem being avoided when choosing to not allowing sync run methods?

I see claims like:

• "unpredictable chaos"
• "asyncio is weird"
• "asyncio is a big breaking change"

Can you elaborate on what is unpredictable or weird about it?

Let me try to make a case...

You mention:

Because sublime_aio explicitly was designed with the idea in mind to exclusively rely on and support modern asyncio, only, which means exclusive use of async def and await mechansims.

As proven in this PR, there is no technical reason why exclusive use of async def is required in these command classes.

Also, what's the point of using async def if synchronous code is to be executed the same way?

The plugin may decide to do something synchronous, and then invoke call_coroutine/run_coroutine manually. That is in fact necessary if you want to apply some textual edit with the edit token, and right after that do something asynchronous.

But on top of that, like I mentioned a few times, there is a gradual refactoring / upgrade path possible when allowing these run functions to remain sync.

unpredictable chaos

Regarding the unpredictable chaos, I really don't see it. (But perhaps I have to get better educated on this). If you have a sync run() method defined, then there is no way to invoke async methods in the special sublime_aio.View / sublime_aio.Window classes anyway.

I hope this helps sway the opinion.

[deathaxe]

... there is no technical reason why exclusive use of async def is required ...

This statement is too true and summarizes python asyncio's major design failure as a whole.

Everything - the insane it might be - is possible.

Quick comparison: Python does not support protected/private modifiers to enforce visibility/accessibility rules like C++/C# or Java. Users are responsible to use the language in sane ways, but they can do everything insane as well.

It's the same with asyncio. Being technically able to to everything doesn't mean it is good design nor desirable.

Instead of asynchronous io concept and its async/await keywords being a core language feature as in other popular languages, python added asyncio as a badly designed and still incomplete library-level hack around ordinary python coroutines.

I don't want to argue about reasons.

It didn't even know about or support async/await in the first days. That's why asyncio's event loops run ordinary synchronous functions at its core, with dozens of levels of wrapper code and an insane callback hell, each of which needing to check types of entities (functions, corotine functions, coroutine objects, asyncio tasks, futures, ...) it handles. This is where nearly each function has to call iscoroutine and friends to know how to treat a passed "function".

See how they even leave you as a user behind with responsibility for pending tasks to not get garbage collected before being called. They track the tasks, would be able to do it on their own, but do it in weakrefs to ensure they get deleted before called.

See how asyncio can't even handle asynchronous file io at its core. Even addons like anyio just call synchronous functions pushed to worker threads in a world of GIL effectively blocking multiple threads from working concurrently, efficiently.

Python's asyncio works on an insane high level of entropy.

This is what I call "chaos".

IMHO, they should restart thinking about it, completely.

---

This is where libraries such as trio jumped in with the goal to improve the situation by focusing on providing users with sane(er) API surface which more behaves like a core language feature.

This is what all 3rd-party libraries are heading to. Exclusively support async/await APIs.

A common design found in libraries is:

1. dedicated AsyncSomething vs. Something classes for asyncio vs. synchronous code with same methods e.g. run() or close().

2. multi-capable AsyncOrSyncSomething classes with e.g. async def arun() vs. def run() to be able to run on both, event loops and synchronous functions.

3. a combination of 1 and 2

4. httpx also defines methods like async def handle_async_request() vs. def handle_request()

The major point is explicit visibility about what something does and what desired target runtime is - async or sync.

The core requirement therefore is:

Classes and their methods "must" be clear about whether they are designed for being executed in an event loop or in synchronous code to avoid possibly fatal confusion and minimize risk of critical bugs, such as running expensive long lasting CPU bound synchronous functions on the core event loop thread, effectively blocking anything else.

And this is what sublime_aio is designed for and what this PR violates.

Merging this PR would open doors with dragons behind, which can't be closed again, adding support undesirable design flaws.

The only use case and justification for any Command or Listener class in this library is to provide a first-class async/await supporting alternative for existing counter classes in sublime and sublime_plugin modules with an as identical as possible API front-end.

Idea is for migration to be as easy as replacing sublime_plugin by sublime_aio and def by async def for related methods in majority of use-cases.

If a package needs extra steps to maintain certain levels of backward compatibility by violating modern python's asyncio design goals, it is to be implemented by that plugin.

It is not within scope of a core library to provide all sorts of insanity which is technically possible.

That's what was refered to as "not designed for" or "design decision". A design decision decides for one of many technically possible directions to keep infrastructure and API in sane limits.