Akuli not happy with _menu_item_enabledness_callbacks

[benjamin-kirkbride]

          Not super happy with this design, but I just want this merged.

_Originally posted by @Akuli in https://github.com/Akuli/porcupine/pull/1349#discussion_r1280652436_

Can you expand on this?

Related: https://github.com/Akuli/porcupine/issues/1343

[Akuli]

It is enough and much simpler to detect whether an action is available or not. We don't need callbacks that run when an action becomes available or not available.

For menubar, I added some sample code to #1343, the idea is to enable/disable menu items when the user opens the menu. For right-click menus, we can similarly update the items to be or not to be grayed out when the menu is opened. For command palette we can check availability as the user is typing and we are about to display matching actions.

[benjamin-kirkbride]

We don't need callbacks that run when an action becomes available or not available.

That's not what that is doing? It's a list of callbacks to determine if a menu item is available (which may or may not correlate directly to an action)

[benjamin-kirkbride]

That said, triggering all of the enabledness callbacks when you click the menubar makes sense

[Akuli]

Ideally all menu items would be actions, and the menubar would just check whether those actions are enabled whenever a user looks into the menubar.

Once we are at that point, we can delete _menu_item_enabledness_callbacks and close this issue. I don't think it makes sense to change it now, because it does work.

[benjamin-kirkbride]

Ideally all menu items would be actions, and the menubar would just check whether those actions are enabled whenever a user looks into the menubar.

That is what _menu_item_enabledness_callbacks is for :sweat_smile: unless I'm very confused

[Moosems]

Why the flip is the name so long 😂?

[benjamin-kirkbride]

Why the flip is the name so long?

Explicit is better than implicit.

[rdbende]

Why the flip is the name so long 😂?

Oh it's average. Look at this:

https://github.com/Akuli/porcupine/blob/9ff4be00597ec2be3cc7a53f1a383d1f36df46e5/porcupine/settings.py#L969

[Moosems]

Simple is better than complex.

[rdbende]

It's 2023, autocompletion is a thing.

[Moosems]

Oh it's average. Look at this:

https://github.com/Akuli/porcupine/blob/9ff4be00597ec2be3cc7a53f1a383d1f36df46e5/porcupine/settings.py#L969

That was a complete sentence 😄.

[Akuli]

.. menubar would just check whether those actions are enabled whenever a user looks into the menubar.

That is what _menu_item_enabledness_callbacks is for sweat_smile unless I'm very confused

Once all menu items are actions, this can be done in a less fragile way without a global callback list. Each action knows whether it is active or not, and if we have a way to find the action corresponding to each menu item, that's all we need.

[benjamin-kirkbride]

Each action knows whether it is active or not

Not exactly: each action has a list of callbacks that determine whether or not an Action is available or not. This might seem like a pedantic distinction, but the reason I bring it up is because we do need to keep track of what needs to be executed to determine if a menu item is available.

Either way though I think I agree that in time this will become cleaner