refactor: a number of fixes (#26)

.pre-commit-config.yaml

@@ -58,3 +58,4 @@ repos:
         files: "^src/"
         additional_dependencies:
           - pydantic
+          - in-n-out

src/app_model/_app.py

@@ -30,12 +30,12 @@ def __init__(self, name: str) -> None:
                 f"`Application.get_or_create({name!r})`."
             )
         Application._instances[name] = self
+        self.injection_store = ino.Store.create(name)
 
         self.keybindings = KeyBindingsRegistry()
         self.menus = MenusRegistry()
-        self.commands = CommandsRegistry()
+        self.commands = CommandsRegistry(self.injection_store)
 
-        self.injection_store = ino.Store.create(name)
         self.injection_store.on_unannotated_required_args = "ignore"
 
         self._disposers: List[Tuple[str, DisposeCallable]] = []

src/app_model/backends/qt/_qaction.py

@@ -65,7 +65,6 @@ def __init__(
         parent: Optional[QObject] = None,
         *,
         use_short_title: bool = False,
-        show_icon: bool = False,
     ):
         super().__init__(command_rule.id, app, parent)
         self._cmd_rule = command_rule
@@ -74,11 +73,13 @@ def __init__(
             self.setText(command_rule.short_title)  # pragma: no cover
         else:
             self.setText(command_rule.title)
-        if command_rule.icon and show_icon:
+        if command_rule.icon:
             self.setIcon(to_qicon(command_rule.icon))
         if command_rule.tooltip:
             self.setToolTip(command_rule.tooltip)
 
+        self.setIconVisibleInMenu(False)
+
     def update_from_context(self, ctx: Mapping[str, object]) -> None:
         """Update the enabled state of this menu item from `ctx`."""
         self.setEnabled(expr.eval(ctx) if (expr := self._cmd_rule.enablement) else True)
@@ -97,7 +98,7 @@ def __init__(
         app: Union[str, Application],
         parent: Optional[QObject] = None,
     ):
-        super().__init__(menu_item.command, app, parent, show_icon=False)
+        super().__init__(menu_item.command, app, parent)
         self._menu_item = menu_item
 
     def update_from_context(self, ctx: Mapping[str, object]) -> None:

src/app_model/registries/_commands_reg.py

@@ -2,23 +2,22 @@
 
 from concurrent.futures import Future, ThreadPoolExecutor
 from functools import cached_property
-from typing import TYPE_CHECKING, Any, Callable, Optional, Union, cast
+from typing import TYPE_CHECKING, Any, Callable, Generic, Optional, TypeVar, Union, cast
 
 from in_n_out import Store
 from psygnal import Signal
+from typing_extensions import ParamSpec
 
 if TYPE_CHECKING:
-    from typing import Dict, Iterator, List, Tuple, TypeVar
-
-    from typing_extensions import ParamSpec
-
-    P = ParamSpec("P")
-    R = TypeVar("R")
+    from typing import Dict, Iterator, List, Tuple
 
     DisposeCallable = Callable[[], None]
 
+P = ParamSpec("P")
+R = TypeVar("R")
 
-class _RegisteredCommand:
+
+class _RegisteredCommand(Generic[P, R]):
     """Small object to represent a command in the CommandsRegistry.
 
     Only used internally by the CommandsRegistry.
@@ -32,7 +31,7 @@ def __init__(
         id: str,
         callback: Union[str, Callable[P, R]],
         title: str,
-        store: Optional[Store],
+        store: Optional[Store] = None,
     ) -> None:
         self.id = id
         self.callback = callback
@@ -48,15 +47,15 @@ def resolved_callback(self) -> Callable[P, R]:
             try:
                 self._resolved_callback = import_python_name(str(self.callback))
             except ImportError as e:
-                self._resolved_callback = cast("Callable[P, R]", lambda *a, **k: None)
+                self._resolved_callback = cast(Callable[P, R], 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):
                 # don't try to import again, just create a no-op
-                self._resolved_callback = cast("Callable[P, R]", lambda *a, **k: None)
+                self._resolved_callback = cast(Callable[P, R], 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."
@@ -65,24 +64,20 @@ def resolved_callback(self) -> Callable[P, R]:
 
     @cached_property
     def run_injected(self) -> Callable[P, R]:
-        out = self._injection_store.inject(self.resolved_callback, processors=True)
-        return cast("Callable[P, R]", out)
+        return self._injection_store.inject(self.resolved_callback, processors=True)
 
 
 class CommandsRegistry:
     """Registry for commands (callable objects)."""
 
     registered = Signal(str)
 
-    def __init__(self) -> None:
+    def __init__(self, injection_store: Optional[Store] = None) -> None:
         self._commands: Dict[str, List[_RegisteredCommand]] = {}
+        self._injection_store = injection_store
 
     def register_command(
-        self,
-        id: str,
-        callback: Union[str, Callable[P, R]],
-        title: str = "",
-        store: Optional[Store] = None,
+        self, id: str, callback: Union[str, Callable[P, R]], title: str
     ) -> DisposeCallable:
         """Register a callable as the handler for command `id`.
 
@@ -93,10 +88,7 @@ def register_command(
         callback : Callable
             Callable to be called when the command is executed
         title : str
-            Optional title for the command.
-        store: Optional[in_n_out.Store]
-            Optional store to use for dependency injection.  If not provided,
-            the global store will be used.
+            Title for the command.
 
         Returns
         -------
@@ -105,7 +97,7 @@ def register_command(
         """
         commands = self._commands.setdefault(id, [])
 
-        cmd = _RegisteredCommand(id, callback, title, store)
+        cmd = _RegisteredCommand(id, callback, title, self._injection_store)
         commands.insert(0, cmd)
 
         def _dispose() -> None:

src/app_model/registries/_menus_reg.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import Callable, Dict, Iterator, List, Optional, Sequence, Set, Tuple
+from typing import Callable, Dict, Final, Iterator, List, Optional, Sequence, Set, Tuple
 
 from psygnal import Signal
 
@@ -11,6 +11,7 @@
 class MenusRegistry:
     """Registry for menu and submenu items."""
 
+    COMMAND_PALETTE_ID: Final = "_command_pallet_"
     menus_changed = Signal(set)
 
     def __init__(self) -> None:
@@ -33,12 +34,11 @@ def append_menu_items(
         """
         changed_ids: Set[str] = set()
         disposers: List[Callable[[], None]] = []
-
-        for id, item in items:
+        for menu_id, item in items:
             item = MenuItem.validate(item)  # type: ignore
-            menu_list = self._menu_items.setdefault(id, [])
+            menu_list = self._menu_items.setdefault(menu_id, [])
             menu_list.append(item)
-            changed_ids.add(id)
+            changed_ids.add(menu_id)
             disposers.append(lambda: menu_list.remove(item))
 
         def _dispose() -> None:

src/app_model/registries/_register.py

@@ -203,26 +203,34 @@ def _register_action_obj(
     app = app if isinstance(app, Application) else Application.get_or_create(app)
 
     # command
-    disposers = [
-        app.commands.register_command(
-            action.id, action.callback, action.title, app.injection_store
-        )
-    ]
+    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.add_to_command_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 _d := app.keybindings.register_keybinding_rule(action.id, keyb):
+        if action.enablement is not None:
+            _keyb = keyb.copy()
+            if _keyb.when is None:
+                _keyb.when = action.enablement
+            else:
+                _keyb.when = action.enablement | _keyb.when
+        else:
+            _keyb = keyb
+        if _d := app.keybindings.register_keybinding_rule(action.id, _keyb):
             disposers.append(_d)
 
     def _dispose() -> None:

src/app_model/types/__init__.py

@@ -2,14 +2,9 @@
 from ._action import Action
 from ._command_rule import CommandRule
 from ._icon import Icon, IconOrDict
-from ._keybinding_rule import (
-    KeyBindingRule,
-    KeyBindingRuleDict,
-    KeyBindingRuleOrDict,
-    KeyCodeStr,
-)
+from ._keybinding_rule import KeyBindingRule, KeyBindingRuleDict, KeyBindingRuleOrDict
 from ._keys import KeyBinding, KeyChord, KeyCode, KeyMod, SimpleKeyBinding
-from ._menu import (
+from ._menu_rule import (
     MenuItem,
     MenuOrSubmenu,
     MenuRule,
@@ -29,7 +24,6 @@
     "KeyBindingRuleOrDict",
     "KeyChord",
     "KeyCode",
-    "KeyCodeStr",
     "KeyMod",
     "MenuItem",
     "MenuOrSubmenu",

src/app_model/types/_action.py

@@ -5,7 +5,7 @@
 
 from ._command_rule import CommandRule
 from ._keybinding_rule import KeyBindingRule
-from ._menu import MenuRule
+from ._menu_rule import MenuRule
 from ._utils import _validate_python_name
 
 P = ParamSpec("P")

src/app_model/types/_keybinding_rule.py

@@ -1,12 +1,11 @@
-from typing import NewType, Optional, TypedDict, Union
+from typing import Optional, TypedDict, Union
 
 from pydantic import Field
 
 from .. import expressions
 from ._base import _StrictModel
 from ._constants import OperatingSystem
 
-KeyCodeStr = NewType("KeyCodeStr", str)
 KeyEncoding = Union[int, str]
 
 _OS = OperatingSystem.current()
@@ -57,10 +56,10 @@ def _bind_to_current_platform(self) -> Optional[KeyEncoding]:
 class KeyBindingRuleDict(TypedDict, total=False):
     """Typed dict for KeyBindingRule kwargs."""
 
-    primary: Optional[KeyCodeStr]
-    win: Optional[KeyCodeStr]
-    linux: Optional[KeyCodeStr]
-    mac: Optional[KeyCodeStr]
+    primary: Optional[str]
+    win: Optional[str]
+    linux: Optional[str]
+    mac: Optional[str]
     weight: int
     when: Optional[expressions.Expr]
 

tests/test_actions.py

@@ -21,6 +21,7 @@
         ]
     ),
     dict(keybindings=[{"primary": "ctrl+a"}], menus=[{"id": MENUID}]),
+    dict(add_to_command_palette=False),
 ]
 
 
@@ -80,12 +81,18 @@ def f2():
     assert app.commands.execute_command(cmd_id).result() == "hi"
 
     # make sure menus are registered if specified
+    menus = kwargs.get("menus", [])
     if menus := kwargs.get("menus"):
         for entry in menus:
             assert entry["id"] in app.menus
-            app.menus_changed.assert_called_with({entry["id"]})
+            app.menus_changed.assert_any_call({entry["id"]})
     else:
-        assert not list(app.menus)
+        menus = list(app.menus)
+        if kwargs.get("add_to_command_palette") is not False:
+            assert app.menus.COMMAND_PALETTE_ID in app.menus
+            assert len(menus) == 1
+        else:
+            assert not list(app.menus)
 
     # make sure keybindings are registered if specified
     if keybindings := kwargs.get("keybindings"):