Skip to content

Commit

Permalink
Change CommandSet._cmd to a read-only property which never returns None.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kmvanbrunt committed Nov 5, 2024
1 parent f85e7fc commit b2c1908
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
32 changes: 27 additions & 5 deletions cmd2/command_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,42 @@ 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.__internal_cmd.
Using this property ensures that self.__internal_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
not been added to the CLI object at this point. Subclasses can override this to perform any initialization
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')

Expand All @@ -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:
Expand All @@ -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}')
Expand Down
8 changes: 7 additions & 1 deletion tests_isolated/test_commandset/test_commandset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit b2c1908

Please sign in to comment.