Merge pull request #2714 from freakboy3742/commandset-api

Add delete/remove APIs to Commandset.

changes/2701.feature.rst

@@ -0,0 +1 @@
+CommandSet now exposes a full set and dictionary interface. Commands can be added to a CommandSet using ``[]`` notation and a command ID; they can be removed using set-like ``remove()`` or ``discard()`` calls with a Command instance, or using dictionary-like ``pop()`` or ``del`` calls with the command ID.

core/src/toga/command.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
 from collections.abc import Iterator
-from typing import TYPE_CHECKING, Protocol
+from typing import TYPE_CHECKING, MutableMapping, MutableSet, Protocol
 
 from toga.handlers import wrapped_handler
 from toga.icons import Icon
@@ -325,7 +325,7 @@ def __call__(self, **kwargs) -> object:
         """
 
 
-class CommandSet:
+class CommandSet(MutableSet[Command], MutableMapping[str, Command]):
     def __init__(
         self,
         on_change: CommandSetChangeHandler | None = None,
@@ -392,6 +392,25 @@ def __contains__(self, obj: str | Command) -> Command:
     def __getitem__(self, id: str) -> Command:
         return self._commands[id]
 
+    def __setitem__(self, id: str, command: Command) -> Command:
+        if id != command.id:
+            raise ValueError(f"Command has id {command.id!r}; can't add as {id!r}")
+
+        self.add(command)
+
+    def __delitem__(self, id: str) -> Command:
+        del self._commands[id]
+        if self.on_change:
+            self.on_change()
+
+    def discard(self, command: Command):
+        try:
+            self._commands.pop(command.id)
+            if self.on_change:
+                self.on_change()
+        except KeyError:
+            pass
+
     def __len__(self) -> int:
         return len(self._commands)
 

core/tests/command/test_commandset.py

@@ -127,6 +127,50 @@ def test_add_clear_with_app(app, change_handler):
     assert list(app.commands) == [cmd_a, cmd1b, cmd2, cmd1a, cmd_b]
 
 
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_add_by_existing_id(change_handler):
+    """Commands can be added by ID."""
+    change_handler = Mock()
+    cs = CommandSet(on_change=change_handler)
+
+    # Define a command with an ID
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+
+    # Install a command without an ID
+    cs["custom-command-a"] = cmd_a
+
+    # The command can be retrieved by ID or instance
+    assert "custom-command-a" in cs
+    assert cmd_a in cs
+    assert cs["custom-command-a"] == cmd_a
+    # Change handler was invoked
+    change_handler.assert_called_once_with()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_add_by_different_id(change_handler):
+    """If a command is added using a different ID, an error is raised."""
+    change_handler = Mock()
+    cs = CommandSet(on_change=change_handler)
+
+    # Define a command with an ID
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+
+    # Install a command with a different ID:
+    with pytest.raises(
+        ValueError,
+        match=r"Command has id 'custom-command-a'; can't add as 'new-id'",
+    ):
+        cs["new-id"] = cmd_a
+
+    # The command can be retrieved by ID or instance
+    assert "new-id" not in cs
+    assert "custom-command-a" not in cs
+    assert cmd_a not in cs
+    # Change handler was not invoked
+    change_handler.assert_not_called()
+
+
 def test_retrieve_by_id(app):
     """Commands can be retrieved by ID."""
 
@@ -153,6 +197,146 @@ def test_retrieve_by_id(app):
     assert app.commands[toga.Command.ABOUT].text == "About Test App"
 
 
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_delitem(change_handler):
+    """A command can be deleted by ID."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # Delete one of the commands
+    del cs["custom-command-a"]
+
+    # The deleted command is no longer in the command set.
+    assert "custom-command-a" not in cs
+    assert cmd_a not in cs
+    # Change handler was invoked
+    if change_handler:
+        change_handler.assert_called_once_with()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_delitem_missing(change_handler):
+    """If an ID doesn't exist, delitem raises an error."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # Try to delete a command that doesn't exist
+    with pytest.raises(KeyError, match=r"does-not-exist"):
+        del cs["does-not-exist"]
+
+    # The deleted command is no longer in the command set.
+    assert "custom-command-a" in cs
+    assert cmd_a in cs
+    # Change handler was invoked
+    if change_handler:
+        change_handler.assert_not_called()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_discard(change_handler):
+    """A command can be discarded."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # discard one of the commands
+    cs.discard(cmd_a)
+
+    # The discarded command is no longer in the command set.
+    assert "custom-command-a" not in cs
+    assert cmd_a not in cs
+    # Change handler was invoked
+    if change_handler:
+        change_handler.assert_called_once_with()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_discard_missing(change_handler):
+    """If a command doesn't exist, discard is an no-op."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # Define a third command that isn't added.
+    cmd_c = toga.Command(None, text="App command c", id="custom-command-c")
+
+    # Try to discard a command that doesn't exist; this is a no-op
+    cs.discard(cmd_c)
+
+    # Change handler was not invoked
+    if change_handler:
+        change_handler.assert_not_called()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_remove(change_handler):
+    """A command can be removed from a commandset."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # Remove one of the commands
+    cs.remove(cmd_a)
+
+    # The removed command is no longer in the command set.
+    assert "custom-command-a" not in cs
+    assert cmd_a not in cs
+    # Change handler was invoked
+    if change_handler:
+        change_handler.assert_called_once_with()
+
+
+@pytest.mark.parametrize("change_handler", [(None), (Mock())])
+def test_remove_missing(change_handler):
+    """If a command doesn't exist, remove raises an error."""
+    cs = CommandSet(on_change=change_handler)
+
+    # Define some commands
+    cmd_a = toga.Command(None, text="App command a", id="custom-command-a")
+    cmd_b = toga.Command(None, text="App command b", id="custom-command-b")
+    cs.add(cmd_a, cmd_b)
+    if change_handler:
+        change_handler.reset_mock()
+
+    # Define a third command that isn't added.
+    cmd_c = toga.Command(None, text="App command c", id="custom-command-c")
+
+    # Try to remove a command that doesn't exist
+    with pytest.raises(KeyError, match=str(cmd_c)):
+        cs.remove(cmd_c)
+
+    # Change handler was not invoked
+    if change_handler:
+        change_handler.assert_not_called()
+
+
 def test_default_command_ordering(app):
     """The default app commands are in a known order."""
 

docs/reference/api/resources/command.rst

@@ -78,6 +78,17 @@ defined; if no ID is provided, a random ID will be generated for the Command. Th
 identifier can be used to retrieve a command from :any:`toga.App.commands` and
 :any:`toga.MainWindow.toolbar`.
 
+Commands can be removed using set-like and dictionary-like APIs. The set-like APIs use
+the command instance; the dictionary-like APIs use the command ID:
+
+.. code-block:: python
+
+    # Remove the app using the instance
+    app.commands.remove(cmd_1)
+
+    # Remove a command by ID
+    del app.commands["Some-Command-ID"]
+
 Reference
 ---------