From 2a002fb70095601d3580069b1ed75c67b08bd6ca Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 21 Apr 2026 16:08:46 -0400 Subject: [PATCH 1/5] Making more of the subcommand management API public. --- cmd2/argparse_utils.py | 88 +++++++++++++++++++++--------- cmd2/cmd2.py | 2 +- examples/scripts/save_help_text.py | 2 +- tests/test_argparse_utils.py | 49 +++++++++++++++-- tests/test_cmd2.py | 4 +- 5 files changed, 110 insertions(+), 35 deletions(-) diff --git a/cmd2/argparse_utils.py b/cmd2/argparse_utils.py index a4219e35a..c01086401 100644 --- a/cmd2/argparse_utils.py +++ b/cmd2/argparse_utils.py @@ -494,6 +494,52 @@ def _ActionsContainer_add_argument( # noqa: N802 # Overwrite _ActionsContainer.add_argument with our patch argparse._ActionsContainer.add_argument = _ActionsContainer_add_argument # type: ignore[method-assign] +############################################################################################################ +# Patch argparse._SubParsersAction by adding remove_parser() function +############################################################################################################ + + +def _SubParsersAction_remove_parser( # noqa: N802 + self: argparse._SubParsersAction, # type: ignore[type-arg] + name: str, +) -> argparse.ArgumentParser: + """Remove a subparser from a subparsers group. + + This function is added by cmd2 as a method called ``remove_parser()`` + to ``argparse._SubParsersAction`` class. + + To call: ``action.remove_parser(name)`` + + :param self: instance of the _SubParsersAction being edited + :param name: name of the subcommand for the subparser to remove + :return: the removed parser + :raises ValueError: if the subcommand doesn't exist + """ + if name not in self._name_parser_map: + raise ValueError(f"Subcommand '{name}' not found") + + subparser = self._name_parser_map[name] + + # Find all names (primary and aliases) that map to this subparser + all_names = [cur_name for cur_name, cur_parser in self._name_parser_map.items() if cur_parser is subparser] + + # Remove the help entry for this subparser. To handle the case where + # name is an alias, we remove the action whose 'dest' matches any of + # the names mapped to this subparser. + for choice_action in self._choices_actions: + if choice_action.dest in all_names: + self._choices_actions.remove(choice_action) + break + + # Remove all references to this subparser, including aliases. + for cur_name in all_names: + del self._name_parser_map[cur_name] + + return cast(argparse.ArgumentParser, subparser) + + +argparse._SubParsersAction.remove_parser = _SubParsersAction_remove_parser # type: ignore[attr-defined] + class Cmd2ArgumentParser(argparse.ArgumentParser): """Custom ArgumentParser class that improves error and help output.""" @@ -556,7 +602,7 @@ def __init__( self.description: RenderableType | None # type: ignore[assignment] self.epilog: RenderableType | None # type: ignore[assignment] - def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": + def get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": """Get the _SubParsersAction for this parser if it exists. :return: the _SubParsersAction for this parser @@ -619,7 +665,7 @@ def update_prog(self, prog: str) -> None: self.prog = prog try: - subparsers_action = self._get_subparsers_action() + subparsers_action = self.get_subparsers_action() except ValueError: # This parser has no subcommands return @@ -651,7 +697,7 @@ def update_prog(self, prog: str) -> None: subcmd_parser.update_prog(subcmd_prog) updated_parsers.add(subcmd_parser) - def _find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser": + def find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser": """Find a parser in the hierarchy based on a sequence of subcommand names. :param subcommand_path: sequence of subcommand names leading to the target parser @@ -660,7 +706,7 @@ def _find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser": """ parser = self for name in subcommand_path: - subparsers_action = parser._get_subparsers_action() + subparsers_action = parser.get_subparsers_action() if name not in subparsers_action.choices: raise ValueError(f"Subcommand '{name}' not found in '{parser.prog}'") parser = subparsers_action.choices[name] @@ -691,8 +737,8 @@ def attach_subcommand( f"Received: '{type(subcommand_parser).__name__}'." ) - target_parser = self._find_parser(subcommand_path) - subparsers_action = target_parser._get_subparsers_action() + target_parser = self.find_parser(subcommand_path) + subparsers_action = target_parser.get_subparsers_action() # Verify the parser is compatible with the 'parser_class' configured for this # subcommand group. We use isinstance() here to allow for subclasses, providing @@ -728,28 +774,16 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> :return: the detached parser :raises ValueError: if the command path is invalid or the subcommand doesn't exist """ - target_parser = self._find_parser(subcommand_path) - subparsers_action = target_parser._get_subparsers_action() - - subparser = subparsers_action._name_parser_map.get(subcommand) - if subparser is None: - raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") - - # Remove this subcommand and all its aliases from the base command - to_remove = [] - for cur_name, cur_parser in subparsers_action._name_parser_map.items(): - if cur_parser is subparser: - to_remove.append(cur_name) - for cur_name in to_remove: - del subparsers_action._name_parser_map[cur_name] - - # Remove this subcommand from its base command's help text - for choice_action in subparsers_action._choices_actions: - if choice_action.dest == subcommand: - subparsers_action._choices_actions.remove(choice_action) - break + target_parser = self.find_parser(subcommand_path) + subparsers_action = target_parser.get_subparsers_action() - return subparser + try: + return cast( + Cmd2ArgumentParser, + subparsers_action.remove_parser(subcommand), # type: ignore[attr-defined] + ) + except ValueError: + raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") from None def error(self, message: str) -> NoReturn: """Override that applies custom formatting to the error message.""" diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 0a5880e3a..a92e77448 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1026,7 +1026,7 @@ def _check_uninstallable(self, cmdset: CommandSet[Any]) -> None: def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: try: - subparsers_action = parser._get_subparsers_action() + subparsers_action = parser.get_subparsers_action() except ValueError: # No subcommands to check return diff --git a/examples/scripts/save_help_text.py b/examples/scripts/save_help_text.py index a9e196cc7..e319bf13f 100644 --- a/examples/scripts/save_help_text.py +++ b/examples/scripts/save_help_text.py @@ -14,7 +14,7 @@ def get_sub_commands(parser: Cmd2ArgumentParser) -> list[str]: """Get a list of subcommands for a Cmd2ArgumentParser.""" try: - subparsers_action = parser._get_subparsers_action() + subparsers_action = parser.get_subparsers_action() except ValueError: # No subcommands return [] diff --git a/tests/test_argparse_utils.py b/tests/test_argparse_utils.py index 43c294a8f..0bdf932f6 100644 --- a/tests/test_argparse_utils.py +++ b/tests/test_argparse_utils.py @@ -367,9 +367,9 @@ def test_subcommand_attachment() -> None: # Verify hierarchy navigation ############################### - assert root_parser._find_parser(["child", "grandchild"]) is grandchild_parser - assert root_parser._find_parser(["child"]) is child_parser - assert root_parser._find_parser([]) is root_parser + assert root_parser.find_parser(["child", "grandchild"]) is grandchild_parser + assert root_parser.find_parser(["child"]) is child_parser + assert root_parser.find_parser([]) is root_parser ############################### # Verify attachments @@ -382,6 +382,10 @@ def test_subcommand_attachment() -> None: # Verify grandchild attachment assert child_subparsers._name_parser_map["grandchild"] is grandchild_parser + # Verify help entries are present + assert any(action.dest == "child" for action in root_subparsers._choices_actions) + assert any(action.dest == "grandchild" for action in child_subparsers._choices_actions) + ############################### # Detach subcommands ############################### @@ -390,12 +394,49 @@ def test_subcommand_attachment() -> None: detached_grandchild = root_parser.detach_subcommand(["child"], "grandchild") assert detached_grandchild is grandchild_parser assert "grandchild" not in child_subparsers._name_parser_map + assert not any(action.dest == "grandchild" for action in child_subparsers._choices_actions) # Detach child from root detached_child = root_parser.detach_subcommand([], "child") assert detached_child is child_parser assert "child" not in root_subparsers._name_parser_map assert "child_alias" not in root_subparsers._name_parser_map + assert not any(action.dest == "child" for action in root_subparsers._choices_actions) + + +def test_detach_subcommand_by_alias() -> None: + """Test detaching a subcommand using one of its alias names.""" + root_parser = Cmd2ArgumentParser(prog="root") + root_subparsers = root_parser.add_subparsers() + + child_parser = Cmd2ArgumentParser(prog="child") + root_parser.attach_subcommand( + [], + "child", + child_parser, + help="a child command", + aliases=["alias1", "alias2"], + ) + + # Verify all names map to the parser + assert root_subparsers._name_parser_map["child"] is child_parser + assert root_subparsers._name_parser_map["alias1"] is child_parser + assert root_subparsers._name_parser_map["alias2"] is child_parser + + # Verify help entry is present + assert any(action.dest == "child" for action in root_subparsers._choices_actions) + + # Detach using an alias + detached = root_parser.detach_subcommand([], "alias1") + assert detached is child_parser + + # Verify all names are gone + assert "child" not in root_subparsers._name_parser_map + assert "alias1" not in root_subparsers._name_parser_map + assert "alias2" not in root_subparsers._name_parser_map + + # Verify help entry is gone + assert not any(action.dest == "child" for action in root_subparsers._choices_actions) def test_subcommand_attachment_errors() -> None: @@ -411,7 +452,7 @@ def test_subcommand_attachment_errors() -> None: # Allow subcommands for the next tests root_parser.add_subparsers() - # Verify ValueError when path is invalid (_find_parser() fails) + # Verify ValueError when path is invalid (find_parser() fails) with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"): root_parser.attach_subcommand(["nonexistent"], "anything", child_parser) with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"): diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index a35df53bf..958ac1cd0 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4490,7 +4490,7 @@ def do_root(self, _args: argparse.Namespace) -> None: app.attach_subcommand("root", "child", child_parser, help="child help") # Verify child was attached - root_subparsers_action = root_parser._get_subparsers_action() + root_subparsers_action = root_parser.get_subparsers_action() assert "child" in root_subparsers_action._name_parser_map assert root_subparsers_action._name_parser_map["child"] is child_parser @@ -4499,7 +4499,7 @@ def do_root(self, _args: argparse.Namespace) -> None: app.attach_subcommand("root child", "grandchild", grandchild_parser) # Verify grandchild was attached - child_subparsers_action = child_parser._get_subparsers_action() + child_subparsers_action = child_parser.get_subparsers_action() assert "grandchild" in child_subparsers_action._name_parser_map # Detach grandchild From f216ed4fd19ef19879e1a6d5610b6650dbb93130 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 21 Apr 2026 17:06:49 -0400 Subject: [PATCH 2/5] Make Cmd._command_parsers and Cmd._get_root_parser_and_subcmd_path() public. --- cmd2/cmd2.py | 28 ++++++++++++++++------------ cmd2/decorators.py | 2 +- docs/features/argument_processing.md | 2 +- examples/scripts/save_help_text.py | 2 +- tests/test_cmd2.py | 10 +++++----- tests/test_commandset.py | 10 +++++----- 6 files changed, 29 insertions(+), 25 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index a92e77448..615915312 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -223,13 +223,17 @@ def __init__(self) -> None: DisabledCommand = namedtuple("DisabledCommand", ["command_function", "help_function", "completer_function"]) # noqa: PYI024 -class _CommandParsers: +class CommandParsers: """Create and store all command method argument parsers for a given Cmd instance. Parser creation and retrieval are accomplished through the get() method. """ def __init__(self, cmd: "Cmd") -> None: + """Initialize CommandParsers. + + :param cmd: the Cmd instance whose parsers are being managed + """ self._cmd = cmd # Keyed by the fully qualified method names. This is more reliable than @@ -620,7 +624,7 @@ def __init__( self.disabled_categories: dict[str, str] = {} # Command parsers for this Cmd instance. - self._command_parsers: _CommandParsers = _CommandParsers(self) + self.command_parsers: CommandParsers = CommandParsers(self) # Members related to printing asynchronous alerts self._alert_queue: deque[AsyncAlert] = deque() @@ -935,7 +939,7 @@ def _install_command_function(self, command_func_name: str, command_method: Boun if not command_func_name.startswith(COMMAND_FUNC_PREFIX): raise CommandSetRegistrationError(f"{command_func_name} does not begin with '{COMMAND_FUNC_PREFIX}'") - # command_method must start with COMMAND_FUNC_PREFIX for use in self._command_parsers. + # command_method must start with COMMAND_FUNC_PREFIX for use in self.command_parsers. if not command_method.__name__.startswith(COMMAND_FUNC_PREFIX): raise CommandSetRegistrationError(f"{command_method.__name__} does not begin with '{COMMAND_FUNC_PREFIX}'") @@ -1009,7 +1013,7 @@ def unregister_command_set(self, cmdset: CommandSet[Any]) -> None: # Only remove the parser if this is the actual # command since command synonyms don't own it. if cmd_func_name == command_method.__name__: - self._command_parsers.remove(command_method) + self.command_parsers.remove(command_method) if hasattr(self, COMPLETER_FUNC_PREFIX + command): delattr(self, COMPLETER_FUNC_PREFIX + command) @@ -1059,7 +1063,7 @@ def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: # We only need to check if it's safe to remove the parser if this # is the actual command since command synonyms don't own it. if cmd_func_name == command_method.__name__: - command_parser = self._command_parsers.get(command_method) + command_parser = self.command_parsers.get(command_method) if command_parser is not None: check_parser_uninstallable(command_parser) @@ -1140,7 +1144,7 @@ def _unregister_subcommands(self, cmdset: CmdOrSet) -> None: with contextlib.suppress(ValueError): self.detach_subcommand(full_command_name, subcommand_name) - def _get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentParser, list[str]]: + def get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentParser, list[str]]: """Tokenize a command string and resolve the associated root parser and relative subcommand path. This helper handles the initial resolution of a command string (e.g., 'foo bar baz') by @@ -1170,7 +1174,7 @@ def _get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentPa if command_func is None: raise ValueError(f"Root command '{root_command}' not found") - root_parser = self._command_parsers.get(command_func) + root_parser = self.command_parsers.get(command_func) if root_parser is None: raise ValueError(f"Command '{root_command}' does not use argparse") @@ -1195,7 +1199,7 @@ def attach_subcommand( 2. The parser_class configured for the target subcommand group :raises ValueError: if the command path is invalid or doesn't support subcommands """ - root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) + root_parser, subcommand_path = self.get_root_parser_and_subcmd_path(command) root_parser.attach_subcommand(subcommand_path, subcommand, subcommand_parser, **add_parser_kwargs) def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser: @@ -1207,7 +1211,7 @@ def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser :return: the detached parser :raises ValueError: if the command path is invalid or the subcommand doesn't exist """ - root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) + root_parser, subcommand_path = self.get_root_parser_and_subcmd_path(command) return root_parser.detach_subcommand(subcommand_path, subcommand) @property @@ -2448,7 +2452,7 @@ def _perform_completion( else: # There's no completer function, next see if the command uses argparse func = self.get_command_func(command) - argparser = None if func is None else self._command_parsers.get(func) + argparser = None if func is None else self.command_parsers.get(func) if func is not None and argparser is not None: # Get arguments for complete() @@ -4218,7 +4222,7 @@ def complete_help_subcommands( return Completions() # Check if this command uses argparse - if (func := self.get_command_func(command)) is None or (argparser := self._command_parsers.get(func)) is None: + if (func := self.get_command_func(command)) is None or (argparser := self.command_parsers.get(func)) is None: return Completions() completer = argparse_completer.DEFAULT_AP_COMPLETER(argparser, self) @@ -4312,7 +4316,7 @@ def do_help(self, args: argparse.Namespace) -> None: # Getting help for a specific command func = self.get_command_func(args.command) help_func = getattr(self, constants.HELP_FUNC_PREFIX + args.command, None) - argparser = None if func is None else self._command_parsers.get(func) + argparser = None if func is None else self.command_parsers.get(func) # If the command function uses argparse, then use argparse's help if func is not None and argparser is not None: diff --git a/cmd2/decorators.py b/cmd2/decorators.py index 9561bc997..204161b44 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -287,7 +287,7 @@ def cmd_wrapper(*args: Any, **kwargs: Any) -> bool | None: ) # Pass cmd_wrapper instead of func, since it contains the parser info. - arg_parser = cmd2_app._command_parsers.get(cmd_wrapper) + arg_parser = cmd2_app.command_parsers.get(cmd_wrapper) if arg_parser is None: # This shouldn't be possible to reach raise ValueError(f"No argument parser found for {command_name}") # pragma: no cover diff --git a/docs/features/argument_processing.md b/docs/features/argument_processing.md index b18aabe1c..a0a577380 100644 --- a/docs/features/argument_processing.md +++ b/docs/features/argument_processing.md @@ -50,7 +50,7 @@ stores internally. A consequence is that parsers don't need to be unique across Since the `@with_argparser` decorator is making a deep-copy of the parser provided, if you wish to dynamically modify this parser at a later time, you need to retrieve this deep copy. This can - be done using `self._command_parsers.get(self.do_commandname)`. + be done using `self.command_parsers.get(self.do_commandname)`. ## Argument Parsing diff --git a/examples/scripts/save_help_text.py b/examples/scripts/save_help_text.py index e319bf13f..be792d2c9 100644 --- a/examples/scripts/save_help_text.py +++ b/examples/scripts/save_help_text.py @@ -88,7 +88,7 @@ def main() -> None: continue cmd_func = self.get_command_func(item) - parser = self._command_parsers.get(cmd_func) + parser = self.command_parsers.get(cmd_func) if parser is None: continue diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 958ac1cd0..de4c176c4 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4115,10 +4115,10 @@ def test_startup_script_with_odd_file_names(startup_script) -> None: def test_command_parser_retrieval(outsim_app: cmd2.Cmd) -> None: # Pass something that isn't a method not_a_method = "just a string" - assert outsim_app._command_parsers.get(not_a_method) is None + assert outsim_app.command_parsers.get(not_a_method) is None # Pass a non-command method - assert outsim_app._command_parsers.get(outsim_app.__init__) is None + assert outsim_app.command_parsers.get(outsim_app.__init__) is None def test_command_synonym_parser() -> None: @@ -4128,8 +4128,8 @@ class SynonymApp(cmd2.cmd2.Cmd): app = SynonymApp() - synonym_parser = app._command_parsers.get(app.do_synonym) - help_parser = app._command_parsers.get(app.do_help) + synonym_parser = app.command_parsers.get(app.do_synonym) + help_parser = app.command_parsers.get(app.do_help) assert synonym_parser is not None assert synonym_parser is help_parser @@ -4481,7 +4481,7 @@ def do_root(self, _args: argparse.Namespace) -> None: app = SubcmdApp() # Verify root exists and uses argparse - root_parser = app._command_parsers.get(app.do_root) + root_parser = app.command_parsers.get(app.do_root) assert root_parser is not None # Attach child to root diff --git a/tests/test_commandset.py b/tests/test_commandset.py index 1beba737b..72d31e52e 100644 --- a/tests/test_commandset.py +++ b/tests/test_commandset.py @@ -173,13 +173,13 @@ def do_builtin(self, _) -> None: app = WithCommandSets(command_sets=[cs]) # Make sure the synonyms have the same parser as what they alias - builtin_parser = app._command_parsers.get(app.do_builtin) - builtin_synonym_parser = app._command_parsers.get(app.do_builtin_synonym) + builtin_parser = app.command_parsers.get(app.do_builtin) + builtin_synonym_parser = app.command_parsers.get(app.do_builtin_synonym) assert builtin_parser is not None assert builtin_parser is builtin_synonym_parser - alias_parser = app._command_parsers.get(cmd2.Cmd.do_alias) - alias_synonym_parser = app._command_parsers.get(app.do_alias_synonym) + alias_parser = app.command_parsers.get(cmd2.Cmd.do_alias) + alias_synonym_parser = app.command_parsers.get(app.do_alias_synonym) assert alias_parser is not None assert alias_parser is alias_synonym_parser @@ -190,7 +190,7 @@ def do_builtin(self, _) -> None: assert not hasattr(app, "do_alias_synonym") # Make sure the alias command still exists, has the same parser, and works. - assert alias_parser is app._command_parsers.get(cmd2.Cmd.do_alias) + assert alias_parser is app.command_parsers.get(cmd2.Cmd.do_alias) out, _err = run_cmd(app, "alias --help") assert normalize(alias_parser.format_help())[0] in out From 9416464f4d0f2115883788d6151c140914ce3e5a Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 21 Apr 2026 19:29:18 -0400 Subject: [PATCH 3/5] Made is so command_parsers.get() returns parsers for disabled commands. --- cmd2/cmd2.py | 67 +++++++++++++++++++++++++++++----------- tests/test_cmd2.py | 77 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 19 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 615915312..e8587eb26 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -617,10 +617,12 @@ def __init__( # Commands disabled during specific application states # Key: Command name | Value: DisabledCommand object + # NOTE: Use disable_command() and enable_command() to modify this dictionary. self.disabled_commands: dict[str, DisabledCommand] = {} # Categories of commands to be disabled # Key: Category name | Value: Message to display + # NOTE: Use disable_category() and enable_category() to modify this dictionary. self.disabled_categories: dict[str, str] = {} # Command parsers for this Cmd instance. @@ -1148,9 +1150,8 @@ def get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentPar """Tokenize a command string and resolve the associated root parser and relative subcommand path. This helper handles the initial resolution of a command string (e.g., 'foo bar baz') by - identifying 'foo' as the root command (even if disabled), retrieving its associated - parser, and returning any remaining tokens (['bar', 'baz']) as a path relative - to that parser for further traversal. + identifying 'foo' as the root command, retrieving its associated parser, and returning + any remaining tokens (['bar', 'baz']) as a path relative to that parser for further traversal. :param command: full space-delimited command path leading to a parser (e.g. 'foo' or 'foo bar') :return: a tuple containing the Cmd2ArgumentParser for the root command and a list of @@ -1166,11 +1167,7 @@ def get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentPar subcommand_path = tokens[1:] # Search for the base command function and verify it has an argparser defined - if root_command in self.disabled_commands: - command_func = self.disabled_commands[root_command].command_function - else: - command_func = self.get_command_func(root_command) - + command_func = self.get_command_func(root_command) if command_func is None: raise ValueError(f"Root command '{root_command}' not found") @@ -4314,8 +4311,21 @@ def do_help(self, args: argparse.Namespace) -> None: else: # Getting help for a specific command - func = self.get_command_func(args.command) + disabled = args.command in self.disabled_commands help_func = getattr(self, constants.HELP_FUNC_PREFIX + args.command, None) + + # If the command is disabled, then call the help function which was + # overwritten by disable_command() to print the disabled message. + if disabled: + if help_func is not None: + help_func() + else: + # This is a defensive fallback in case someone disabled a command + # without using disable_command() resulting in no help function. + self._report_disabled_command_usage(message_to_print=f"{args.command} is currently disabled.") + return + + func = self.get_command_func(args.command) argparser = None if func is None else self.command_parsers.get(func) # If the command function uses argparse, then use argparse's help @@ -5652,15 +5662,27 @@ def disable_command(self, command: str, message_to_print: str) -> None: completer_function=getattr(self, completer_func_name, None), ) - # Overwrite the command and help functions to print the message - new_func = functools.partial( - self._report_disabled_command_usage, message_to_print=message_to_print.replace(constants.COMMAND_NAME, command) - ) - setattr(self, cmd_func_name, new_func) - setattr(self, help_func_name, new_func) + # Overwrite command function to print the message + message_to_print = message_to_print.replace(constants.COMMAND_NAME, command) + new_cmd_func = functools.partial(self._report_disabled_command_usage, message_to_print=message_to_print) - # Set the completer to a function that returns a blank list - setattr(self, completer_func_name, lambda *_args, **_kwargs: []) + # Preserve the metadata of the original command function + functools.update_wrapper(new_cmd_func, command_function) + setattr(self, cmd_func_name, new_cmd_func) + + # Overwrite the help function to print the message + new_help_func = functools.partial(self._report_disabled_command_usage, message_to_print=message_to_print) + if (help_function := self.disabled_commands[command].help_function) is not None: + # Preserve the metadata of the original help function + functools.update_wrapper(new_help_func, help_function) + setattr(self, help_func_name, new_help_func) + + # Set the completer to a function that returns a nothing + new_completer_func = functools.partial(self._disabled_completer) + if (completer_function := self.disabled_commands[command].completer_function) is not None: + # Preserve the metadata of the original completer function + functools.update_wrapper(new_completer_func, completer_function) + setattr(self, completer_func_name, new_completer_func) def disable_category(self, category: str, message_to_print: str) -> None: """Disable an entire category of commands. @@ -5685,7 +5707,7 @@ def disable_category(self, category: str, message_to_print: str) -> None: self.disabled_categories[category] = message_to_print def _report_disabled_command_usage(self, *_args: Any, message_to_print: str, **_kwargs: Any) -> None: - """Report when a disabled command has been run or had help called on it. + """Report when a disabled command or its help function is run. :param _args: not used :param message_to_print: the message reporting that the command is disabled @@ -5693,6 +5715,15 @@ def _report_disabled_command_usage(self, *_args: Any, message_to_print: str, **_ """ self.perror(message_to_print, style=None) + def _disabled_completer(self, *_args: Any, **_kwargs: Any) -> Completions: + """Completer function for a disabled command. + + :param _args: not used + :param _kwargs: not used + :return: an empty Completions object + """ + return Completions() + def cmdloop(self, intro: RenderableType = "") -> int: """Deal with extra features provided by cmd2, this is an outer wrapper around _cmdloop(). diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index de4c176c4..ccd3fc569 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -3837,6 +3837,10 @@ def do_is_not_decorated(self, arg) -> None: """This will be in the DEFAULT_CATEGORY.""" self.poutput("The real is_not_decorated") + @cmd2.with_argparser(cmd2.Cmd2ArgumentParser()) + def do_argparse_command(self, args) -> None: + """Help for argparse_command""" + class DisableCommandSet(CommandSet[cmd2.Cmd]): """Test registering a command which is in a disabled category""" @@ -4008,7 +4012,50 @@ def test_disabled_command_not_in_history(disable_commands_app) -> None: assert saved_len == len(disable_commands_app.history) -def test_disabled_message_command_name(disable_commands_app) -> None: +def test_get_parser_while_disabled(disable_commands_app: DisableCommandsApp) -> None: + # Get parser before disabling + parser_before = disable_commands_app.command_parsers.get(disable_commands_app.do_argparse_command) + assert parser_before is not None + + # Disable command + disable_commands_app.disable_command("argparse_command", "Disabled") + + # Get parser after disabling - this is what was failing (returning None) + parser_after = disable_commands_app.command_parsers.get(disable_commands_app.do_argparse_command) + assert parser_after is not None + assert parser_after is parser_before + + +def test_metadata_preservation_while_disabled(disable_commands_app: DisableCommandsApp) -> None: + orig_cmd_func = disable_commands_app.do_has_helper_func + orig_help = disable_commands_app.help_has_helper_func + orig_complete = disable_commands_app.complete_has_helper_func + + disable_commands_app.disable_command("has_helper_func", "Disabled") + + # Names and qualnames should be preserved + assert disable_commands_app.do_has_helper_func.__name__ == orig_cmd_func.__name__ + assert disable_commands_app.do_has_helper_func.__qualname__ == orig_cmd_func.__qualname__ + + assert disable_commands_app.help_has_helper_func.__name__ == orig_help.__name__ + assert disable_commands_app.help_has_helper_func.__qualname__ == orig_help.__qualname__ + + assert disable_commands_app.complete_has_helper_func.__name__ == orig_complete.__name__ + assert disable_commands_app.complete_has_helper_func.__qualname__ == orig_complete.__qualname__ + + # Docstrings should be preserved + assert disable_commands_app.do_has_helper_func.__doc__ == orig_cmd_func.__doc__ + assert disable_commands_app.help_has_helper_func.__doc__ == orig_help.__doc__ + assert disable_commands_app.complete_has_helper_func.__doc__ == orig_complete.__doc__ + + +def test_disabled_completer_returns_empty(disable_commands_app: DisableCommandsApp) -> None: + disable_commands_app.disable_command("has_helper_func", "Disabled") + completions = disable_commands_app.complete_has_helper_func("", "has_helper_func ", 16, 16) + assert len(completions) == 0 + + +def test_disabled_message_command_name(disable_commands_app: DisableCommandsApp) -> None: message_to_print = f"{COMMAND_NAME} is currently disabled" disable_commands_app.disable_command("has_helper_func", message_to_print) @@ -4016,6 +4063,34 @@ def test_disabled_message_command_name(disable_commands_app) -> None: assert err[0].startswith("has_helper_func is currently disabled") +def test_help_argparse_command_while_disabled(disable_commands_app: DisableCommandsApp) -> None: + message_to_print = "This command is disabled" + disable_commands_app.disable_command("argparse_command", message_to_print) + + # help should show the disabled message + _out, err = run_cmd(disable_commands_app, "help argparse_command") + assert err[0].startswith(message_to_print) + + # Re-enabling should restore the real help + disable_commands_app.enable_command("argparse_command") + out, _err = run_cmd(disable_commands_app, "help argparse_command") + assert "Usage: argparse_command" in out[0] + + +def test_help_disabled_no_help_func(base_app: cmd2.Cmd) -> None: + from cmd2.cmd2 import DisabledCommand + + # Manually disable a command without a help function to trigger the defensive fallback + command = "quit" + command_func = base_app.get_command_func(command) + base_app.disabled_commands[command] = DisabledCommand( + command_function=command_func, help_function=None, completer_function=None + ) + + _out, err = run_cmd(base_app, f"help {command}") + assert err[0].startswith(f"{command} is currently disabled.") + + def test_register_command_in_enabled_category(disable_commands_app) -> None: # Enable commands which are decorated with a category disable_commands_app.enable_category(DisableCommandSet.category_name) From 907fa3a9418361a9a806da7bf877fffd7c0b38cb Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 22 Apr 2026 00:37:44 -0400 Subject: [PATCH 4/5] Did some refactoring and updated comments. --- cmd2/cmd2.py | 153 ++++++++++++++++++++++++--------------------- tests/test_cmd2.py | 14 +++-- 2 files changed, 91 insertions(+), 76 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index e8587eb26..0d80dd007 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -41,10 +41,7 @@ import threading import time from code import InteractiveConsole -from collections import ( - deque, - namedtuple, -) +from collections import deque from collections.abc import ( Callable, Iterable, @@ -61,6 +58,7 @@ TYPE_CHECKING, Any, ClassVar, + NamedTuple, TextIO, TypeVar, cast, @@ -219,8 +217,15 @@ def __init__(self) -> None: self.completer: Callable[[str, int], str | None] | None = None -# Contains data about a disabled command which is used to restore its original functions when the command is enabled -DisabledCommand = namedtuple("DisabledCommand", ["command_function", "help_function", "completer_function"]) # noqa: PYI024 +class DisabledCommand(NamedTuple): + """Stores data about a disabled command. + + This data is used to restore its functions when the command is enabled. + """ + + command_func: BoundCommandFunc + help_func: Callable[[], Any] | None + completer_func: BoundCompleter | None class CommandParsers: @@ -2448,12 +2453,12 @@ def _perform_completion( completer_func = func_attr else: # There's no completer function, next see if the command uses argparse - func = self.get_command_func(command) - argparser = None if func is None else self.command_parsers.get(func) + command_func = self.get_command_func(command) + argparser = None if command_func is None else self.command_parsers.get(command_func) - if func is not None and argparser is not None: + if command_func is not None and argparser is not None: # Get arguments for complete() - preserve_quotes = getattr(func, constants.CMD_ATTR_PRESERVE_QUOTES) + preserve_quotes = getattr(command_func, constants.CMD_ATTR_PRESERVE_QUOTES) cmd_set = self.find_commandset_for_command(command) # Create the argparse completer @@ -2718,9 +2723,8 @@ def _get_commands_aliases_and_macros_choices(self) -> Choices: # Add commands for command in self.get_visible_commands(): - # Get the command method - func = getattr(self, constants.COMMAND_FUNC_PREFIX + command) - description = strip_doc_annotations(func.__doc__).splitlines()[0] if func.__doc__ else "" + command_func = cast(BoundCommandFunc, self.get_command_func(command)) + description = strip_doc_annotations(command_func.__doc__).splitlines()[0] if command_func.__doc__ else "" items.append(CompletionItem(command, display_meta=description)) # Add aliases @@ -3328,9 +3332,9 @@ def get_command_func(self, command: str) -> BoundCommandFunc | None: :param command: the name of the command :return: the bound function implementing the command, or None if not found """ - func_name = constants.COMMAND_FUNC_PREFIX + command - func = getattr(self, func_name, None) - return cast(BoundCommandFunc, func) if callable(func) else None + command_func_name = constants.COMMAND_FUNC_PREFIX + command + command_func = getattr(self, command_func_name, None) + return cast(BoundCommandFunc, command_func) if callable(command_func) else None def _get_command_category(self, func: BoundCommandFunc) -> str: """Determine the category for a command. @@ -3363,8 +3367,8 @@ def onecmd(self, statement: Statement | str, *, add_to_history: bool = True) -> if not isinstance(statement, Statement): statement = self._input_line_to_statement(statement) - func = self.get_command_func(statement.command) - if func: + command_func = self.get_command_func(statement.command) + if command_func: # Check to see if this command should be stored in history if ( statement.command not in self.exclude_from_history @@ -3375,7 +3379,7 @@ def onecmd(self, statement: Statement | str, *, add_to_history: bool = True) -> try: self.current_command = statement - stop = func(statement) + stop = command_func(statement) finally: self.current_command = None @@ -4219,7 +4223,9 @@ def complete_help_subcommands( return Completions() # Check if this command uses argparse - if (func := self.get_command_func(command)) is None or (argparser := self.command_parsers.get(func)) is None: + if (command_func := self.get_command_func(command)) is None or ( + argparser := self.command_parsers.get(command_func) + ) is None: return Completions() completer = argparse_completer.DEFAULT_AP_COMPLETER(argparser, self) @@ -4245,8 +4251,8 @@ def _build_command_info(self) -> tuple[dict[str, list[str]], list[str]]: help_topics.remove(command) # Store the command within its category - func = cast(BoundCommandFunc, self.get_command_func(command)) - category = self._get_command_category(func) + command_func = cast(BoundCommandFunc, self.get_command_func(command)) + category = self._get_command_category(command_func) cmds_cats.setdefault(category, []).append(command) return cmds_cats, help_topics @@ -4320,16 +4326,16 @@ def do_help(self, args: argparse.Namespace) -> None: if help_func is not None: help_func() else: - # This is a defensive fallback in case someone disabled a command - # without using disable_command() resulting in no help function. + # Handle potential case where command is disabled by manually editing + # self.disabled_commands instead of using disable_command(). self._report_disabled_command_usage(message_to_print=f"{args.command} is currently disabled.") return - func = self.get_command_func(args.command) - argparser = None if func is None else self.command_parsers.get(func) + command_func = self.get_command_func(args.command) + argparser = None if command_func is None else self.command_parsers.get(command_func) # If the command function uses argparse, then use argparse's help - if func is not None and argparser is not None: + if command_func is not None and argparser is not None: completer = argparse_completer.DEFAULT_AP_COMPLETER(argparser, self) completer.print_help(args.subcommands, self.stdout) @@ -4338,8 +4344,8 @@ def do_help(self, args: argparse.Namespace) -> None: help_func() # If the command function has a docstring, then print it - elif func is not None and func.__doc__ is not None: - self.poutput(pydoc.getdoc(func)) + elif command_func is not None and command_func.__doc__ is not None: + self.poutput(pydoc.getdoc(command_func)) # If there is no help information then print an error else: @@ -4380,15 +4386,15 @@ def print_topics(self, header: str, cmds: Sequence[str] | None, cmdlen: int, max self.columnize(cmds, maxcol) self.poutput() - def _print_documented_command_topics(self, header: str, cmds: Sequence[str], verbose: bool) -> None: + def _print_documented_command_topics(self, header: str, commands: Sequence[str], verbose: bool) -> None: """Print topics which are documented commands, switching between verbose or traditional output.""" import io - if not cmds: + if not commands: return if not verbose: - self.print_topics(header, cmds, 15, 80) + self.print_topics(header, commands, 15, 80) return topic_table = Cmd2SimpleTable( @@ -4398,8 +4404,8 @@ def _print_documented_command_topics(self, header: str, cmds: Sequence[str], ver # Try to get the documentation string for each command topics = self.get_help_topics() - for command in cmds: - if (cmd_func := self.get_command_func(command)) is None: + for command in commands: + if (command_func := self.get_command_func(command)) is None: continue doc: str | None @@ -4424,7 +4430,7 @@ def _print_documented_command_topics(self, header: str, cmds: Sequence[str], ver doc = result.getvalue() else: - doc = cmd_func.__doc__ + doc = command_func.__doc__ # Attempt to locate the first documentation block cmd_desc = strip_doc_annotations(doc) if doc else "" @@ -5593,25 +5599,25 @@ def enable_command(self, command: str) -> None: if command not in self.disabled_commands: return - cmd_func_name = constants.COMMAND_FUNC_PREFIX + command + command_func_name = constants.COMMAND_FUNC_PREFIX + command help_func_name = constants.HELP_FUNC_PREFIX + command completer_func_name = constants.COMPLETER_FUNC_PREFIX + command # Restore the command function to its original value dc = self.disabled_commands[command] - setattr(self, cmd_func_name, dc.command_function) + setattr(self, command_func_name, dc.command_func) # Restore the help function to its original value - if dc.help_function is None: + if dc.help_func is None: delattr(self, help_func_name) else: - setattr(self, help_func_name, dc.help_function) + setattr(self, help_func_name, dc.help_func) # Restore the completer function to its original value - if dc.completer_function is None: + if dc.completer_func is None: delattr(self, completer_func_name) else: - setattr(self, completer_func_name, dc.completer_function) + setattr(self, completer_func_name, dc.completer_func) # Remove the disabled command entry del self.disabled_commands[command] @@ -5625,15 +5631,15 @@ def enable_category(self, category: str) -> None: if category not in self.disabled_categories: return - for cmd_name in list(self.disabled_commands): - func = self.disabled_commands[cmd_name].command_function - if self._get_command_category(func) == category: - self.enable_command(cmd_name) + for command in list(self.disabled_commands): + command_func = self.disabled_commands[command].command_func + if self._get_command_category(command_func) == category: + self.enable_command(command) del self.disabled_categories[category] def disable_command(self, command: str, message_to_print: str) -> None: - """Disable a command and overwrite its functions. + """Disable a command and replace its functions with disabled versions. :param command: the command being disabled :param message_to_print: what to print when this command is run or help is called on it while disabled @@ -5647,41 +5653,48 @@ def disable_command(self, command: str, message_to_print: str) -> None: return # Make sure this is an actual command - command_function = self.get_command_func(command) - if command_function is None: + command_func = self.get_command_func(command) + if command_func is None: raise AttributeError(f"'{command}' does not refer to a command") - cmd_func_name = constants.COMMAND_FUNC_PREFIX + command + command_func_name = constants.COMMAND_FUNC_PREFIX + command + help_func_name = constants.HELP_FUNC_PREFIX + command + help_func = getattr(self, help_func_name, None) + completer_func_name = constants.COMPLETER_FUNC_PREFIX + command + completer_func = getattr(self, completer_func_name, None) # Add the disabled command record self.disabled_commands[command] = DisabledCommand( - command_function=command_function, - help_function=getattr(self, help_func_name, None), - completer_function=getattr(self, completer_func_name, None), + command_func=command_func, + help_func=help_func, + completer_func=completer_func, ) - # Overwrite command function to print the message + # Replace command and help functions to report the disabled message message_to_print = message_to_print.replace(constants.COMMAND_NAME, command) - new_cmd_func = functools.partial(self._report_disabled_command_usage, message_to_print=message_to_print) + new_cmd_func = functools.partial( + self._report_disabled_command_usage, + message_to_print=message_to_print, + ) - # Preserve the metadata of the original command function - functools.update_wrapper(new_cmd_func, command_function) - setattr(self, cmd_func_name, new_cmd_func) + # Ensure the replacement function identifies as the original for introspection + functools.update_wrapper(new_cmd_func, command_func) + setattr(self, command_func_name, new_cmd_func) - # Overwrite the help function to print the message - new_help_func = functools.partial(self._report_disabled_command_usage, message_to_print=message_to_print) - if (help_function := self.disabled_commands[command].help_function) is not None: - # Preserve the metadata of the original help function - functools.update_wrapper(new_help_func, help_function) + new_help_func = functools.partial( + self._report_disabled_command_usage, + message_to_print=message_to_print, + ) + if help_func is not None: + functools.update_wrapper(new_help_func, help_func) setattr(self, help_func_name, new_help_func) - # Set the completer to a function that returns a nothing + # Replace completer with a function that returns nothing new_completer_func = functools.partial(self._disabled_completer) - if (completer_function := self.disabled_commands[command].completer_function) is not None: - # Preserve the metadata of the original completer function - functools.update_wrapper(new_completer_func, completer_function) + if completer_func is not None: + functools.update_wrapper(new_completer_func, completer_func) setattr(self, completer_func_name, new_completer_func) def disable_category(self, category: str, message_to_print: str) -> None: @@ -5699,10 +5712,10 @@ def disable_category(self, category: str, message_to_print: str) -> None: all_commands = self.get_all_commands() - for cmd_name in all_commands: - func = cast(BoundCommandFunc, self.get_command_func(cmd_name)) - if self._get_command_category(func) == category: - self.disable_command(cmd_name, message_to_print) + for command in all_commands: + command_func = cast(BoundCommandFunc, self.get_command_func(command)) + if self._get_command_category(command_func) == category: + self.disable_command(command, message_to_print) self.disabled_categories[category] = message_to_print diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index ccd3fc569..aae10dbe9 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -6,7 +6,10 @@ import sys import tempfile from code import InteractiveConsole -from typing import NoReturn +from typing import ( + NoReturn, + cast, +) from unittest import mock import pytest @@ -34,6 +37,7 @@ ) from cmd2 import rich_utils as ru from cmd2 import string_utils as su +from cmd2.types import BoundCommandFunc from .conftest import ( SHORTCUTS_TXT, @@ -4080,12 +4084,10 @@ def test_help_argparse_command_while_disabled(disable_commands_app: DisableComma def test_help_disabled_no_help_func(base_app: cmd2.Cmd) -> None: from cmd2.cmd2 import DisabledCommand - # Manually disable a command without a help function to trigger the defensive fallback + # Intentionally bypass disable_command() to test the fallback in do_help() command = "quit" - command_func = base_app.get_command_func(command) - base_app.disabled_commands[command] = DisabledCommand( - command_function=command_func, help_function=None, completer_function=None - ) + command_func = cast(BoundCommandFunc, base_app.get_command_func(command)) + base_app.disabled_commands[command] = DisabledCommand(command_func=command_func, help_func=None, completer_func=None) _out, err = run_cmd(base_app, f"help {command}") assert err[0].startswith(f"{command} is currently disabled.") From 1381c72b4007bddcac24b5000cc49fa3f0968d10 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 22 Apr 2026 00:50:28 -0400 Subject: [PATCH 5/5] Updated CHANGELOG and comment. --- CHANGELOG.md | 1 + tests/test_cmd2.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cf9b497f..4e24f2006 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,7 @@ prompt is displayed. - Replaced the global `APP_THEME` constant in `rich_utils.py` with `get_theme()` and `set_theme()` functions to support lazy initialization and safer in-place updates of the theme. + - Renamed `Cmd._command_parsers` to `Cmd.command_parsers`. - Enhancements - New `cmd2.Cmd` parameters - **auto_suggest**: (boolean) if `True`, provide fish shell style auto-suggestions. These diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index aae10dbe9..39aad2d27 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4017,6 +4017,7 @@ def test_disabled_command_not_in_history(disable_commands_app) -> None: def test_get_parser_while_disabled(disable_commands_app: DisableCommandsApp) -> None: + """Test that command_parsers can find a disabled command's parser.""" # Get parser before disabling parser_before = disable_commands_app.command_parsers.get(disable_commands_app.do_argparse_command) assert parser_before is not None @@ -4024,7 +4025,7 @@ def test_get_parser_while_disabled(disable_commands_app: DisableCommandsApp) -> # Disable command disable_commands_app.disable_command("argparse_command", "Disabled") - # Get parser after disabling - this is what was failing (returning None) + # Get parser after disabling parser_after = disable_commands_app.command_parsers.get(disable_commands_app.do_argparse_command) assert parser_after is not None assert parser_after is parser_before