From 87a3e5d379ea538d8e939269fa85c7f7343d6b27 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 5 Nov 2024 00:39:43 -0500 Subject: [PATCH 1/2] Change CommandSet._cmd to a read-only property which never returns None. This addresses issue where CommandSet methods which only run after it is registered have to either check if self._cmd is None or cast it to the CLI class to avoid type checker errors. --- cmd2/command_definition.py | 32 ++++++++++++++++--- .../test_commandset/test_commandset.py | 8 ++++- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/cmd2/command_definition.py b/cmd2/command_definition.py index 9f713d83..a9b580b8 100644 --- a/cmd2/command_definition.py +++ b/cmd2/command_definition.py @@ -92,10 +92,31 @@ class CommandSet(object): """ def __init__(self) -> None: - self._cmd: Optional[cmd2.Cmd] = None + # Private reference to the CLI instance in which this CommandSet running. + # This will be set when the CommandSet is registered and it should be + # accessed by child classes using the self._cmd property. + self.__private_cmd: Optional[cmd2.Cmd] = None + self._settables: Dict[str, Settable] = {} self._settable_prefix = self.__class__.__name__ + @property + def _cmd(self) -> 'cmd2.Cmd': + """ + Property for child classes to access self.__private_cmd. + + Using this property ensures that self.__private_cmd has been set + and it tells type checkers that it's no longer a None type. + + Override this property if you need to change its return type to a + child class of Cmd. + + :raises: CommandSetRegistrationError if CommandSet is not registered. + """ + if self.__private_cmd is None: + raise CommandSetRegistrationError('This CommandSet is not registered') + return self.__private_cmd + def on_register(self, cmd: 'cmd2.Cmd') -> None: """ Called by cmd2.Cmd as the first step to registering a CommandSet. The commands defined in this class have @@ -103,9 +124,10 @@ def on_register(self, cmd: 'cmd2.Cmd') -> None: requiring access to the Cmd object (e.g. configure commands and their parsers based on CLI state data). :param cmd: The cmd2 main application + :raises: CommandSetRegistrationError if CommandSet is already registered. """ - if self._cmd is None: - self._cmd = cmd + if self.__private_cmd is None: + self.__private_cmd = cmd else: raise CommandSetRegistrationError('This CommandSet has already been registered') @@ -129,7 +151,7 @@ def on_unregistered(self) -> None: Called by ``cmd2.Cmd`` after a CommandSet has been unregistered and all its commands removed from the CLI. Subclasses can override this to perform remaining cleanup steps. """ - self._cmd = None + self.__private_cmd = None @property def settable_prefix(self) -> str: @@ -145,7 +167,7 @@ def add_settable(self, settable: Settable) -> None: :param settable: Settable object being added """ - if self._cmd: + if self.__private_cmd is not None: if not self._cmd.always_prefix_settables: if settable.name in self._cmd.settables.keys() and settable.name not in self._settables.keys(): raise KeyError(f'Duplicate settable: {settable.name}') diff --git a/tests_isolated/test_commandset/test_commandset.py b/tests_isolated/test_commandset/test_commandset.py index c7293d41..7bbe06c4 100644 --- a/tests_isolated/test_commandset/test_commandset.py +++ b/tests_isolated/test_commandset/test_commandset.py @@ -151,8 +151,14 @@ def test_autoload_commands(command_sets_app): def test_custom_construct_commandsets(): - # Verifies that a custom initialized CommandSet loads correctly when passed into the constructor command_set_b = CommandSetB('foo') + + # Verify that _cmd cannot be accessed until CommandSet is registered. + with pytest.raises(CommandSetRegistrationError) as excinfo: + command_set_b._cmd.poutput("test") + assert "is not registered" in str(excinfo.value) + + # Verifies that a custom initialized CommandSet loads correctly when passed into the constructor app = WithCommandSets(command_sets=[command_set_b]) cmds_cats, cmds_doc, cmds_undoc, help_topics = app._build_command_info() From aae5f384446514448a4fccee0e89ad1d5977c6e4 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 5 Nov 2024 08:39:23 -0500 Subject: [PATCH 2/2] Rename CommandSet.__private_cmd to CommandSet.__cmd_internal. --- cmd2/command_definition.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd2/command_definition.py b/cmd2/command_definition.py index a9b580b8..1c1ff0cf 100644 --- a/cmd2/command_definition.py +++ b/cmd2/command_definition.py @@ -95,7 +95,7 @@ def __init__(self) -> None: # Private reference to the CLI instance in which this CommandSet running. # This will be set when the CommandSet is registered and it should be # accessed by child classes using the self._cmd property. - self.__private_cmd: Optional[cmd2.Cmd] = None + self.__cmd_internal: Optional[cmd2.Cmd] = None self._settables: Dict[str, Settable] = {} self._settable_prefix = self.__class__.__name__ @@ -103,9 +103,9 @@ def __init__(self) -> None: @property def _cmd(self) -> 'cmd2.Cmd': """ - Property for child classes to access self.__private_cmd. + Property for child classes to access self.__cmd_internal. - Using this property ensures that self.__private_cmd has been set + Using this property ensures that self.__cmd_internal has been set and it tells type checkers that it's no longer a None type. Override this property if you need to change its return type to a @@ -113,9 +113,9 @@ def _cmd(self) -> 'cmd2.Cmd': :raises: CommandSetRegistrationError if CommandSet is not registered. """ - if self.__private_cmd is None: + if self.__cmd_internal is None: raise CommandSetRegistrationError('This CommandSet is not registered') - return self.__private_cmd + return self.__cmd_internal def on_register(self, cmd: 'cmd2.Cmd') -> None: """ @@ -126,8 +126,8 @@ def on_register(self, cmd: 'cmd2.Cmd') -> None: :param cmd: The cmd2 main application :raises: CommandSetRegistrationError if CommandSet is already registered. """ - if self.__private_cmd is None: - self.__private_cmd = cmd + if self.__cmd_internal is None: + self.__cmd_internal = cmd else: raise CommandSetRegistrationError('This CommandSet has already been registered') @@ -151,7 +151,7 @@ def on_unregistered(self) -> None: Called by ``cmd2.Cmd`` after a CommandSet has been unregistered and all its commands removed from the CLI. Subclasses can override this to perform remaining cleanup steps. """ - self.__private_cmd = None + self.__cmd_internal = None @property def settable_prefix(self) -> str: @@ -167,7 +167,7 @@ def add_settable(self, settable: Settable) -> None: :param settable: Settable object being added """ - if self.__private_cmd is not None: + if self.__cmd_internal is not None: if not self._cmd.always_prefix_settables: if settable.name in self._cmd.settables.keys() and settable.name not in self._settables.keys(): raise KeyError(f'Duplicate settable: {settable.name}')