From 539161df10bb64b449607ef1348cd1e680b4744b Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 15 Jul 2024 10:43:19 +0800 Subject: [PATCH 1/4] Make CommandSet satisfy MutableSet and MutableMapping interfaces. --- core/src/toga/command.py | 23 +++- core/tests/command/test_commandset.py | 184 ++++++++++++++++++++++++++ 2 files changed, 205 insertions(+), 2 deletions(-) diff --git a/core/src/toga/command.py b/core/src/toga/command.py index 78924c99c2..c3bf83c7ce 100644 --- a/core/src/toga/command.py +++ b/core/src/toga/command.py @@ -1,6 +1,6 @@ from __future__ import annotations -from collections.abc import Iterator +from collections.abc import Iterator, MutableMapping, MutableSet from typing import TYPE_CHECKING, Protocol from toga.handlers import wrapped_handler @@ -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) diff --git a/core/tests/command/test_commandset.py b/core/tests/command/test_commandset.py index 2aa3b11d77..89993e4b26 100644 --- a/core/tests/command/test_commandset.py +++ b/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.""" From 21b0e0867c2a42be966425d82c5471764764ae3f Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 15 Jul 2024 10:43:32 +0800 Subject: [PATCH 2/4] Add changenote. --- changes/2701.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/2701.feature.rst diff --git a/changes/2701.feature.rst b/changes/2701.feature.rst new file mode 100644 index 0000000000..63dc7a0328 --- /dev/null +++ b/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. From 0f8095023dc67e4a9d9962cddf39ee3cdb8c9524 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 15 Jul 2024 10:48:30 +0800 Subject: [PATCH 3/4] Add documentation for removal APIs. --- docs/reference/api/resources/command.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/reference/api/resources/command.rst b/docs/reference/api/resources/command.rst index 5a4b6c0870..80d0a2ea28 100644 --- a/docs/reference/api/resources/command.rst +++ b/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 --------- From 6e7a2ddd922a6ae4cc560d73fc07d571e5d2125f Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 15 Jul 2024 11:05:49 +0800 Subject: [PATCH 4/4] Use imports from typing for Python3.8 compatibility. --- core/src/toga/command.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/toga/command.py b/core/src/toga/command.py index c3bf83c7ce..2430849a91 100644 --- a/core/src/toga/command.py +++ b/core/src/toga/command.py @@ -1,7 +1,7 @@ from __future__ import annotations -from collections.abc import Iterator, MutableMapping, MutableSet -from typing import TYPE_CHECKING, Protocol +from collections.abc import Iterator +from typing import TYPE_CHECKING, MutableMapping, MutableSet, Protocol from toga.handlers import wrapped_handler from toga.icons import Icon