From 9b5e99c9a1d3e024de8003acfbe772f8d515bd7a Mon Sep 17 00:00:00 2001 From: Samuel Abels Date: Mon, 12 Oct 2026 23:08:06 +0200 Subject: [PATCH] fix: attribute action extension handlers by identity; add enabled_when ActionExtensionRegistry keyed its addon map by handler function name, so addons registering same-named handlers (print-and-cut, sketcher) collided: disabling one left stale handlers behind or stripped the other addon's handlers. The map is now keyed by handler identity, and registering an already-registered handler is a no-op. The register_actions hook is now also invoked when an addon is enabled while the window exists, and action_registry.register() accepts an enabled_when predicate that ActionManager evaluates on every action state update. Addons can therefore create, place and enable/disable their actions in the register_actions hook alone, without action_extension_registry. Plugin API 22. --- CHANGELOG.md | 10 +- rayforge/core/hooks.py | 12 +- rayforge/ui_gtk/action_registry.py | 20 ++++ rayforge/ui_gtk/actions.py | 37 ++++-- rayforge/ui_gtk/mainwindow.py | 4 + tests/ui_gtk/test_action_registries.py | 155 +++++++++++++++++++++++++ 6 files changed, 226 insertions(+), 12 deletions(-) create mode 100644 tests/ui_gtk/test_action_registries.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 618e33fde..d50890289 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,11 +62,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 existing machine configurations continue to work unchanged. Machines saved while the driver carried its experimental name keep working through a compatibility alias +- Addon API 22: actions registered via the `register_actions` hook + accept an `enabled_when` predicate for their enabled state, and the + hook is invoked again when an addon is enabled while the window + exists, so addons no longer need `action_extension_registry` for + either - Updated translations ### Fixed -- Engrave: the step's Bidirectional Scan Offset had no effect since +- Addon API: disabling an addon did not reliably remove its + `action_extension_registry` handlers when several addons registered + same-named handlers, which could leave stale handlers behind or + strip other addons' handlers- Engrave: the step's Bidirectional Scan Offset had no effect since the pipeline moved to raygeo intents, because the value no longer reached the transformer; changing it now also refreshes the cached toolpath (#307) diff --git a/rayforge/core/hooks.py b/rayforge/core/hooks.py index 16abbab0f..880fffbb7 100644 --- a/rayforge/core/hooks.py +++ b/rayforge/core/hooks.py @@ -4,13 +4,23 @@ hookimpl = pluggy.HookimplMarker("rayforge") MINIMUM_API_VERSION = 16 -PLUGIN_API_VERSION = 21 +PLUGIN_API_VERSION = 22 """ API Changelog ============= +Version 22 +---------- +``action_registry.register()`` accepts an ``enabled_when`` predicate that +receives the ``ActionManager`` and is evaluated on every action state +update. Addons that used ``action_extension_registry`` only to enable and +disable their action can now register directly in the +``register_actions`` hook. That hook is also invoked again when an addon +is enabled while the window exists, so runtime-enabled addons no longer +need ``action_extension_registry`` to have their actions created. + Version 21 ---------- Renamed ``WorkOriginElement.set_coordinate_space(space)`` to diff --git a/rayforge/ui_gtk/action_registry.py b/rayforge/ui_gtk/action_registry.py index 3a508d5c7..82077a2c5 100644 --- a/rayforge/ui_gtk/action_registry.py +++ b/rayforge/ui_gtk/action_registry.py @@ -1,9 +1,14 @@ import logging +from collections.abc import Callable from dataclasses import dataclass +from typing import TYPE_CHECKING from blinker import Signal from gi.repository import Gio, Gtk +if TYPE_CHECKING: + from .actions import ActionManager + logger = logging.getLogger(__name__) @@ -29,6 +34,7 @@ class ActionInfo: addon_name: str | None = None menu: MenuPlacement | None = None toolbar: ToolbarPlacement | None = None + enabled_when: 'Callable[["ActionManager"], bool] | None' = None class ActionRegistry: @@ -64,6 +70,7 @@ def register( shortcut: str | None = None, menu: MenuPlacement | None = None, toolbar: ToolbarPlacement | None = None, + enabled_when: 'Callable[["ActionManager"], bool] | None' = None, ) -> None: """ Register an action with the window and track it by addon. @@ -78,6 +85,9 @@ def register( menu: Menu placement info if this action should appear in a menu. toolbar: Toolbar placement info if this action should appear in the toolbar. + enabled_when: Predicate receiving the ActionManager, + evaluated on every action state update to set whether + the action is enabled. """ if self._window is None: logger.warning("Cannot register action: window not set") @@ -102,6 +112,7 @@ def register( addon_name=addon_name or "", menu=menu, toolbar=toolbar, + enabled_when=enabled_when, ) self._actions[action_name] = info @@ -216,6 +227,15 @@ def get_toolbar_items(self, group: str) -> list[ActionInfo]: items, key=lambda x: x.toolbar.priority if x.toolbar else 100 ) + def get_all(self) -> list[ActionInfo]: + """ + Get all registered actions. + + Returns: + List of all ActionInfo objects. + """ + return list(self._actions.values()) + def get_all_with_shortcuts(self) -> list[ActionInfo]: """ Get all actions that have keyboard shortcuts defined. diff --git a/rayforge/ui_gtk/actions.py b/rayforge/ui_gtk/actions.py index 2b6528625..a3561cf4f 100644 --- a/rayforge/ui_gtk/actions.py +++ b/rayforge/ui_gtk/actions.py @@ -111,21 +111,23 @@ class ActionExtensionRegistry: def __init__(self): self._setup_handlers: list[ActionSetupHandler] = [] self._state_update_handlers: list[ActionStateUpdateHandler] = [] - self._setup_addon_map: dict[str, str] = {} - self._state_update_addon_map: dict[str, str] = {} + self._setup_addon_map: dict[int, str] = {} + self._state_update_addon_map: dict[int, str] = {} def register_setup(self, handler: ActionSetupHandler, addon_name: str): """Register a handler to be called during action setup.""" + if handler in self._setup_handlers: + return self._setup_handlers.append(handler) if addon_name: - self._setup_addon_map[handler.__name__] = addon_name + self._setup_addon_map[id(handler)] = addon_name logger.debug(f"Registered action setup handler: {handler.__name__}") def unregister_setup(self, handler: ActionSetupHandler) -> bool: """Unregister an action setup handler.""" try: self._setup_handlers.remove(handler) - self._setup_addon_map.pop(handler.__name__, None) + self._setup_addon_map.pop(id(handler), None) return True except ValueError: return False @@ -134,9 +136,11 @@ def register_state_update( self, handler: ActionStateUpdateHandler, addon_name: str ): """Register a handler to be called during action state updates.""" + if handler in self._state_update_handlers: + return self._state_update_handlers.append(handler) if addon_name: - self._state_update_addon_map[handler.__name__] = addon_name + self._state_update_addon_map[id(handler)] = addon_name logger.debug( f"Registered action state update handler: {handler.__name__}" ) @@ -147,7 +151,7 @@ def unregister_state_update( """Unregister an action state update handler.""" try: self._state_update_handlers.remove(handler) - self._state_update_addon_map.pop(handler.__name__, None) + self._state_update_addon_map.pop(id(handler), None) return True except ValueError: return False @@ -167,21 +171,21 @@ def unregister_all_from_addon(self, addon_name: str) -> int: setup_to_remove = [ h for h in self._setup_handlers - if self._setup_addon_map.get(h.__name__) == addon_name + if self._setup_addon_map.get(id(h)) == addon_name ] for h in setup_to_remove: self._setup_handlers.remove(h) - self._setup_addon_map.pop(h.__name__, None) + self._setup_addon_map.pop(id(h), None) count += len(setup_to_remove) state_to_remove = [ h for h in self._state_update_handlers - if self._state_update_addon_map.get(h.__name__) == addon_name + if self._state_update_addon_map.get(id(h)) == addon_name ] for h in state_to_remove: self._state_update_handlers.remove(h) - self._state_update_addon_map.pop(h.__name__, None) + self._state_update_addon_map.pop(id(h), None) count += len(state_to_remove) if count > 0: @@ -583,6 +587,19 @@ def update_action_states(self, *args, **kwargs): ) action_extension_registry.invoke_state_update_handlers(self) + self._update_predicated_actions() + + def _update_predicated_actions(self): + """Sets enabled state of actions registered with enabled_when.""" + for info in action_registry.get_all(): + if info.enabled_when is None: + continue + try: + info.action.set_enabled(info.enabled_when(self)) + except Exception: + logger.exception( + f"Error updating state of action '{info.action_name}'" + ) def on_add_stock(self, action, param): """Handler for the 'add-stock' action.""" diff --git a/rayforge/ui_gtk/mainwindow.py b/rayforge/ui_gtk/mainwindow.py index 44c5aed66..db1cadddc 100644 --- a/rayforge/ui_gtk/mainwindow.py +++ b/rayforge/ui_gtk/mainwindow.py @@ -973,6 +973,10 @@ def _on_job_started(self, sender): def _on_addon_state_changed(self, sender, addon_name): """Handle addon enable/disable to refresh action handlers.""" + if addon_name in sender.loaded_addons: + call_registration_hooks( + get_context().plugin_mgr, window_required=True + ) action_extension_registry.invoke_setup_handlers(self.action_manager) self.action_manager.update_action_states() diff --git a/tests/ui_gtk/test_action_registries.py b/tests/ui_gtk/test_action_registries.py new file mode 100644 index 000000000..1212c229d --- /dev/null +++ b/tests/ui_gtk/test_action_registries.py @@ -0,0 +1,155 @@ +# flake8: noqa: E402 +import os +import sys +from typing import cast + +import pytest + +if sys.platform.startswith("linux") and not os.environ.get("DISPLAY"): + pytest.skip( + "DISPLAY not set on Linux, skipping UI tests. Run with xvfb-run.", + allow_module_level=True, + ) + +import gi + +gi.require_version("Gtk", "4.0") +gi.require_version("Adw", "1") +from gi.repository import Gio, Gtk + +from rayforge.ui_gtk.action_registry import ActionRegistry +from rayforge.ui_gtk.actions import ActionExtensionRegistry, ActionManager + +pytestmark = pytest.mark.ui + + +class StubWindow: + def __init__(self): + self.added: list[Gio.SimpleAction] = [] + self.removed: list[Gio.SimpleAction] = [] + + def add_action(self, action): + self.added.append(action) + + def remove_action(self, action): + self.removed.append(action) + + +def _make_handler(): + def _register_actions(action_manager): + pass + + return _register_actions + + +def _make_manager() -> ActionManager: + return ActionManager.__new__(ActionManager) + + +def test_unregister_all_from_addon_with_same_handler_names(): + registry = ActionExtensionRegistry() + sketcher_handler = _make_handler() + print_and_cut_handler = _make_handler() + registry.register_setup(sketcher_handler, "sketcher") + registry.register_setup(print_and_cut_handler, "print_and_cut") + + assert registry.unregister_all_from_addon("sketcher") == 1 + assert registry._setup_handlers == [print_and_cut_handler] + + assert registry.unregister_all_from_addon("print_and_cut") == 1 + assert registry._setup_handlers == [] + + +def test_register_setup_is_idempotent(): + registry = ActionExtensionRegistry() + handler = _make_handler() + registry.register_setup(handler, "addon") + registry.register_setup(handler, "addon") + + assert registry._setup_handlers == [handler] + assert registry.unregister_all_from_addon("addon") == 1 + + +def test_register_state_update_is_idempotent(): + registry = ActionExtensionRegistry() + + def _update_action_states(action_manager): + pass + + registry.register_state_update(_update_action_states, "addon") + registry.register_state_update(_update_action_states, "addon") + + assert registry._state_update_handlers == [_update_action_states] + + +def test_register_stores_enabled_when(): + registry = ActionRegistry() + registry.set_window(cast(Gtk.ApplicationWindow, StubWindow())) + action = Gio.SimpleAction.new("test-action", None) + + def enabled_when(action_manager): + return True + + registry.register( + action_name="test-action", + action=action, + addon_name="test", + enabled_when=enabled_when, + ) + + info = registry.get("test-action") + assert info is not None and info.enabled_when is enabled_when + registry.unregister("test-action") + + +def test_update_predicated_actions_evaluates_predicate(monkeypatch): + registry = ActionRegistry() + registry.set_window(cast(Gtk.ApplicationWindow, StubWindow())) + monkeypatch.setattr("rayforge.ui_gtk.actions.action_registry", registry) + action = Gio.SimpleAction.new("predicated-test-action", None) + action.set_enabled(False) + received = [] + + def enabled_when(action_manager): + received.append(action_manager) + return True + + registry.register( + action_name="predicated-test-action", + action=action, + addon_name="test", + enabled_when=enabled_when, + ) + + manager = _make_manager() + manager._update_predicated_actions() + assert action.get_enabled() is True + assert received == [manager] + + +def test_update_predicated_actions_isolates_failing_predicate(monkeypatch): + registry = ActionRegistry() + registry.set_window(cast(Gtk.ApplicationWindow, StubWindow())) + monkeypatch.setattr("rayforge.ui_gtk.actions.action_registry", registry) + bad = Gio.SimpleAction.new("predicated-bad-action", None) + good = Gio.SimpleAction.new("predicated-good-action", None) + good.set_enabled(False) + + def failing(action_manager): + raise RuntimeError("boom") + + registry.register( + action_name="predicated-bad-action", + action=bad, + addon_name="test", + enabled_when=failing, + ) + registry.register( + action_name="predicated-good-action", + action=good, + addon_name="test", + enabled_when=lambda am: True, + ) + + _make_manager()._update_predicated_actions() + assert good.get_enabled() is True