feat: give registries more control over registration of actions

.github/workflows/ci.yml

@@ -67,7 +67,6 @@ jobs:
           - python-version: "3.10"
             os: "ubuntu-latest"
             qt: "PySide2~=5.15.0"
-
           - python-version: "3.10"
             os: "ubuntu-latest"
             qt: "PyQt6~=6.2.0"

src/app_model/registries/__init__.py

@@ -1,6 +1,6 @@
 """App-model registries, such as menus, keybindings, commands."""
 
-from ._commands_reg import CommandsRegistry
+from ._commands_reg import CommandsRegistry, RegisteredCommand
 from ._keybindings_reg import KeyBindingsRegistry
 from ._menus_reg import MenusRegistry
 from ._register import register_action
@@ -10,4 +10,5 @@
     "KeyBindingsRegistry",
     "MenusRegistry",
     "register_action",
+    "RegisteredCommand",
 ]

src/app_model/registries/_commands_reg.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 from concurrent.futures import Future, ThreadPoolExecutor
-from functools import cached_property
 from typing import TYPE_CHECKING, Any, Callable, Generic, Iterator, TypeVar, cast
 
 from in_n_out import Store
@@ -11,7 +10,7 @@
 if TYPE_CHECKING:
     from typing_extensions import ParamSpec
 
-    from app_model.types import DisposeCallable
+    from app_model.types import Action, DisposeCallable
 
     P = ParamSpec("P")
 else:
@@ -26,15 +25,26 @@
 R = TypeVar("R")
 
 
-class _RegisteredCommand(Generic[P, R]):
+class RegisteredCommand(Generic[P, R]):
     """Small object to represent a command in the CommandsRegistry.
 
-    Only used internally by the CommandsRegistry.
+    Used internally by the CommandsRegistry.
+
     This helper class allows us to cache the dependency-injected variant of the
-    command. As usual with `cached_property`, the cache can be cleard by deleting
-    the attribute: `del cmd.run_injected`
+    command, so that type resolution and dependency injection is performed only
+    once.
     """
 
+    __slots__ = (
+        "id",
+        "callback",
+        "title",
+        "_resolved_callback",
+        "_injection_store",
+        "_injected_callback",
+        "_initialized",
+    )
+
     def __init__(
         self,
         id: str,
@@ -45,35 +55,55 @@ def __init__(
         self.id = id
         self.callback = callback
         self.title = title
-        self._resolved_callback = callback if callable(callback) else None
         self._injection_store: Store = store or Store.get_store()
+        self._resolved_callback = callback if callable(callback) else None
+        self._injected_callback: Callable[P, R] | None = None
+        self._initialized = True
+
+    def __setattr__(self, name: str, value: Any) -> None:
+        """Object is immutable after initialization."""
+        if getattr(self, "_initialized", False):
+            raise AttributeError("RegisteredCommand object is immutable.")
+        super().__setattr__(name, value)
 
     @property
     def resolved_callback(self) -> Callable[P, R]:
+        """Return the resolved command callback.
+
+        This property is cached, so the callback types are only resolved once.
+        """
         if self._resolved_callback is None:
             from app_model.types._utils import import_python_name
 
             try:
-                self._resolved_callback = import_python_name(str(self.callback))
+                cb = import_python_name(str(self.callback))
             except ImportError as e:
-                self._resolved_callback = cast(Callable[P, R], lambda *a, **k: None)
+                object.__setattr__(self, "_resolved_callback", lambda *a, **k: None)
                 raise type(e)(
                     f"Command pointer {self.callback!r} registered for Command "
                     f"{self.id!r} was not importable: {e}"
                 ) from e
 
-            if not callable(self._resolved_callback):
+            if not callable(cb):
                 # don't try to import again, just create a no-op
-                self._resolved_callback = cast(Callable[P, R], lambda *a, **k: None)
+                object.__setattr__(self, "_resolved_callback", lambda *a, **k: None)
                 raise TypeError(
                     f"Command pointer {self.callback!r} registered for Command "
                     f"{self.id!r} did not resolve to a callble object."
                 )
-        return self._resolved_callback
+            object.__setattr__(self, "_resolved_callback", cb)
+        return cast("Callable[P, R]", self._resolved_callback)
 
-    @cached_property
+    @property
     def run_injected(self) -> Callable[P, R]:
-        return self._injection_store.inject(self.resolved_callback, processors=True)
+        """Return the command callback with dependencies injected.
+
+        This property is cached, so the injected version is only created once.
+        """
+        if self._injected_callback is None:
+            cb = self._injection_store.inject(self.resolved_callback, processors=True)
+            object.__setattr__(self, "_injected_callback", cb)
+        return cast("Callable[P, R]", self._injected_callback)
 
 
 class CommandsRegistry:
@@ -86,10 +116,28 @@ def __init__(
         injection_store: Store | None = None,
         raise_synchronous_exceptions: bool = False,
     ) -> None:
-        self._commands: dict[str, _RegisteredCommand] = {}
+        self._commands: dict[str, RegisteredCommand] = {}
         self._injection_store = injection_store
         self._raise_synchronous_exceptions = raise_synchronous_exceptions
 
+    def register_action(self, action: Action) -> DisposeCallable:
+        """Register an Action object.
+
+        This is a convenience method that registers the action's callback
+        with the action's ID and title using `register_command`.
+
+        Parameters
+        ----------
+        action: Action
+            Action to register
+
+        Returns
+        -------
+        DisposeCallable
+            A function that can be called to unregister the action.
+        """
+        return self.register_command(action.id, action.callback, action.title)
+
     def register_command(
         self, id: str, callback: Callable[P, R] | str, title: str
     ) -> DisposeCallable:
@@ -115,7 +163,7 @@ def register_command(
                 f"{self._commands[id].callback!r} (new callback: {callback!r})"
             )
 
-        cmd = _RegisteredCommand(id, callback, title, self._injection_store)
+        cmd = RegisteredCommand(id, callback, title, self._injection_store)
         self._commands[id] = cmd
 
         def _dispose() -> None:
@@ -124,7 +172,7 @@ def _dispose() -> None:
         self.registered.emit(id)
         return _dispose
 
-    def __iter__(self) -> Iterator[tuple[str, _RegisteredCommand]]:
+    def __iter__(self) -> Iterator[tuple[str, RegisteredCommand]]:
         yield from self._commands.items()
 
     def __len__(self) -> int:
@@ -137,7 +185,7 @@ def __repr__(self) -> str:
         name = self.__class__.__name__
         return f"<{name} at {hex(id(self))} ({len(self._commands)} commands)>"
 
-    def __getitem__(self, id: str) -> _RegisteredCommand:
+    def __getitem__(self, id: str) -> RegisteredCommand:
         """Retrieve commands registered under a given ID."""
         if id not in self._commands:
             raise KeyError(f"Command {id!r} not registered")

src/app_model/registries/_keybindings_reg.py

@@ -10,7 +10,7 @@
     from typing import Iterator, TypeVar
 
     from app_model import expressions
-    from app_model.types import DisposeCallable, KeyBindingRule
+    from app_model.types import Action, DisposeCallable, KeyBindingRule
 
     CommandDecorator = Callable[[Callable], Callable]
     CommandCallable = TypeVar("CommandCallable", bound=Callable)
@@ -33,6 +33,47 @@ class KeyBindingsRegistry:
     def __init__(self) -> None:
         self._keybindings: list[_RegisteredKeyBinding] = []
 
+    def register_action_keybindings(self, action: Action) -> DisposeCallable | None:
+        """Register all keybindings declared in `action.keybindings`.
+
+        Parameters
+        ----------
+        action : Action
+            The action to register keybindings for.
+
+        Returns
+        -------
+        DisposeCallable | None
+            A function that can be called to unregister the keybindings.  If no
+            keybindings were registered, returns None.
+        """
+        if not (keybindings := action.keybindings):
+            return None
+
+        disposers: list[Callable[[], None]] = []
+        for keyb in keybindings:
+            if action.enablement is not None:
+                kwargs = keyb.model_dump()
+                kwargs["when"] = (
+                    action.enablement
+                    if keyb.when is None
+                    else action.enablement | keyb.when
+                )
+                _keyb = type(keyb)(**kwargs)
+            else:
+                _keyb = keyb
+            if d := self.register_keybinding_rule(action.id, _keyb):
+                disposers.append(d)
+
+        if not disposers:  # pragma: no cover
+            return None
+
+        def _dispose() -> None:
+            for disposer in disposers:
+                disposer()
+
+        return _dispose
+
     def register_keybinding_rule(
         self, id: str, rule: KeyBindingRule
     ) -> DisposeCallable | None:

src/app_model/registries/_menus_reg.py

@@ -1,13 +1,13 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Any, Callable, Final, Iterable, Iterator, Sequence
+from typing import TYPE_CHECKING, Any, Callable, Final, Iterable, Iterator
 
 from psygnal import Signal
 
 from app_model.types import MenuItem
 
 if TYPE_CHECKING:
-    from app_model.types import DisposeCallable, MenuOrSubmenu
+    from app_model.types import Action, DisposeCallable, MenuOrSubmenu
 
 MenuId = str
 
@@ -21,14 +21,54 @@ class MenusRegistry:
     def __init__(self) -> None:
         self._menu_items: dict[MenuId, dict[MenuOrSubmenu, None]] = {}
 
+    def append_action_menus(self, action: Action) -> DisposeCallable | None:
+        """Append all MenuRule items declared in `action.menus`.
+
+        Parameters
+        ----------
+        action : Action
+            The action containing menus to append.
+
+        Returns
+        -------
+        DisposeCallable | None
+            A function that can be called to unregister the menu items. If no
+            menu items were registered, returns `None`.
+        """
+        disposers: list[Callable[[], None]] = []
+        disp1 = self.append_menu_items(
+            (
+                rule.id,
+                MenuItem(
+                    command=action, when=rule.when, group=rule.group, order=rule.order
+                ),
+            )
+            for rule in action.menus or ()
+        )
+        disposers.append(disp1)
+
+        if action.palette:
+            menu_item = MenuItem(command=action, when=action.enablement)
+            disp = self.append_menu_items([(self.COMMAND_PALETTE_ID, menu_item)])
+            disposers.append(disp)
+
+        if not disposers:  # pragma: no cover
+            return None
+
+        def _dispose() -> None:
+            for disposer in disposers:
+                disposer()
+
+        return _dispose
+
     def append_menu_items(
-        self, items: Sequence[tuple[MenuId, MenuOrSubmenu]]
+        self, items: Iterable[tuple[MenuId, MenuOrSubmenu]]
     ) -> DisposeCallable:
         """Append menu items to the registry.
 
         Parameters
         ----------
-        items : Sequence[Tuple[str, MenuOrSubmenu]]
+        items : Iterable[Tuple[str, MenuOrSubmenu]]
             Items to append.
 
         Returns

src/app_model/registries/_register.py

@@ -2,7 +2,7 @@
 
 from typing import TYPE_CHECKING, overload
 
-from app_model.types import Action, MenuItem
+from app_model.types import Action
 
 if TYPE_CHECKING:
     from typing import Any, Callable, Literal, TypeVar
@@ -270,38 +270,14 @@ def _register_action_obj(app: Application | str, action: Action) -> DisposeCalla
 
     app = app if isinstance(app, Application) else Application.get_or_create(app)
 
-    # command
-    disp_cmd = app.commands.register_command(action.id, action.callback, action.title)
-    disposers = [disp_cmd]
-
-    # menu
-    items = []
-    for rule in action.menus or ():
-        menu_item = MenuItem(
-            command=action, when=rule.when, group=rule.group, order=rule.order
-        )
-        items.append((rule.id, menu_item))
-    disposers.append(app.menus.append_menu_items(items))
-
-    if action.palette:
-        menu_item = MenuItem(command=action, when=action.enablement)
-        disp = app.menus.append_menu_items([(app.menus.COMMAND_PALETTE_ID, menu_item)])
-        disposers.append(disp)
-
-    # keybinding
-    for keyb in action.keybindings or ():
-        if action.enablement is not None:
-            kwargs = keyb.model_dump()
-            kwargs["when"] = (
-                action.enablement
-                if keyb.when is None
-                else action.enablement | keyb.when
-            )
-            _keyb = type(keyb)(**kwargs)
-        else:
-            _keyb = keyb
-        if _d := app.keybindings.register_keybinding_rule(action.id, _keyb):
-            disposers.append(_d)
+    # commands
+    disposers = [app.commands.register_action(action)]
+    # menus
+    if dm := app.menus.append_action_menus(action):
+        disposers.append(dm)
+    # keybindings
+    if dk := app.keybindings.register_action_keybindings(action):
+        disposers.append(dk)
 
     def _dispose() -> None:
         for d in disposers: