Actions return Literal["break"] | None

[benjamin-kirkbride]

....
Thinking about it though, you would need a way to return "break" from the action (not currently done). More thought is needed on that.
....

Originally posted by @benjamin-kirkbride in #1397 (comment)

What if all "Action" types optionally returned "break"

Also, what if the signature was Literal["break"] | None?

We discussed this before (#1309) but I think that for actions the issue of other bindings returning arbitrary strings is not a concern ?

[rdbende]

In theory we're still able to add an after_idle as the action callback, right? So it might be an issue.

[benjamin-kirkbride]

@rdbende could you give a minimal example to illustrate what you mean? I'll admit I'm out of my depth on this 😅

[rdbende]

So you can make an action like this:

some_action = actions.register_bare_action(
    name="Spam",
    description="Ham",
    callback=lambda: widget.after_idle(egg()),
)

Which, when invoked would run egg() after the event loop has become idle. However, after_idle returns a string (the id of the scheduled after callback -> something like "after#0") and so the type checker would complain about it, because it's neither None nor Literal["break"].

I think it would be useful to be able to return "break" from an action, but the signature should be str | None.

[Akuli]

I don't like this, because it duplicates the functionality that bind() already has.

[benjamin-kirkbride]

Can you be more specific on what you don't like and why @Akuli? What I'm trying to figure out here is how to make Actions work with bind(), so I'm not sure what your concern is.

[Akuli]

If I understand correctly, this issue is all about triggering an action with a bind. Specifically, you want a convenient way to do it without writing "if action is enabled: invoke action" in every plugin that does it.

I still think we shouldn't design this yet, or even decide if we need it yet. After all, the goal is to clean up repetitive code that doesn't exist yet, and we won't know how repeatitive it is or how ugly it will be until the code exists. If this really needs to be done, we will create a new issue and come up with a concrete design later.

[benjamin-kirkbride]

Specifically, you want a convenient way to do it without writing "if action is enabled: invoke action" in every plugin that does it.

I'm not sure this is correct, but I'm also not 100% sure what you mean.

I am just trying to iron out how to bind an action to a widget.

I still think we shouldn't design this yet, or even decide if we need it yet

Well, this does block #1328 as I need to know how to handle binding actions to tab/shift-tab, but it can definitely wait if you want to table it for the moment

[Akuli]

So you want your action to decide whether a tab key press should do something else apart from triggering the action?

I think it should be the bind's responsibility, not the action's responsibility. In your case, the bind callback should probably prevent tab key presses from doing anything else when the action runs, so maybe something like this pseudo-code:

def on_tab_key_press(event):
    if action.is_available(tab):
        action.run(tab)
        return "break"

utils.bind_tab_key(tab.textwidget, on_tab)

This doesn't require us to design anything new and isn't blocking anything. I still think you are trying to design a convenience thing, not something that is needed to make things work.

[benjamin-kirkbride]

So you want your action to decide whether a tab key press should do something else apart from triggering the action?

Yes and no. I was wanting to pass the action directly to bind(), which obviously would require returning "break" if we want nothing else to run.

so maybe something like this pseudo-code

That seems reasonable, we can go with this :)

I still think you are trying to design a convenience thing

I was just trying to find a design we would be happy with