refactor: restrict to only one command per id (#30)

src/app_model/registries/_commands_reg.py

@@ -9,7 +9,7 @@
 from typing_extensions import ParamSpec
 
 if TYPE_CHECKING:
-    from typing import Dict, Iterator, List, Tuple
+    from typing import Dict, Iterator, Tuple
 
     DisposeCallable = Callable[[], None]
 
@@ -73,7 +73,7 @@ class CommandsRegistry:
     registered = Signal(str)
 
     def __init__(self, injection_store: Optional[Store] = None) -> None:
-        self._commands: Dict[str, List[_RegisteredCommand]] = {}
+        self._commands: Dict[str, _RegisteredCommand] = {}
         self._injection_store = injection_store
 
     def register_command(
@@ -95,20 +95,19 @@ def register_command(
         DisposeCallable
             A function that can be called to unregister the command.
         """
-        commands = self._commands.setdefault(id, [])
+        if id in self._commands:
+            raise ValueError(f"Command {id!r} already registered")
 
         cmd = _RegisteredCommand(id, callback, title, self._injection_store)
-        commands.insert(0, cmd)
+        self._commands[id] = cmd
 
         def _dispose() -> None:
-            commands.remove(cmd)
-            if not commands:
-                del self._commands[id]
+            self._commands.pop(id, None)
 
         self.registered.emit(id)
         return _dispose
 
-    def __iter__(self) -> Iterator[Tuple[str, List[_RegisteredCommand]]]:
+    def __iter__(self) -> Iterator[Tuple[str, _RegisteredCommand]]:
         yield from self._commands.items()
 
     def __contains__(self, id: str) -> bool:
@@ -118,7 +117,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) -> List[_RegisteredCommand]:
+    def __getitem__(self, id: str) -> _RegisteredCommand:
         """Retrieve commands registered under a given ID."""
         return self._commands[id]
 
@@ -145,22 +144,19 @@ def execute_command(
 
         Returns
         -------
-        Future: conconrent.futures.Future
+        Future: concurrent.futures.Future
             Future object containing the result of the command
 
         Raises
         ------
         KeyError
             If the command is not registered or has no callbacks.
         """
-        if cmds := self[id]:
-            # TODO: decide whether we'll ever have more than one command
-            # and if so, how to handle it
-            cmd = cmds[0].run_injected
-        else:
-            raise KeyError(
-                f'Command "{id}" has no registered callbacks'
-            )  # pragma: no cover
+        try:
+            cmd = self._commands[id].run_injected
+        except KeyError as e:
+            raise KeyError(f"Command {id!r} not registered") from e
+
         if execute_asychronously:
             with ThreadPoolExecutor() as executor:
                 return executor.submit(cmd, *args, **kwargs)
@@ -173,7 +169,5 @@ def execute_command(
             return future
 
     def __str__(self) -> str:
-        lines: list = []
-        for id, cmds in self:
-            lines.extend(f"{id!r:<32} -> {cmd.title!r}" for cmd in cmds)
+        lines = [f"{id!r:<32} -> {cmd.title!r}" for id, cmd in self]
         return "\n".join(lines)

tests/conftest.py

@@ -1,5 +1,6 @@
 import sys
 from pathlib import Path
+from typing import List
 from unittest.mock import Mock
 
 import pytest
@@ -25,6 +26,7 @@ class Menus:
 
 
 class Commands:
+    TOP = "top"
     OPEN = "open"
     UNDO = "undo"
     REDO = "redo"
@@ -98,7 +100,7 @@ def build_app(name: str = "complete_test_app") -> FullApp:
         ]
     )
 
-    actions = [
+    actions: List[Action] = [
         Action(
             id=Commands.OPEN,
             title="Open...",
@@ -160,9 +162,9 @@ def build_app(name: str = "complete_test_app") -> FullApp:
         ),
         # test the navigation key
         Action(
-            id=Commands.OPEN,
+            id=Commands.TOP,
             title="AtTop",
-            callback=app.mocks.open,
+            callback=lambda: None,
             menus=[{"id": Menus.EDIT, "group": "navigation"}],
         ),
         # test submenus

tests/test_registries.py

@@ -1,3 +1,5 @@
+import pytest
+
 from app_model.registries import CommandsRegistry, KeyBindingsRegistry, MenusRegistry
 from app_model.types import MenuItem
 
@@ -23,3 +25,6 @@ def test_commands_registry():
     reg.register_command("my.id", lambda: None, "My Title")
     assert "(1 commands)" in repr(reg)
     assert "my.id" in str(reg)
+
+    with pytest.raises(ValueError, match="Command 'my.id' already registered"):
+        reg.register_command("my.id", lambda: None, "My Title")