Should notify_shell() really be async?

[Patryk27]

Hi, I saw this bit (this somewhat unnecessarily-async function):

crux/crux_core/src/capability/mod.rs

Line 559 in 238b8cc

pub async fn notify_shell(&self, operation: Op) {

... and got hooked - could you explain a bit more what it does?
(as in: I see the comment there, just don't follow the explanation 😄)

From what I've gathered, it might be related to this bit:

crux/crux_core/src/command/stream.rs

Line 36 in 238b8cc

// Check events first to preserve the order in which items were emitted. This is because

... but details remain a bit fuzzy, could use an explanation 👀

[charypar]

I've grappled with the semantics of it quite a bit and went back and forth on it. It's philosophically async, because it amounts to a side-effect which will be executed later, but it's mechanically not async, since there's nothing to wait for, not even completion.

Note that in the CommandContext, it no longer is: https://github.com/redbadger/crux/blob/master/crux_core/src/command/context.rs#L40

I suppose it would be possible to make it return a future which resolves to () on 'completion', i.e. once the notification is delivered -taken off the effects channel using Command::effects(). Only then would the enclosing future be allowed to proceed.

As a general suggestion, don't look too closely at the capability module, it is all about to get deprecated and later removed 😅.