From e7273602b6112bef458cf36cf9a830f138701ab3 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Thu, 2 Apr 2026 12:47:21 -0400 Subject: [PATCH 01/21] Added attach_subcommand() and detach_subcommand() to public API. --- cmd2/argparse_custom.py | 63 +++++++++++++++++ cmd2/cmd2.py | 147 +++++++++++++++++++++------------------ tests/test_completion.py | 2 +- 3 files changed, 142 insertions(+), 70 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 5711ffb68..2818fbcef 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -243,6 +243,7 @@ def get_choices(self) -> Choices: from argparse import ArgumentError from collections.abc import ( Callable, + Iterable, Iterator, Sequence, ) @@ -927,6 +928,68 @@ def add_subparsers(self, **kwargs: Any) -> argparse._SubParsersAction: # type: return super().add_subparsers(**kwargs) + def _find_subparsers_action(self) -> argparse._SubParsersAction: # type: ignore[type-arg] + """Find the _SubParsersAction for this parser. + + :return: the _SubParsersAction for this parser + :raises ValueError: if this parser does not support subcommands + """ + for action in self._actions: + if isinstance(action, argparse._SubParsersAction): + return action + raise ValueError(f"Command '{self.prog}' does not support subcommands") + + 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 + :return: the discovered Cmd2ArgumentParser + :raises ValueError: if any subcommand in the path is not found or a level doesn't support subcommands + """ + parser = self + for name in subcommand_path: + subparsers_action = parser._find_subparsers_action() + if name not in subparsers_action.choices: + raise ValueError(f"Subcommand '{name}' not found in '{parser.prog}'") + parser = cast(Cmd2ArgumentParser, subparsers_action.choices[name]) + return parser + + def attach_subcommand( + self, + subcommand_path: Iterable[str], + subcommand: str, + parser: 'Cmd2ArgumentParser', + **add_parser_kwargs: Any, + ) -> None: + """Attach a parser as a subcommand to a command at the specified path. + + :param subcommand_path: sequence of subcommand names leading to the parser that will + host the new subcommand. An empty sequence indicates this parser. + :param subcommand: name of the new subcommand + :param parser: the parser to attach + :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :raises ValueError: if the command path is invalid or doesn't support subcommands + """ + target_parser = self._find_parser(subcommand_path) + subparsers_action = target_parser._find_subparsers_action() + subparsers_action.attach_parser(subcommand, parser, **add_parser_kwargs) # type: ignore[attr-defined] + + def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> 'Cmd2ArgumentParser': + """Detach a subcommand from a command at the specified path. + + :param subcommand_path: sequence of subcommand names leading to the parser hosting the + subcommand to be detached. An empty sequence indicates this parser. + :param subcommand: name of the subcommand to detach + :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._find_subparsers_action() + detached = subparsers_action.detach_parser(subcommand) # type: ignore[attr-defined] + if detached is None: + raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") + return cast(Cmd2ArgumentParser, detached) + def error(self, message: str) -> NoReturn: """Override that applies custom formatting to the error message.""" lines = message.split('\n') diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 9181f01e1..eb6b0d1d0 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -49,7 +49,6 @@ Callable, Iterable, Mapping, - MutableSequence, Sequence, ) from dataclasses import ( @@ -1085,40 +1084,8 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: if not subcommand_valid: raise CommandSetRegistrationError(f'Subcommand {subcommand_name} is not valid: {errmsg}') - command_tokens = full_command_name.split() - command_name = command_tokens[0] - subcommand_names = command_tokens[1:] - - # Search for the base command function and verify it has an argparser defined - if command_name in self.disabled_commands: - command_func = self.disabled_commands[command_name].command_function - else: - command_func = self.cmd_func(command_name) - - if command_func is None: - raise CommandSetRegistrationError(f"Could not find command '{command_name}' needed by subcommand: {method}") - command_parser = self._command_parsers.get(command_func) - if command_parser is None: - raise CommandSetRegistrationError( - f"Could not find argparser for command '{command_name}' needed by subcommand: {method}" - ) - - def find_subcommand(action: Cmd2ArgumentParser, subcmd_names: MutableSequence[str]) -> Cmd2ArgumentParser: - if not subcmd_names: - return action - cur_subcmd = subcmd_names.pop(0) - for sub_action in action._actions: - if isinstance(sub_action, argparse._SubParsersAction): - for choice_name, choice in sub_action.choices.items(): - if choice_name == cur_subcmd: - return find_subcommand(choice, subcmd_names) - break - raise CommandSetRegistrationError(f"Could not find subcommand '{action}'") - - target_parser = find_subcommand(command_parser, subcommand_names) - # Create the subcommand parser and configure it - subcmd_parser = self._build_parser(cmdset, subcmd_parser_builder, f'{command_name} {subcommand_name}') + subcmd_parser = self._build_parser(cmdset, subcmd_parser_builder, f'{full_command_name} {subcommand_name}') if subcmd_parser.description is None and method.__doc__: subcmd_parser.description = strip_doc_annotations(method.__doc__) @@ -1129,19 +1096,14 @@ def find_subcommand(action: Cmd2ArgumentParser, subcmd_names: MutableSequence[st # Set what instance the handler is bound to setattr(subcmd_parser, constants.PARSER_ATTR_COMMANDSET_ID, id(cmdset)) - # Find the argparse action that handles subcommands - for action in target_parser._actions: - if isinstance(action, argparse._SubParsersAction): - # Get add_parser() kwargs (aliases, help, etc.) defined by the decorator - add_parser_kwargs = getattr(method, constants.SUBCMD_ATTR_ADD_PARSER_KWARGS, {}) - - # Attach existing parser as a subcommand - action.attach_parser( # type: ignore[attr-defined] - subcommand_name, - subcmd_parser, - **add_parser_kwargs, - ) - break + # Get add_parser() kwargs (aliases, help, etc.) defined by the decorator + add_parser_kwargs = getattr(method, constants.SUBCMD_ATTR_ADD_PARSER_KWARGS, {}) + + # Attach existing parser as a subcommand + try: + self.attach_subcommand(full_command_name, subcommand_name, subcmd_parser, **add_parser_kwargs) + except ValueError as ex: + raise CommandSetRegistrationError(str(ex)) from ex def _unregister_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: """Unregister subcommands from their base command. @@ -1165,30 +1127,77 @@ def _unregister_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: # iterate through all matching methods for _method_name, method in methods: subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME) - command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND) + full_command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND) - # Search for the base command function and verify it has an argparser defined - if command_name in self.disabled_commands: - command_func = self.disabled_commands[command_name].command_function - else: - command_func = self.cmd_func(command_name) - - if command_func is None: # pragma: no cover - # This really shouldn't be possible since _register_subcommands would prevent this from happening - # but keeping in case it does for some strange reason - raise CommandSetRegistrationError(f"Could not find command '{command_name}' needed by subcommand: {method}") - command_parser = self._command_parsers.get(command_func) - if command_parser is None: # pragma: no cover - # This really shouldn't be possible since _register_subcommands would prevent this from happening - # but keeping in case it does for some strange reason - raise CommandSetRegistrationError( - f"Could not find argparser for command '{command_name}' needed by subcommand: {method}" - ) + with contextlib.suppress(ValueError): + self.detach_subcommand(full_command_name, subcommand_name) - for action in command_parser._actions: - if isinstance(action, argparse._SubParsersAction): - action.detach_parser(subcommand_name) # type: ignore[attr-defined] - break + 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 + 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. + + :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 + strings representing the relative path to the desired hosting parser. + :raises ValueError: if the command is empty, the root command is not found, or + the root command does not use an argparse parser. + """ + tokens = command.split() + if not tokens: + raise ValueError("Command path cannot be empty") + + root_command = tokens[0] + 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.cmd_func(root_command) + + if command_func is None: + raise ValueError(f"Root command '{root_command}' not found") + + root_parser = self._command_parsers.get(command_func) + if root_parser is None: + raise ValueError(f"Command '{root_command}' does not use argparse") + + return root_parser, subcommand_path + + def attach_subcommand( + self, + command: str, + subcommand: str, + parser: Cmd2ArgumentParser, + **add_parser_kwargs: Any, + ) -> None: + """Attach a parser as a subcommand to a command at the specified path. + + :param command: full command path (space-delimited) leading to the parser that will + host the new subcommand (e.g. 'foo bar') + :param subcommand: name of the new subcommand + :param parser: the parser to attach + :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :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.attach_subcommand(subcommand_path, subcommand, parser, **add_parser_kwargs) + + def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser: + """Detach a subcommand from a command at the specified path. + + :param command: full command path (space-delimited) leading to the parser hosting the + subcommand to be detached (e.g. 'foo bar') + :param subcommand: name of the subcommand to detach + :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) + return root_parser.detach_subcommand(subcommand_path, subcommand) @property def always_prefix_settables(self) -> bool: diff --git a/tests/test_completion.py b/tests/test_completion.py index 02018ba3c..992ba4fdd 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -942,7 +942,7 @@ def test_clean_display() -> None: assert completion_item.display == expected assert completion_item.display_meta == expected - # Verify that text derived display is also sanitized + # Verify that text-derived display is also sanitized text = "item\nwith\nnewlines" expected_text_display = "item with newlines" completion_item = CompletionItem(text) From 043d225cb8909c1bab17325c083ec83695ac0623 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Thu, 2 Apr 2026 15:46:25 -0400 Subject: [PATCH 02/21] Moved set_parser_prog() to Cmd2ArgumentParser.update_prog(). --- CHANGELOG.md | 1 + cmd2/argparse_custom.py | 118 +++++++++++++++++++++------------------- cmd2/cmd2.py | 22 ++++---- 3 files changed, 73 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 368a487e0..5f7d3d380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ prompt is displayed. class of it. - Removed `set_ap_completer_type()` and `get_ap_completer_type()` since `ap_completer_type` is now a public member of `Cmd2ArgumentParser`. + - Moved `set_parser_prog()` to `Cmd2ArgumentParser.update_prog()`. - Enhancements - New `cmd2.Cmd` parameters - **auto_suggest**: (boolean) if `True`, provide fish shell style auto-suggestions. These diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 2818fbcef..a4f706b49 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -303,56 +303,6 @@ def generate_range_error(range_min: int, range_max: float) -> str: return err_msg -def set_parser_prog(parser: argparse.ArgumentParser, prog: str) -> None: - """Recursively set prog attribute of a parser and all of its subparsers. - - Does so that the root command is a command name and not sys.argv[0]. - - :param parser: the parser being edited - :param prog: new value for the parser's prog attribute - """ - # Set the prog value for this parser - parser.prog = prog - req_args: list[str] = [] - - # Set the prog value for the parser's subcommands - for action in parser._actions: - if isinstance(action, argparse._SubParsersAction): - # Set the _SubParsersAction's _prog_prefix value. That way if its add_parser() method is called later, - # the correct prog value will be set on the parser being added. - action._prog_prefix = parser.prog - - # The keys of action.choices are subcommand names as well as subcommand aliases. The aliases point to the - # same parser as the actual subcommand. We want to avoid placing an alias into a parser's prog value. - # Unfortunately there is nothing about an action.choices entry which tells us it's an alias. In most cases - # we can filter out the aliases by checking the contents of action._choices_actions. This list only contains - # help information and names for the subcommands and not aliases. However, subcommands without help text - # won't show up in that list. Since dictionaries are ordered in Python 3.6 and above and argparse inserts the - # subcommand name into choices dictionary before aliases, we should be OK assuming the first time we see a - # parser, the dictionary key is a subcommand and not alias. - processed_parsers = [] - - # Set the prog value for each subcommand's parser - for subcmd_name, subcmd_parser in action.choices.items(): - # Check if we've already edited this parser - if subcmd_parser in processed_parsers: - continue - - subcmd_prog = parser.prog - if req_args: - subcmd_prog += " " + " ".join(req_args) - subcmd_prog += " " + subcmd_name - set_parser_prog(subcmd_parser, subcmd_prog) - processed_parsers.append(subcmd_parser) - - # We can break since argparse only allows 1 group of subcommands per level - break - - # Need to save required args so they can be prepended to the subcommand usage - if action.required: - req_args.append(action.dest) - - ############################################################################################################ # Allow developers to add custom action attributes ############################################################################################################ @@ -928,16 +878,70 @@ def add_subparsers(self, **kwargs: Any) -> argparse._SubParsersAction: # type: return super().add_subparsers(**kwargs) - def _find_subparsers_action(self) -> argparse._SubParsersAction: # type: ignore[type-arg] - """Find the _SubParsersAction for this parser. + def _get_subparsers_action(self) -> argparse._SubParsersAction: # type: ignore[type-arg] + """Get the _SubParsersAction for this parser if it exists. :return: the _SubParsersAction for this parser :raises ValueError: if this parser does not support subcommands """ + if self._subparsers is not None: + for action in self._subparsers._group_actions: + if isinstance(action, argparse._SubParsersAction): + return action + raise ValueError(f"Command '{self.prog}' does not support subcommands") + + def update_prog(self, prog: str) -> None: + """Recursively update the prog attribute of this parser and all of its subparsers. + + :param prog: new value for the parser's prog attribute + """ + # Set the prog value for this parser + self.prog = prog + + if self._subparsers is None: + # This parser has no subcommands + return + + # Required args which come before the subcommand + req_args: list[str] = [] + + # Set the prog value for the parser's subcommands for action in self._actions: if isinstance(action, argparse._SubParsersAction): - return action - raise ValueError(f"Command '{self.prog}' does not support subcommands") + # Set the _SubParsersAction's _prog_prefix value. That way if its add_parser() + # method is called later, the correct prog value will be set on the parser being added. + action._prog_prefix = self.prog + + # The keys of action.choices are subcommand names as well as subcommand aliases. + # The aliases point to the same parser as the actual subcommand. We want to avoid + # placing an alias into a parser's prog value. Unfortunately there is nothing about + # an action.choices entry which tells us it's an alias. In most cases we can filter out + # the aliases by checking the contents of action._choices_actions. This list only contains + # help information and names for the subcommands and not aliases. However, subcommands + # without help text won't show up in that list. Since dictionaries are ordered and + # argparse inserts the subcommand name into choices dictionary before aliases, we should + # be OK assuming the first time we see a parser, the dictionary key is a subcommand and + # not an alias. + processed_parsers = [] + + # Set the prog value for each subcommand's parser + for subcmd_name, subcmd_parser in action.choices.items(): + if subcmd_parser in processed_parsers: + continue + + subcmd_prog = self.prog + if req_args: + subcmd_prog += " " + " ".join(req_args) + subcmd_prog += " " + subcmd_name + subcmd_parser.update_prog(subcmd_prog) + processed_parsers.append(subcmd_parser) + + # We can break since argparse only allows 1 group of subcommands per level + break + + # Need to save required args so they can be prepended to the subcommand usage + if action.required: + req_args.append(action.dest) def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': """Find a parser in the hierarchy based on a sequence of subcommand names. @@ -948,7 +952,7 @@ def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': """ parser = self for name in subcommand_path: - subparsers_action = parser._find_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 = cast(Cmd2ArgumentParser, subparsers_action.choices[name]) @@ -971,7 +975,7 @@ def attach_subcommand( :raises ValueError: if the command path is invalid or doesn't support subcommands """ target_parser = self._find_parser(subcommand_path) - subparsers_action = target_parser._find_subparsers_action() + subparsers_action = target_parser._get_subparsers_action() subparsers_action.attach_parser(subcommand, parser, **add_parser_kwargs) # type: ignore[attr-defined] def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> 'Cmd2ArgumentParser': @@ -984,7 +988,7 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> :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._find_subparsers_action() + subparsers_action = target_parser._get_subparsers_action() detached = subparsers_action.detach_parser(subcommand) # type: ignore[attr-defined] if detached is None: raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index eb6b0d1d0..68a922371 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -920,7 +920,7 @@ def _build_parser( builder_name = getattr(parser_builder, "__name__", str(parser_builder)) # type: ignore[unreachable] raise TypeError(f"The parser returned by '{builder_name}' must be a Cmd2ArgumentParser or a subclass of it") - argparse_custom.set_parser_prog(parser, prog) + parser.update_prog(prog) return parser @@ -1027,16 +1027,16 @@ def unregister_command_set(self, cmdset: CommandSet) -> None: def _check_uninstallable(self, cmdset: CommandSet) -> None: def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: cmdset_id = id(cmdset) - for action in parser._actions: - if isinstance(action, argparse._SubParsersAction): - for subparser in action.choices.values(): - attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) - if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: - raise CommandSetRegistrationError( - 'Cannot uninstall CommandSet when another CommandSet depends on it' - ) - check_parser_uninstallable(subparser) - break + try: + subparsers_action = parser._get_subparsers_action() + for subparser in subparsers_action.choices.values(): + attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) + if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: + raise CommandSetRegistrationError('Cannot uninstall CommandSet when another CommandSet depends on it') + check_parser_uninstallable(cast(Cmd2ArgumentParser, subparser)) + except ValueError: + # No subcommands to check + pass methods: list[tuple[str, Callable[..., Any]]] = inspect.getmembers( cmdset, From 179e1197bec317ac63d0d2cd216494e72662938a Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Thu, 2 Apr 2026 17:09:48 -0400 Subject: [PATCH 03/21] Fixing logic in update_prog(). --- cmd2/argparse_custom.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index a4f706b49..bfc991cd8 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -902,15 +902,24 @@ def update_prog(self, prog: str) -> None: # This parser has no subcommands return - # Required args which come before the subcommand - req_args: list[str] = [] + # argparse includes positional arguments that appear before the subcommand in its + # subparser prog strings. We must track these while iterating through actions. + positionals: list[str] = [] + + # Use a formatter to get the same argument strings that argparse uses. + formatter = self._get_formatter() # Set the prog value for the parser's subcommands for action in self._actions: if isinstance(action, argparse._SubParsersAction): + # Build the prefix that should be prepended to subcommand names + prefix = self.prog + if positionals: + prefix += " " + " ".join(positionals) + # Set the _SubParsersAction's _prog_prefix value. That way if its add_parser() # method is called later, the correct prog value will be set on the parser being added. - action._prog_prefix = self.prog + action._prog_prefix = prefix # The keys of action.choices are subcommand names as well as subcommand aliases. # The aliases point to the same parser as the actual subcommand. We want to avoid @@ -922,26 +931,23 @@ def update_prog(self, prog: str) -> None: # argparse inserts the subcommand name into choices dictionary before aliases, we should # be OK assuming the first time we see a parser, the dictionary key is a subcommand and # not an alias. - processed_parsers = [] + processed_parsers: list[argparse.ArgumentParser] = [] # Set the prog value for each subcommand's parser for subcmd_name, subcmd_parser in action.choices.items(): if subcmd_parser in processed_parsers: continue - subcmd_prog = self.prog - if req_args: - subcmd_prog += " " + " ".join(req_args) - subcmd_prog += " " + subcmd_name - subcmd_parser.update_prog(subcmd_prog) + subcmd_prog = f"{prefix} {subcmd_name}" + subcmd_parser.update_prog(subcmd_prog) # type: ignore[attr-defined] processed_parsers.append(subcmd_parser) # We can break since argparse only allows 1 group of subcommands per level break - # Need to save required args so they can be prepended to the subcommand usage - if action.required: - req_args.append(action.dest) + # Save positional argument + if not action.option_strings: + positionals.append(formatter._format_args(action, action.dest)) def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': """Find a parser in the hierarchy based on a sequence of subcommand names. From e0702f240eb9a0c7a1c7d0ca2851549bc1459e33 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Thu, 2 Apr 2026 17:43:01 -0400 Subject: [PATCH 04/21] Simplifying code. --- cmd2/argparse_custom.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index bfc991cd8..f42c25aff 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -912,14 +912,11 @@ def update_prog(self, prog: str) -> None: # Set the prog value for the parser's subcommands for action in self._actions: if isinstance(action, argparse._SubParsersAction): - # Build the prefix that should be prepended to subcommand names - prefix = self.prog + # Set the _SubParsersAction's _prog_prefix value. This ensures that any subcommands + # added later via add_parser() will have the correct prog value. + action._prog_prefix = self.prog if positionals: - prefix += " " + " ".join(positionals) - - # Set the _SubParsersAction's _prog_prefix value. That way if its add_parser() - # method is called later, the correct prog value will be set on the parser being added. - action._prog_prefix = prefix + action._prog_prefix += " " + " ".join(positionals) # The keys of action.choices are subcommand names as well as subcommand aliases. # The aliases point to the same parser as the actual subcommand. We want to avoid @@ -938,7 +935,7 @@ def update_prog(self, prog: str) -> None: if subcmd_parser in processed_parsers: continue - subcmd_prog = f"{prefix} {subcmd_name}" + subcmd_prog = f"{action._prog_prefix} {subcmd_name}" subcmd_parser.update_prog(subcmd_prog) # type: ignore[attr-defined] processed_parsers.append(subcmd_parser) From e07f44ee44a46de411861b6dc97e25ccb670f957 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Thu, 2 Apr 2026 19:21:00 -0400 Subject: [PATCH 05/21] Copying argparse's method for setting a subcommand's prog value. --- cmd2/argparse_custom.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index f42c25aff..b4bf79535 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -903,20 +903,18 @@ def update_prog(self, prog: str) -> None: return # argparse includes positional arguments that appear before the subcommand in its - # subparser prog strings. We must track these while iterating through actions. - positionals: list[str] = [] - - # Use a formatter to get the same argument strings that argparse uses. - formatter = self._get_formatter() + # subparser prog strings. Track these while iterating through actions. + positionals: list[argparse.Action] = [] # Set the prog value for the parser's subcommands for action in self._actions: if isinstance(action, argparse._SubParsersAction): - # Set the _SubParsersAction's _prog_prefix value. This ensures that any subcommands - # added later via add_parser() will have the correct prog value. - action._prog_prefix = self.prog - if positionals: - action._prog_prefix += " " + " ".join(positionals) + # Use a formatter to generate _prog_prefix exactly as argparse does in + # add_subparsers(). This ensures that any subcommands added later via + # add_parser() will have the correct prog value. + formatter = self._get_formatter() + formatter.add_usage(self.usage, positionals, self._mutually_exclusive_groups, '') + action._prog_prefix = formatter.format_help().strip() # The keys of action.choices are subcommand names as well as subcommand aliases. # The aliases point to the same parser as the actual subcommand. We want to avoid @@ -944,7 +942,7 @@ def update_prog(self, prog: str) -> None: # Save positional argument if not action.option_strings: - positionals.append(formatter._format_args(action, action.dest)) + positionals.append(action) def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': """Find a parser in the hierarchy based on a sequence of subcommand names. From ad4a3de5909aff749d32c4947c7f50cafd100f34 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 12:19:19 -0400 Subject: [PATCH 06/21] More updates to update_prog(). --- cmd2/argparse_custom.py | 23 +++++++++-------------- tests/test_argparse.py | 4 ++-- tests/test_argparse_subcommands.py | 2 +- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index b4bf79535..d2c4e7ac3 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -893,7 +893,7 @@ def _get_subparsers_action(self) -> argparse._SubParsersAction: # type: ignore[ def update_prog(self, prog: str) -> None: """Recursively update the prog attribute of this parser and all of its subparsers. - :param prog: new value for the parser's prog attribute + :param prog: new value for this parser's prog attribute """ # Set the prog value for this parser self.prog = prog @@ -916,26 +916,21 @@ def update_prog(self, prog: str) -> None: formatter.add_usage(self.usage, positionals, self._mutually_exclusive_groups, '') action._prog_prefix = formatter.format_help().strip() - # The keys of action.choices are subcommand names as well as subcommand aliases. - # The aliases point to the same parser as the actual subcommand. We want to avoid - # placing an alias into a parser's prog value. Unfortunately there is nothing about - # an action.choices entry which tells us it's an alias. In most cases we can filter out - # the aliases by checking the contents of action._choices_actions. This list only contains - # help information and names for the subcommands and not aliases. However, subcommands - # without help text won't show up in that list. Since dictionaries are ordered and - # argparse inserts the subcommand name into choices dictionary before aliases, we should - # be OK assuming the first time we see a parser, the dictionary key is a subcommand and - # not an alias. - processed_parsers: list[argparse.ArgumentParser] = [] + # Note: action.choices contains both subcommand names and aliases. + # To ensure subcommands (and not aliases) are used in 'prog': + # 1. We can't use action._choices_actions because it excludes subcommands without help text. + # 2. Since dictionaries are ordered and argparse inserts the primary name before aliases, + # we assume the first time we encounter a parser, the key is the true subcommand name. + updated_parsers: set[argparse.ArgumentParser] = set() # Set the prog value for each subcommand's parser for subcmd_name, subcmd_parser in action.choices.items(): - if subcmd_parser in processed_parsers: + if subcmd_parser in updated_parsers: continue subcmd_prog = f"{action._prog_prefix} {subcmd_name}" subcmd_parser.update_prog(subcmd_prog) # type: ignore[attr-defined] - processed_parsers.append(subcmd_parser) + updated_parsers.add(subcmd_parser) # We can break since argparse only allows 1 group of subcommands per level break diff --git a/tests/test_argparse.py b/tests/test_argparse.py index d1fed524d..2f59a410f 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -335,7 +335,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to set_parser_prog() which + # This subcommand has aliases and no help text. It exists to prevent changes to update_prog() which # use an approach which relies on action._choices_actions list. See comment in that function for more # details. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) @@ -446,7 +446,7 @@ def test_subcommand_invalid_help(subcommand_app) -> None: def test_add_another_subcommand(subcommand_app) -> None: - """This tests makes sure set_parser_prog() sets _prog_prefix on every _SubParsersAction so that all future calls + """This tests makes sure update_prog() sets _prog_prefix on every _SubParsersAction so that all future calls to add_parser() write the correct prog value to the parser being added. """ base_parser = subcommand_app._command_parsers.get(subcommand_app.do_base) diff --git a/tests/test_argparse_subcommands.py b/tests/test_argparse_subcommands.py index 558924d1e..56a1fc201 100644 --- a/tests/test_argparse_subcommands.py +++ b/tests/test_argparse_subcommands.py @@ -45,7 +45,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to set_parser_prog() which + # This subcommand has aliases and no help text. It exists to prevent changes to update_prog() which # use an approach which relies on action._choices_actions list. See comment in that function for more # details. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) From cbeb22d6640022d81f16c3a6fe2e420ce3cd3ed4 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 13:51:20 -0400 Subject: [PATCH 07/21] Updated argparse_custom unit tests. --- tests/test_argparse.py | 17 +-------- tests/test_argparse_custom.py | 58 +++++++++++++++++++++++++++++- tests/test_argparse_subcommands.py | 4 +-- 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/tests/test_argparse.py b/tests/test_argparse.py index 2f59a410f..b361e1502 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -335,9 +335,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to update_prog() which - # use an approach which relies on action._choices_actions list. See comment in that function for more - # details. + # This subcommand has aliases and no help text. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) parser_helpless.add_argument('z', help='string') parser_helpless.set_defaults(func=base_helpless) @@ -445,19 +443,6 @@ def test_subcommand_invalid_help(subcommand_app) -> None: assert out[0].startswith('Usage: base') -def test_add_another_subcommand(subcommand_app) -> None: - """This tests makes sure update_prog() sets _prog_prefix on every _SubParsersAction so that all future calls - to add_parser() write the correct prog value to the parser being added. - """ - base_parser = subcommand_app._command_parsers.get(subcommand_app.do_base) - for sub_action in base_parser._actions: - if isinstance(sub_action, argparse._SubParsersAction): - new_parser = sub_action.add_parser('new_sub', help='stuff') - break - - assert new_parser.prog == "base new_sub" - - def test_subcmd_decorator(subcommand_app) -> None: # Test subcommand that has help option out, err = run_cmd(subcommand_app, 'test_subcmd_decorator subcmd') diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 95f5527c7..0a9dfc246 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -14,10 +14,10 @@ ) from cmd2.argparse_custom import ( Cmd2HelpFormatter, - Cmd2RichArgparseConsole, generate_range_error, register_argparse_argument_parameter, ) +from cmd2.rich_utils import Cmd2RichArgparseConsole from .conftest import run_cmd @@ -476,3 +476,59 @@ def side_effect(color, **kwargs): assert mock_set_color.call_count == 2 mock_set_color.assert_any_call(True, file=sys.stdout) mock_set_color.assert_any_call(True) + + +def test_update_prog() -> None: + """Test Cmd2ArgumentParser.update_prog() across various scenarios.""" + + # Set up a complex parser hierarchy + old_root = 'old_app' + parser = Cmd2ArgumentParser(prog=old_root) + + # Positionals before subcommand + parser.add_argument('pos1') + + # Mutually exclusive group with positionals + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument('posA', nargs='?') + group.add_argument('posB', nargs='?') + + # Subparsers with aliases and no help text + subparsers = parser.add_subparsers(dest='cmd') + + # Subcommand with aliases + sub1 = subparsers.add_parser('sub1', aliases=['s1', 'alias1'], help='help for sub1') + + # Subcommand with no help text + sub2 = subparsers.add_parser('sub2') + + # Nested subparser + sub2.add_argument('inner_pos') + sub2_subparsers = sub2.add_subparsers(dest='sub2_cmd') + leaf = sub2_subparsers.add_parser('leaf', help='leaf help') + + # Verify initial progs look correct + assert parser.prog == 'old_app' + assert sub1.prog == 'old_app pos1 (posA | posB) sub1' + assert sub2.prog == 'old_app pos1 (posA | posB) sub2' + assert leaf.prog == 'old_app pos1 (posA | posB) sub2 inner_pos leaf' + + # Perform update + new_root = 'new_app' + parser.update_prog(new_root) + + # Verify new progs look correct + assert parser.prog == 'new_app' + assert sub1.prog == 'new_app pos1 (posA | posB) sub1' + assert sub2.prog == 'new_app pos1 (posA | posB) sub2' + assert leaf.prog == 'new_app pos1 (posA | posB) sub2 inner_pos leaf' + + # Verify that action._prog_prefix was updated by adding a new subparser + sub3 = subparsers.add_parser('sub3') + assert sub3.prog == 'new_app pos1 (posA | posB) sub3' + + # Verify aliases still point to the correct parser + for action in parser._actions: + if isinstance(action, argparse._SubParsersAction): + assert action.choices['s1'].prog == sub1.prog + assert action.choices['alias1'].prog == sub1.prog diff --git a/tests/test_argparse_subcommands.py b/tests/test_argparse_subcommands.py index 56a1fc201..653f3fdcd 100644 --- a/tests/test_argparse_subcommands.py +++ b/tests/test_argparse_subcommands.py @@ -45,9 +45,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to update_prog() which - # use an approach which relies on action._choices_actions list. See comment in that function for more - # details. + # This subcommand has aliases and no help text. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) parser_helpless.add_argument('z', help='string') parser_helpless.set_defaults(func=base_helpless) From 31049483cb359ad1ec35c44b85d869f28e3c2224 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 14:08:03 -0400 Subject: [PATCH 08/21] Added another test. --- tests/test_argparse_custom.py | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 0a9dfc246..a9fb92156 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -384,6 +384,51 @@ def test_parser_attachment() -> None: assert root_subparsers.detach_parser("fake") is None # type: ignore[attr-defined] +def test_subcommand_attachment() -> None: + # Attach a subcommand + root_parser = Cmd2ArgumentParser(description="root command") + root_subparsers = root_parser.add_subparsers() + + child_parser = Cmd2ArgumentParser(description="child command") + root_parser.attach_subcommand( + [], + "child", + child_parser, + help="a child command", + aliases=["child_alias"], + ) + + # Verify the same parser instance was used + assert root_subparsers._name_parser_map["child"] is child_parser + assert root_subparsers._name_parser_map["child_alias"] is child_parser + + # Verify an action with the help text exists + child_action = None + for action in root_subparsers._choices_actions: + if action.dest == "child": + child_action = action + break + assert child_action is not None + assert child_action.help == "a child command" + + # Detatch the subcommand + detached_parser = root_parser.detach_subcommand([], "child") + + # Verify subcommand and its aliases were removed + assert detached_parser is child_parser + assert "child" not in root_subparsers._name_parser_map + assert "child_alias" not in root_subparsers._name_parser_map + + # Verify the help text action was removed + choices_actions = [action.dest for action in root_subparsers._choices_actions] + assert "child" not in choices_actions + + # Verify it raises a ValueError when subcommand does not exist + expected_err = "Subcommand 'fake' not found" + with pytest.raises(ValueError, match=expected_err): + assert root_parser.detach_subcommand([], "fake") is None + + def test_completion_items_as_choices(capsys) -> None: """Test cmd2's patch to Argparse._check_value() which supports CompletionItems as choices. Choices are compared to CompletionItems.orig_value instead of the CompletionItem instance. From 6344e237ca81bd567165591aa6f7cf71bcd391fc Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 14:53:21 -0400 Subject: [PATCH 09/21] More test updates. --- tests/test_argparse_custom.py | 88 +++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index a9fb92156..0b3ae9dba 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -343,11 +343,12 @@ def test_register_argparse_argument_parameter() -> None: def test_parser_attachment() -> None: + """Test the monkey-patched attach_parser and detach_parser methods on argparse._SubParsersAction.""" # Attach a parser as a subcommand - root_parser = Cmd2ArgumentParser(description="root command") + root_parser = Cmd2ArgumentParser(prog="root", description="root command") root_subparsers = root_parser.add_subparsers() - child_parser = Cmd2ArgumentParser(description="child command") + child_parser = Cmd2ArgumentParser(prog="child", description="child command") root_subparsers.attach_parser( # type: ignore[attr-defined] "child", child_parser, @@ -385,11 +386,24 @@ def test_parser_attachment() -> None: def test_subcommand_attachment() -> None: - # Attach a subcommand - root_parser = Cmd2ArgumentParser(description="root command") + """Test Cmd2ArgumentParser convenience methods for attaching and detaching subcommands.""" + + ############################### + # Set up parsers + ############################### + root_parser = Cmd2ArgumentParser(prog="root", description="root command") root_subparsers = root_parser.add_subparsers() - child_parser = Cmd2ArgumentParser(description="child command") + child_parser = Cmd2ArgumentParser(prog="child", description="child command") + child_subparsers = child_parser.add_subparsers() # Must have subparsers to host grandchild + + grandchild_parser = Cmd2ArgumentParser(prog="grandchild", description="grandchild command") + + ############################### + # Attach subcommands + ############################### + + # Attach child to root root_parser.attach_subcommand( [], "child", @@ -398,35 +412,59 @@ def test_subcommand_attachment() -> None: aliases=["child_alias"], ) - # Verify the same parser instance was used + # Attach grandchild to child + root_parser.attach_subcommand( + ["child"], + "grandchild", + grandchild_parser, + help="a grandchild command", + ) + + ############################### + # 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 + + ############################### + # Verify attachments + ############################### + + # Verify child attachment and aliases assert root_subparsers._name_parser_map["child"] is child_parser assert root_subparsers._name_parser_map["child_alias"] is child_parser - # Verify an action with the help text exists - child_action = None - for action in root_subparsers._choices_actions: - if action.dest == "child": - child_action = action - break - assert child_action is not None - assert child_action.help == "a child command" + # Verify grandchild attachment + assert child_subparsers._name_parser_map["grandchild"] is grandchild_parser - # Detatch the subcommand - detached_parser = root_parser.detach_subcommand([], "child") + ############################### + # Detach subcommands + ############################### - # Verify subcommand and its aliases were removed - assert detached_parser is child_parser + # Detach grandchild from child + detached_grandchild = root_parser.detach_subcommand(["child"], "grandchild") + assert detached_grandchild is grandchild_parser + assert "grandchild" not in child_subparsers._name_parser_map + + # 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 - # Verify the help text action was removed - choices_actions = [action.dest for action in root_subparsers._choices_actions] - assert "child" not in choices_actions + ############################### + # Test error handling + ############################### - # Verify it raises a ValueError when subcommand does not exist - expected_err = "Subcommand 'fake' not found" - with pytest.raises(ValueError, match=expected_err): - assert root_parser.detach_subcommand([], "fake") is None + # Verify ValueError when path is invalid (find_parser fails) + with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"): + root_parser.detach_subcommand(["nonexistent"], "anything") + + # Verify ValueError when path is valid but subcommand name is wrong + with pytest.raises(ValueError, match="Subcommand 'fake' not found in 'root'"): + root_parser.detach_subcommand([], "fake") def test_completion_items_as_choices(capsys) -> None: From 0d243080e9fa7783d5c70ca29579c46f64cc8248 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 15:04:25 -0400 Subject: [PATCH 10/21] Simplified function. --- cmd2/cmd2.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 68a922371..b6847807e 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1025,18 +1025,22 @@ def unregister_command_set(self, cmdset: CommandSet) -> None: self._installed_command_sets.remove(cmdset) def _check_uninstallable(self, cmdset: CommandSet) -> None: + cmdset_id = id(cmdset) + def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: - cmdset_id = id(cmdset) try: subparsers_action = parser._get_subparsers_action() - for subparser in subparsers_action.choices.values(): - attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) - if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: - raise CommandSetRegistrationError('Cannot uninstall CommandSet when another CommandSet depends on it') - check_parser_uninstallable(cast(Cmd2ArgumentParser, subparser)) except ValueError: # No subcommands to check - pass + return + + for subparser in subparsers_action.choices.values(): + attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) + if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: + raise CommandSetRegistrationError( + f"Cannot uninstall CommandSet: '{subparser.prog}' is required by another CommandSet" + ) + check_parser_uninstallable(cast(Cmd2ArgumentParser, subparser)) methods: list[tuple[str, Callable[..., Any]]] = inspect.getmembers( cmdset, From 6f7ca85ea1b50c1bc7f4f601c70d9a04d8c57d2d Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 15:14:15 -0400 Subject: [PATCH 11/21] Updated comments and type hint. --- cmd2/argparse_custom.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index d2c4e7ac3..48b28716d 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -533,7 +533,7 @@ def _SubParsersAction_attach_parser( # noqa: N802 subcmd_parser: argparse.ArgumentParser, **add_parser_kwargs: Any, ) -> None: - """Attach an existing ArgumentParser to a subparsers action. + """Attach an existing parser to a subparsers action. This is useful when a parser is pre-configured (e.g. by cmd2's subcommand decorator) and needs to be attached to a parent parser. @@ -828,7 +828,7 @@ def __init__( *, ap_completer_type: type['ArgparseCompleter'] | None = None, ) -> None: - """Initialize the Cmd2ArgumentParser instance, a custom ArgumentParser added by cmd2. + """Initialize the Cmd2ArgumentParser instance. :param ap_completer_type: optional parameter which specifies a subclass of ArgparseCompleter for custom completion behavior on this parser. If this is None or not present, then cmd2 will use @@ -921,7 +921,7 @@ def update_prog(self, prog: str) -> None: # 1. We can't use action._choices_actions because it excludes subcommands without help text. # 2. Since dictionaries are ordered and argparse inserts the primary name before aliases, # we assume the first time we encounter a parser, the key is the true subcommand name. - updated_parsers: set[argparse.ArgumentParser] = set() + updated_parsers: set[Cmd2ArgumentParser] = set() # Set the prog value for each subcommand's parser for subcmd_name, subcmd_parser in action.choices.items(): @@ -1055,7 +1055,6 @@ def _check_value(self, action: argparse.Action, value: Any) -> None: When displaying choices, use CompletionItem.value instead of the CompletionItem instance. - :param self: ArgumentParser instance :param action: the action being populated :param value: value from command line already run through conversion function by argparse """ @@ -1098,7 +1097,7 @@ def set(self, new_val: Any) -> None: def set_default_argument_parser_type(parser_type: type[Cmd2ArgumentParser]) -> None: - """Set the default ArgumentParser class for cmd2's built-in commands. + """Set the default Cmd2ArgumentParser class for cmd2's built-in commands. Since built-in commands rely on customizations made in Cmd2ArgumentParser, your custom parser class should inherit from Cmd2ArgumentParser. From d6a274c1f0b8ac5b65c3cb61eaa0573d5c7c383b Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 15:52:42 -0400 Subject: [PATCH 12/21] Updated save_help_text.py example script. --- examples/scripts/save_help_text.py | 48 +++++++++++++++++------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/examples/scripts/save_help_text.py b/examples/scripts/save_help_text.py index 41636b085..ddf53cc88 100644 --- a/examples/scripts/save_help_text.py +++ b/examples/scripts/save_help_text.py @@ -2,29 +2,29 @@ This is meant to be run within a cmd2 session using run_pyscript. """ -import argparse import os import sys from typing import TextIO +from cmd2 import Cmd2ArgumentParser + ASTERISKS = "********************************************************" -def get_sub_commands(parser: argparse.ArgumentParser) -> list[str]: - """Get a list of subcommands for an ArgumentParser.""" - sub_cmds = [] +def get_sub_commands(parser: Cmd2ArgumentParser) -> list[str]: + """Get a list of subcommands for a Cmd2ArgumentParser.""" + try: + subparsers_action = parser._get_subparsers_action() + except ValueError: + # No subcommands + return [] - # Check if this is parser has subcommands - if parser is not None and parser._subparsers is not None: - # Find the _SubParsersAction for the subcommands of this parser - for action in parser._subparsers._actions: - if isinstance(action, argparse._SubParsersAction): - for sub_cmd, sub_cmd_parser in action.choices.items(): - sub_cmds.append(sub_cmd) + sub_cmds = [] + for subcmd, subcmd_parser in subparsers_action.choices.items(): + sub_cmds.append(subcmd) - # Look for nested subcommands - sub_cmds.extend(f'{sub_cmd} {nested_sub_cmd}' for nested_sub_cmd in get_sub_commands(sub_cmd_parser)) - break + # Look for nested subcommands + sub_cmds.extend(f'{subcmd} {nested_subcmd}' for nested_subcmd in get_sub_commands(subcmd_parser)) sub_cmds.sort() return sub_cmds @@ -60,8 +60,7 @@ def main() -> None: # Open the output file outfile_path = os.path.expanduser(sys.argv[1]) try: - with open(outfile_path, 'w') as outfile: - pass + outfile = open(outfile_path, 'w') # noqa: SIM115 except OSError as e: print(f"Error opening {outfile_path} because: {e}") return @@ -83,11 +82,18 @@ def main() -> None: is_command = item in all_commands add_help_to_file(item, outfile, is_command) - if is_command: - # Add any subcommands - for subcmd in get_sub_commands(getattr(self.cmd_func(item), 'argparser', None)): - full_cmd = f'{item} {subcmd}' - add_help_to_file(full_cmd, outfile, is_command) + if not is_command: + continue + + cmd_func = self.cmd_func(item) + parser = self._command_parsers.get(cmd_func) + if parser is None: + continue + + # Add any subcommands + for subcmd in get_sub_commands(parser): + full_cmd = f'{item} {subcmd}' + add_help_to_file(full_cmd, outfile, is_command) outfile.close() print(f"Output written to {outfile_path}") From 7be23d2d6f20bf7a8059a7f063ba90553bf4716c Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 16:37:29 -0400 Subject: [PATCH 13/21] Added more tests. --- tests/test_cmd2.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index d0998f30a..6a312fa2a 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4419,3 +4419,75 @@ def test_auto_suggest_default(): """Test that auto_suggest defaults to True.""" app = cmd2.Cmd() assert isinstance(app.main_session.auto_suggest, AutoSuggestFromHistory) + + +def test_attach_subcommand() -> None: + import argparse + + class SubcmdApp(cmd2.Cmd): + def __init__(self) -> None: + super().__init__() + + root_parser = cmd2.Cmd2ArgumentParser() + root_parser.add_subparsers() + + @cmd2.with_argparser(root_parser) + def do_root(self, _args: argparse.Namespace) -> None: + pass + + app = SubcmdApp() + + # Verify root exists and uses argparse + root_parser = app._command_parsers.get(app.do_root) + assert root_parser is not None + + # Attach child to root + child_parser = cmd2.Cmd2ArgumentParser(prog="child") + child_parser.add_subparsers() + app.attach_subcommand("root", "child", child_parser, help="child help") + + # Verify child was attached + 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 + + # Attach grandchild to child + grandchild_parser = cmd2.Cmd2ArgumentParser(prog="grandchild") + app.attach_subcommand("root child", "grandchild", grandchild_parser) + + # Verify grandchild was attached + child_subparsers_action = child_parser._get_subparsers_action() + assert "grandchild" in child_subparsers_action._name_parser_map + + # Detach grandchild + detached_grandchild = app.detach_subcommand("root child", "grandchild") + assert detached_grandchild is grandchild_parser + assert "grandchild" not in child_subparsers_action._name_parser_map + + # Detach child + detached_child = app.detach_subcommand("root", "child") + assert detached_child is child_parser + assert "child" not in root_subparsers_action._name_parser_map + + +def test_attach_subcommand_errors() -> None: + class SubcmdErrorApp(cmd2.Cmd): + def __init__(self) -> None: + super().__init__() + + def do_no_argparse(self, _statement: cmd2.Statement) -> None: + pass + + app = SubcmdErrorApp() + + # Test empty command + with pytest.raises(ValueError, match="Command path cannot be empty"): + app.attach_subcommand("", "sub", cmd2.Cmd2ArgumentParser()) + + # Test non-existent command + with pytest.raises(ValueError, match="Root command 'fake' not found"): + app.attach_subcommand("fake", "sub", cmd2.Cmd2ArgumentParser()) + + # Test command that doesn't use argparse + with pytest.raises(ValueError, match="Command 'no_argparse' does not use argparse"): + app.attach_subcommand("no_argparse", "sub", cmd2.Cmd2ArgumentParser()) From ac5e8993d8d51a746478ecac8a89d176e6555d2e Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 21:06:23 -0400 Subject: [PATCH 14/21] Fixed tests on Python 3.14+. --- tests/test_argparse_custom.py | 52 ++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 0b3ae9dba..6b3d62334 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -565,53 +565,61 @@ def test_update_prog() -> None: """Test Cmd2ArgumentParser.update_prog() across various scenarios.""" # Set up a complex parser hierarchy - old_root = 'old_app' - parser = Cmd2ArgumentParser(prog=old_root) + old_app = 'old_app' + root = Cmd2ArgumentParser(prog=old_app) # Positionals before subcommand - parser.add_argument('pos1') + root.add_argument('pos1') # Mutually exclusive group with positionals - group = parser.add_mutually_exclusive_group(required=True) + group = root.add_mutually_exclusive_group(required=True) group.add_argument('posA', nargs='?') group.add_argument('posB', nargs='?') # Subparsers with aliases and no help text - subparsers = parser.add_subparsers(dest='cmd') + root_subparsers = root.add_subparsers(dest='cmd') # Subcommand with aliases - sub1 = subparsers.add_parser('sub1', aliases=['s1', 'alias1'], help='help for sub1') + sub1 = root_subparsers.add_parser('sub1', aliases=['s1', 'alias1'], help='help for sub1') # Subcommand with no help text - sub2 = subparsers.add_parser('sub2') + sub2 = root_subparsers.add_parser('sub2') # Nested subparser sub2.add_argument('inner_pos') sub2_subparsers = sub2.add_subparsers(dest='sub2_cmd') leaf = sub2_subparsers.add_parser('leaf', help='leaf help') - # Verify initial progs look correct - assert parser.prog == 'old_app' - assert sub1.prog == 'old_app pos1 (posA | posB) sub1' - assert sub2.prog == 'old_app pos1 (posA | posB) sub2' - assert leaf.prog == 'old_app pos1 (posA | posB) sub2 inner_pos leaf' + # Save initial prog values + orig_root_prog = root.prog + orig_sub1_prog = sub1.prog + orig_sub2_prog = sub2.prog + orig_leaf_prog = leaf.prog # Perform update - new_root = 'new_app' - parser.update_prog(new_root) + new_app = 'new_app' + root.update_prog(new_app) - # Verify new progs look correct - assert parser.prog == 'new_app' - assert sub1.prog == 'new_app pos1 (posA | posB) sub1' - assert sub2.prog == 'new_app pos1 (posA | posB) sub2' - assert leaf.prog == 'new_app pos1 (posA | posB) sub2 inner_pos leaf' + # Verify updated prog values + assert root.prog.startswith(new_app) + assert root.prog == orig_root_prog.replace(old_app, new_app, 1) + + assert sub1.prog.startswith(new_app) + assert sub1.prog == orig_sub1_prog.replace(old_app, new_app, 1) + + assert sub2.prog.startswith(new_app) + assert sub2.prog == orig_sub2_prog.replace(old_app, new_app, 1) + + assert leaf.prog.startswith(new_app) + assert leaf.prog == orig_leaf_prog.replace(old_app, new_app, 1) # Verify that action._prog_prefix was updated by adding a new subparser - sub3 = subparsers.add_parser('sub3') - assert sub3.prog == 'new_app pos1 (posA | posB) sub3' + sub3 = root_subparsers.add_parser('sub3') + assert sub3.prog.startswith(new_app) + assert sub3.prog == root_subparsers._prog_prefix + ' sub3' # Verify aliases still point to the correct parser - for action in parser._actions: + for action in root._actions: if isinstance(action, argparse._SubParsersAction): assert action.choices['s1'].prog == sub1.prog assert action.choices['alias1'].prog == sub1.prog From 5dc1f980d68b84dd9ac08d21625a2a2e23de086d Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 21:14:49 -0400 Subject: [PATCH 15/21] Prevent redundant traversal of parser aliases. --- cmd2/cmd2.py | 7 +++++++ examples/scripts/save_help_text.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index b6847807e..fd73bfa6c 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1034,7 +1034,14 @@ def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: # No subcommands to check return + # Prevent redundant traversal of parser aliases + checked_parsers: set[Cmd2ArgumentParser] = set() + for subparser in subparsers_action.choices.values(): + if subparser in checked_parsers: + continue + checked_parsers.add(subparser) + attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: raise CommandSetRegistrationError( diff --git a/examples/scripts/save_help_text.py b/examples/scripts/save_help_text.py index ddf53cc88..0a4ecb393 100644 --- a/examples/scripts/save_help_text.py +++ b/examples/scripts/save_help_text.py @@ -19,8 +19,15 @@ def get_sub_commands(parser: Cmd2ArgumentParser) -> list[str]: # No subcommands return [] + # Prevent redundant traversal of parser aliases + checked_parsers: set[Cmd2ArgumentParser] = set() + sub_cmds = [] for subcmd, subcmd_parser in subparsers_action.choices.items(): + if subcmd_parser in checked_parsers: + continue + checked_parsers.add(subcmd_parser) + sub_cmds.append(subcmd) # Look for nested subcommands From 13c574287d425a472faf36b1f0fa40c20a123d42 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 3 Apr 2026 22:13:51 -0400 Subject: [PATCH 16/21] Increased code coverage. --- tests/test_commandset.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_commandset.py b/tests/test_commandset.py index 330928f23..c0c6f5575 100644 --- a/tests/test_commandset.py +++ b/tests/test_commandset.py @@ -98,7 +98,8 @@ def do_main(self, args: argparse.Namespace) -> None: # main -> sub subcmd_parser = cmd2.Cmd2ArgumentParser(description="Sub Command") - @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command") + # Include aliases to cover the alias check in cmd2's check_parser_uninstallable(). + @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command", aliases="sub_alias") def subcmd_func(self, args: argparse.Namespace) -> None: self._cmd.poutput("Subcommand Ran") From 31af37617b7a8365130b40fcd1b542ac02508425 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 4 Apr 2026 00:46:06 -0400 Subject: [PATCH 17/21] Removed monkey patches and improved type hints. --- cmd2/argparse_custom.py | 148 ++++++++++------------------------ cmd2/cmd2.py | 13 ++- tests/test_argparse.py | 10 +-- tests/test_argparse_custom.py | 43 ---------- 4 files changed, 55 insertions(+), 159 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 48b28716d..47b4b502a 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -222,19 +222,6 @@ def get_choices(self) -> Choices: - ``action.get_()`` - ``action.set_(value)`` - -**Subcommand Manipulation** - -cmd2 has patched ``argparse._SubParsersAction`` with new functions to better facilitate the -addition and removal of subcommand parsers. - -``argparse._SubParsersAction.attach_parser`` - new function to attach -an existing ArgumentParser to a subparsers action. See ``_SubParsersAction_attach_parser`` -for more details. - -``argparse._SubParsersAction.detach_parser`` - new function to detach a -parser from a subparsers action. See ``_SubParsersAction_detach_parser`` for -more details. """ import argparse @@ -522,87 +509,6 @@ def _ActionsContainer_add_argument( # noqa: N802 setattr(argparse._ActionsContainer, 'add_argument', _ActionsContainer_add_argument) -############################################################################################################ -# Patch argparse._SubParsersAction to add attach_parser function -############################################################################################################ - - -def _SubParsersAction_attach_parser( # noqa: N802 - self: argparse._SubParsersAction, # type: ignore[type-arg] - name: str, - subcmd_parser: argparse.ArgumentParser, - **add_parser_kwargs: Any, -) -> None: - """Attach an existing parser to a subparsers action. - - This is useful when a parser is pre-configured (e.g. by cmd2's subcommand decorator) - and needs to be attached to a parent parser. - - This function is added by cmd2 as a method called ``attach_parser()`` - to ``argparse._SubParsersAction`` class. - - To call: ``action.attach_parser(name, subcmd_parser, **add_parser_kwargs)`` - - :param self: instance of the _SubParsersAction being edited - :param name: name of the subcommand to add - :param subcmd_parser: the parser for this new subcommand - :param add_parser_kwargs: registration-specific kwargs for add_parser() - (e.g. help, aliases, deprecated [Python 3.13+]) - """ - # Use add_parser to register the subcommand name and any aliases - self.add_parser(name, **add_parser_kwargs) - - # Replace the parser created by add_parser() with our pre-configured one - self._name_parser_map[name] = subcmd_parser - - # Remap any aliases to our pre-configured parser - for alias in add_parser_kwargs.get("aliases", ()): - self._name_parser_map[alias] = subcmd_parser - - -setattr(argparse._SubParsersAction, 'attach_parser', _SubParsersAction_attach_parser) - -############################################################################################################ -# Patch argparse._SubParsersAction to add detach_parser function -############################################################################################################ - - -def _SubParsersAction_detach_parser( # noqa: N802 - self: argparse._SubParsersAction, # type: ignore[type-arg] - name: str, -) -> argparse.ArgumentParser | None: - """Detach a parser from a subparsers action and return it. - - This function is added by cmd2 as a method called ``detach_parser()`` to ``argparse._SubParsersAction`` class. - - To call: ``action.detach_parser(name)`` - - :param self: instance of the _SubParsersAction being edited - :param name: name of the subcommand for the parser to detach - :return: the parser which was detached or None if the subcommand doesn't exist - """ - subparser = self._name_parser_map.get(name) - - if subparser is not None: - # Remove this subcommand and all its aliases from the base command - to_remove = [] - for cur_name, cur_parser in self._name_parser_map.items(): - if cur_parser is subparser: - to_remove.append(cur_name) - for cur_name in to_remove: - del self._name_parser_map[cur_name] - - # Remove this subcommand from its base command's help text - for choice_action in self._choices_actions: - if choice_action.dest == name: - self._choices_actions.remove(choice_action) - break - - return subparser - - -setattr(argparse._SubParsersAction, 'detach_parser', _SubParsersAction_detach_parser) - ############################################################################################################ # Unless otherwise noted, everything below this point are copied from Python's # argparse implementation with minor tweaks to adjust output. @@ -865,20 +771,26 @@ def __init__( self.ap_completer_type = ap_completer_type - def add_subparsers(self, **kwargs: Any) -> argparse._SubParsersAction: # type: ignore[type-arg] - """Add a subcommand parser. + def add_subparsers( # type: ignore[override] + self, + **kwargs: Any, + ) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": + """Override for improved defaults and type safety. - Set a default title if one was not given. + This override does two things. + 1. Sets a default title if one was not given. + 2. Narrows the return type to provide better IDE autocompletion + and type safety for `Cmd2ArgumentParser` instances. :param kwargs: additional keyword arguments - :return: argparse Subparser Action + :return: _SubParsersAction which stores Cmd2ArgumentParsers """ if 'title' not in kwargs: kwargs['title'] = 'subcommands' return super().add_subparsers(**kwargs) - def _get_subparsers_action(self) -> argparse._SubParsersAction: # type: ignore[type-arg] + def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": """Get the _SubParsersAction for this parser if it exists. :return: the _SubParsersAction for this parser @@ -951,7 +863,7 @@ def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': subparsers_action = parser._get_subparsers_action() if name not in subparsers_action.choices: raise ValueError(f"Subcommand '{name}' not found in '{parser.prog}'") - parser = cast(Cmd2ArgumentParser, subparsers_action.choices[name]) + parser = subparsers_action.choices[name] return parser def attach_subcommand( @@ -972,7 +884,19 @@ def attach_subcommand( """ target_parser = self._find_parser(subcommand_path) subparsers_action = target_parser._get_subparsers_action() - subparsers_action.attach_parser(subcommand, parser, **add_parser_kwargs) # type: ignore[attr-defined] + + # Use add_parser to register the subcommand name and any aliases + new_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs) + + # Ensure the parser and any nested subparsers have the correct 'prog' value. + parser.update_prog(new_parser.prog) + + # Replace the parser created by add_parser() with our pre-configured one + subparsers_action._name_parser_map[subcommand] = parser + + # Remap any aliases to our pre-configured parser + for alias in add_parser_kwargs.get("aliases", ()): + subparsers_action._name_parser_map[alias] = parser def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> 'Cmd2ArgumentParser': """Detach a subcommand from a command at the specified path. @@ -985,10 +909,26 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> """ target_parser = self._find_parser(subcommand_path) subparsers_action = target_parser._get_subparsers_action() - detached = subparsers_action.detach_parser(subcommand) # type: ignore[attr-defined] - if detached is None: + + subparser = subparsers_action._name_parser_map.get(subcommand) + if subparser is None: raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") - return cast(Cmd2ArgumentParser, detached) + + # 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 + + return subparser 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 fd73bfa6c..ad18c646d 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -273,7 +273,10 @@ def get(self, command_method: CommandFunc) -> Cmd2ArgumentParser | None: return None parent = self._cmd.find_commandset_for_command(command) or self._cmd - parser = self._cmd._build_parser(parent, parser_builder, command) + parser = self._cmd._build_parser(parent, parser_builder) + + # Ensure the parser and any nested subparsers have the correct 'prog' value. + parser.update_prog(command) # If the description has not been set, then use the method docstring if one exists if parser.description is None and command_method.__doc__: @@ -888,7 +891,6 @@ def _build_parser( self, parent: CmdOrSet, parser_builder: Cmd2ArgumentParser | Callable[[], Cmd2ArgumentParser] | StaticArgParseBuilder | ClassArgParseBuilder, - prog: str, ) -> Cmd2ArgumentParser: """Build argument parser for a command/subcommand. @@ -897,7 +899,6 @@ def _build_parser( parent's class to it. :param parser_builder: an existing Cmd2ArgumentParser instance or a factory (callable, staticmethod, or classmethod) that returns one. - :param prog: prog value to set in new parser :return: new parser :raises TypeError: if parser_builder is an invalid type or if the factory fails to return a Cmd2ArgumentParser @@ -920,8 +921,6 @@ def _build_parser( builder_name = getattr(parser_builder, "__name__", str(parser_builder)) # type: ignore[unreachable] raise TypeError(f"The parser returned by '{builder_name}' must be a Cmd2ArgumentParser or a subclass of it") - parser.update_prog(prog) - return parser def _install_command_function(self, command_func_name: str, command_method: CommandFunc, context: str = '') -> None: @@ -1047,7 +1046,7 @@ def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: raise CommandSetRegistrationError( f"Cannot uninstall CommandSet: '{subparser.prog}' is required by another CommandSet" ) - check_parser_uninstallable(cast(Cmd2ArgumentParser, subparser)) + check_parser_uninstallable(subparser) methods: list[tuple[str, Callable[..., Any]]] = inspect.getmembers( cmdset, @@ -1096,7 +1095,7 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: raise CommandSetRegistrationError(f'Subcommand {subcommand_name} is not valid: {errmsg}') # Create the subcommand parser and configure it - subcmd_parser = self._build_parser(cmdset, subcmd_parser_builder, f'{full_command_name} {subcommand_name}') + subcmd_parser = self._build_parser(cmdset, subcmd_parser_builder) if subcmd_parser.description is None and method.__doc__: subcmd_parser.description = strip_doc_annotations(method.__doc__) diff --git a/tests/test_argparse.py b/tests/test_argparse.py index b361e1502..13d567bf5 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -248,7 +248,7 @@ def test_preservelist(argparse_app) -> None: def test_invalid_parser_builder(argparse_app): parser_builder = None with pytest.raises(TypeError, match="Invalid type for parser_builder"): - argparse_app._build_parser(argparse_app, parser_builder, "fake_prog") + argparse_app._build_parser(argparse_app, parser_builder) def test_invalid_parser_return_type(argparse_app): @@ -256,7 +256,7 @@ def bad_builder(): return argparse.ArgumentParser() with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"): - argparse_app._build_parser(argparse_app, bad_builder, "fake_prog") + argparse_app._build_parser(argparse_app, bad_builder) def test_invalid_parser_return_type_staticmethod(argparse_app): @@ -266,7 +266,7 @@ def bad_builder(): sm = staticmethod(bad_builder) with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"): - argparse_app._build_parser(argparse_app, sm, "fake_prog") + argparse_app._build_parser(argparse_app, sm) def test_invalid_parser_return_type_classmethod(argparse_app): @@ -276,7 +276,7 @@ def bad_builder(cls): cm = classmethod(bad_builder) with pytest.raises(TypeError, match="must be a Cmd2ArgumentParser or a subclass of it"): - argparse_app._build_parser(argparse_app, cm, "fake_prog") + argparse_app._build_parser(argparse_app, cm) def test_invalid_parser_return_type_nameless_object(argparse_app): @@ -294,7 +294,7 @@ def __call__(self): expected_msg = f"The parser returned by '{builder}' must be a Cmd2ArgumentParser" with pytest.raises(TypeError, match=expected_msg): - argparse_app._build_parser(argparse_app, builder, "fake_prog") + argparse_app._build_parser(argparse_app, builder) def _build_has_subcmd_parser() -> cmd2.Cmd2ArgumentParser: diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 6b3d62334..20f798e90 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -342,49 +342,6 @@ def test_register_argparse_argument_parameter() -> None: delattr(argparse.Action, attr_name) -def test_parser_attachment() -> None: - """Test the monkey-patched attach_parser and detach_parser methods on argparse._SubParsersAction.""" - # Attach a parser as a subcommand - root_parser = Cmd2ArgumentParser(prog="root", description="root command") - root_subparsers = root_parser.add_subparsers() - - child_parser = Cmd2ArgumentParser(prog="child", description="child command") - root_subparsers.attach_parser( # type: ignore[attr-defined] - "child", - child_parser, - help="a child command", - aliases=["child_alias"], - ) - - # Verify the same parser instance was used - assert root_subparsers._name_parser_map["child"] is child_parser - assert root_subparsers._name_parser_map["child_alias"] is child_parser - - # Verify an action with the help text exists - child_action = None - for action in root_subparsers._choices_actions: - if action.dest == "child": - child_action = action - break - assert child_action is not None - assert child_action.help == "a child command" - - # Detatch the subcommand - detached_parser = root_subparsers.detach_parser("child") # type: ignore[attr-defined] - - # Verify subcommand and its aliases were removed - assert detached_parser is child_parser - assert "child" not in root_subparsers._name_parser_map - assert "child_alias" not in root_subparsers._name_parser_map - - # Verify the help text action was removed - choices_actions = [action.dest for action in root_subparsers._choices_actions] - assert "child" not in choices_actions - - # Verify it returns None when subcommand does not exist - assert root_subparsers.detach_parser("fake") is None # type: ignore[attr-defined] - - def test_subcommand_attachment() -> None: """Test Cmd2ArgumentParser convenience methods for attaching and detaching subcommands.""" From 82100df00ee411379d62a28661e5a60b0b229534 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 4 Apr 2026 01:01:04 -0400 Subject: [PATCH 18/21] Updated tests. --- tests/test_argparse_custom.py | 20 ++++++++++++++++---- tests/test_cmd2.py | 4 ++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 20f798e90..da5476cfe 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -411,11 +411,23 @@ def test_subcommand_attachment() -> None: assert "child" not in root_subparsers._name_parser_map assert "child_alias" not in root_subparsers._name_parser_map - ############################### - # Test error handling - ############################### - # Verify ValueError when path is invalid (find_parser fails) +def test_subcommand_attachment_errors() -> None: + root_parser = Cmd2ArgumentParser(prog="root", description="root command") + child_parser = Cmd2ArgumentParser(prog="child", description="child command") + + # Verify ValueError when subcommands are not supported + with pytest.raises(ValueError, match="Command 'root' does not support subcommands"): + root_parser.attach_subcommand([], "anything", child_parser) + with pytest.raises(ValueError, match="Command 'root' does not support subcommands"): + root_parser.detach_subcommand([], "anything") + + # Allow subcommands for the next tests + root_parser.add_subparsers() + + # 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"): root_parser.detach_subcommand(["nonexistent"], "anything") diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 6a312fa2a..e7293c30b 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4421,7 +4421,7 @@ def test_auto_suggest_default(): assert isinstance(app.main_session.auto_suggest, AutoSuggestFromHistory) -def test_attach_subcommand() -> None: +def test_subcommand_attachment() -> None: import argparse class SubcmdApp(cmd2.Cmd): @@ -4470,7 +4470,7 @@ def do_root(self, _args: argparse.Namespace) -> None: assert "child" not in root_subparsers_action._name_parser_map -def test_attach_subcommand_errors() -> None: +def test_subcommand_attachment_errors() -> None: class SubcmdErrorApp(cmd2.Cmd): def __init__(self) -> None: super().__init__() From a1e253aa81eb3e24a5ea479271b30e8f21e0cc52 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 4 Apr 2026 01:23:21 -0400 Subject: [PATCH 19/21] Updated comments. --- cmd2/argparse_custom.py | 3 ++- cmd2/cmd2.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 47b4b502a..2a381b1fe 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -888,7 +888,8 @@ def attach_subcommand( # Use add_parser to register the subcommand name and any aliases new_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs) - # Ensure the parser and any nested subparsers have the correct 'prog' value. + # To ensure accurate usage strings, recursively update 'prog' values + # within the injected parser to match its new location in the command hierarchy. parser.update_prog(new_parser.prog) # Replace the parser created by add_parser() with our pre-configured one diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index ad18c646d..d661509d7 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -275,7 +275,8 @@ def get(self, command_method: CommandFunc) -> Cmd2ArgumentParser | None: parent = self._cmd.find_commandset_for_command(command) or self._cmd parser = self._cmd._build_parser(parent, parser_builder) - # Ensure the parser and any nested subparsers have the correct 'prog' value. + # To ensure accurate usage strings, recursively update 'prog' values + # within the parser to match the command name. parser.update_prog(command) # If the description has not been set, then use the method docstring if one exists From caac18e00d1ce5fc6f86a73cb58ec8dce9babbbe Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 4 Apr 2026 13:26:38 -0400 Subject: [PATCH 20/21] Using argparse to build 'prog' prefix instead of trying to reproduce their code. --- cmd2/argparse_completer.py | 4 +- cmd2/argparse_custom.py | 111 ++++++++++++++++++++++------------ tests/test_argparse_custom.py | 16 ++--- 3 files changed, 83 insertions(+), 48 deletions(-) diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py index 26e96e27d..763d88538 100644 --- a/cmd2/argparse_completer.py +++ b/cmd2/argparse_completer.py @@ -27,7 +27,7 @@ from .argparse_custom import ( Cmd2ArgumentParser, - generate_range_error, + build_range_error, ) from .command_definition import CommandSet from .completion import ( @@ -137,7 +137,7 @@ def __init__(self, flag_arg_state: _ArgumentState) -> None: :param flag_arg_state: information about the unfinished flag action. """ arg = f'{argparse._get_action_name(flag_arg_state.action)}' - err = f'{generate_range_error(flag_arg_state.min, flag_arg_state.max)}' + err = f'{build_range_error(flag_arg_state.min, flag_arg_state.max)}' error = f"Error: argument {arg}: {err} ({flag_arg_state.count} entered)" super().__init__(error) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 2a381b1fe..23b6ad2ba 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -271,8 +271,8 @@ def get_choices(self) -> Choices: from .argparse_completer import ArgparseCompleter -def generate_range_error(range_min: int, range_max: float) -> str: - """Generate an error message when the the number of arguments provided is not within the expected range.""" +def build_range_error(range_min: int, range_max: float) -> str: + """Build an error message when the the number of arguments provided is not within the expected range.""" err_msg = "expected " if range_max == constants.INFINITY: @@ -577,7 +577,7 @@ def _set_color(self, color: bool, **kwargs: Any) -> None: super()._set_color(color) def _build_nargs_range_str(self, nargs_range: tuple[int, int | float]) -> str: - """Generate nargs range string for help text.""" + """Build nargs range string for help text.""" if nargs_range[1] == constants.INFINITY: # {min+} range_str = f"{{{nargs_range[0]}+}}" @@ -761,16 +761,17 @@ def __init__( conflict_handler=conflict_handler, add_help=add_help, allow_abbrev=allow_abbrev, - exit_on_error=exit_on_error, # added in Python 3.9 + exit_on_error=exit_on_error, **kwargs, # added in Python 3.14 ) - # Recast to assist type checkers since these can be Rich renderables in a Cmd2HelpFormatter. + self.ap_completer_type = ap_completer_type + + # To assist type checkers, recast these to reflect our usage of rich-argparse. + self.formatter_class: type[Cmd2HelpFormatter] self.description: RenderableType | None # type: ignore[assignment] self.epilog: RenderableType | None # type: ignore[assignment] - self.ap_completer_type = ap_completer_type - def add_subparsers( # type: ignore[override] self, **kwargs: Any, @@ -802,6 +803,48 @@ def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentPars return action raise ValueError(f"Command '{self.prog}' does not support subcommands") + def _build_subparsers_prog_prefix(self, positionals: list[argparse.Action]) -> str: + """Build the 'prog' prefix for a subparsers action. + + This prefix is stored in the _SubParsersAction's '_prog_prefix' attribute and + is used to construct the 'prog' attribute for its child parsers. It + typically consists of the current parser's 'prog' name followed by any + positional arguments that appear before the _SubParsersAction. + + This method uses a temporary Cmd2ArgumentParser to leverage argparse's + functionality for generating these strings. Subclasses can override this if + they need to change how subcommand 'prog' values are constructed (e.g., if + add_subparsers() was overridden with custom naming logic or if a different + formatting style is desired). + + Note: This method explicitly instantiates Cmd2ArgumentParser rather than + type(self) to avoid potential side effects or mandatory constructor + arguments in user-defined subclasses. + + :param positionals: positional arguments which appear before the _SubParsersAction + :return: the built 'prog' prefix + """ + # 1. usage=None: In Python < 3.14, this prevents the default usage + # string from affecting subparser prog strings. This was fixed in 3.14: + # https://github.com/python/cpython/commit/0cb4d6c6549d2299f7518f083bbe7d10314ecd66 + # + # 2. add_help=False: No need for a help action since we already know which + # actions are needed to build the prefix and have passed them in + # via the 'positionals' argument. + temp_parser = Cmd2ArgumentParser( + prog=self.prog, + usage=None, + formatter_class=self.formatter_class, + add_help=False, + ) + + # Inject the current positional state so add_subparsers() has the right context + temp_parser._actions = positionals + temp_parser._mutually_exclusive_groups = self._mutually_exclusive_groups + + # Call add_subparsers() to build _prog_prefix + return temp_parser.add_subparsers()._prog_prefix + def update_prog(self, prog: str) -> None: """Recursively update the prog attribute of this parser and all of its subparsers. @@ -810,47 +853,39 @@ def update_prog(self, prog: str) -> None: # Set the prog value for this parser self.prog = prog - if self._subparsers is None: + try: + subparsers_action = self._get_subparsers_action() + except ValueError: # This parser has no subcommands return - # argparse includes positional arguments that appear before the subcommand in its - # subparser prog strings. Track these while iterating through actions. + # Get all positional arguments which appear before the subcommand. positionals: list[argparse.Action] = [] - - # Set the prog value for the parser's subcommands for action in self._actions: - if isinstance(action, argparse._SubParsersAction): - # Use a formatter to generate _prog_prefix exactly as argparse does in - # add_subparsers(). This ensures that any subcommands added later via - # add_parser() will have the correct prog value. - formatter = self._get_formatter() - formatter.add_usage(self.usage, positionals, self._mutually_exclusive_groups, '') - action._prog_prefix = formatter.format_help().strip() - - # Note: action.choices contains both subcommand names and aliases. - # To ensure subcommands (and not aliases) are used in 'prog': - # 1. We can't use action._choices_actions because it excludes subcommands without help text. - # 2. Since dictionaries are ordered and argparse inserts the primary name before aliases, - # we assume the first time we encounter a parser, the key is the true subcommand name. - updated_parsers: set[Cmd2ArgumentParser] = set() - - # Set the prog value for each subcommand's parser - for subcmd_name, subcmd_parser in action.choices.items(): - if subcmd_parser in updated_parsers: - continue - - subcmd_prog = f"{action._prog_prefix} {subcmd_name}" - subcmd_parser.update_prog(subcmd_prog) # type: ignore[attr-defined] - updated_parsers.add(subcmd_parser) - - # We can break since argparse only allows 1 group of subcommands per level + if action is subparsers_action: break # Save positional argument if not action.option_strings: positionals.append(action) + # Update _prog_prefix. This ensures that any subcommands added later via + # add_parser() will have the correct prog value. + subparsers_action._prog_prefix = self._build_subparsers_prog_prefix(positionals) + + # subparsers_action.choices includes aliases. Since primary names are inserted first, + # we skip already updated parsers to ensure primary names are used in 'prog'. + updated_parsers: set[Cmd2ArgumentParser] = set() + + # Set the prog value for each subcommand's parser + for subcmd_name, subcmd_parser in subparsers_action.choices.items(): + if subcmd_parser in updated_parsers: + continue + + subcmd_prog = f"{subparsers_action._prog_prefix} {subcmd_name}" + subcmd_parser.update_prog(subcmd_prog) + updated_parsers.add(subcmd_parser) + def _find_parser(self, subcommand_path: Iterable[str]) -> 'Cmd2ArgumentParser': """Find a parser in the hierarchy based on a sequence of subcommand names. @@ -987,7 +1022,7 @@ def _match_argument(self, action: argparse.Action, arg_strings_pattern: str) -> if match is None: nargs_range = action.get_nargs_range() # type: ignore[attr-defined] if nargs_range is not None: - raise ArgumentError(action, generate_range_error(nargs_range[0], nargs_range[1])) + raise ArgumentError(action, build_range_error(nargs_range[0], nargs_range[1])) return super()._match_argument(action, arg_strings_pattern) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index da5476cfe..d4f29688e 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -14,7 +14,7 @@ ) from cmd2.argparse_custom import ( Cmd2HelpFormatter, - generate_range_error, + build_range_error, register_argparse_argument_parameter, ) from cmd2.rich_utils import Cmd2RichArgparseConsole @@ -258,26 +258,26 @@ def test_apcustom_print_message(capsys) -> None: assert test_message in err -def test_generate_range_error() -> None: +def test_build_range_error() -> None: # max is INFINITY - err_msg = generate_range_error(1, constants.INFINITY) + err_msg = build_range_error(1, constants.INFINITY) assert err_msg == "expected at least 1 argument" - err_msg = generate_range_error(2, constants.INFINITY) + err_msg = build_range_error(2, constants.INFINITY) assert err_msg == "expected at least 2 arguments" # min and max are equal - err_msg = generate_range_error(1, 1) + err_msg = build_range_error(1, 1) assert err_msg == "expected 1 argument" - err_msg = generate_range_error(2, 2) + err_msg = build_range_error(2, 2) assert err_msg == "expected 2 arguments" # min and max are not equal - err_msg = generate_range_error(0, 1) + err_msg = build_range_error(0, 1) assert err_msg == "expected 0 to 1 argument" - err_msg = generate_range_error(0, 2) + err_msg = build_range_error(0, 2) assert err_msg == "expected 0 to 2 arguments" From f8d032f3528de2e40ca2fbe0c4f3f839466d989f Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Sat, 4 Apr 2026 14:55:25 -0400 Subject: [PATCH 21/21] Fixed aliases type in test. --- tests/test_commandset.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_commandset.py b/tests/test_commandset.py index c0c6f5575..69129106d 100644 --- a/tests/test_commandset.py +++ b/tests/test_commandset.py @@ -99,7 +99,7 @@ def do_main(self, args: argparse.Namespace) -> None: subcmd_parser = cmd2.Cmd2ArgumentParser(description="Sub Command") # Include aliases to cover the alias check in cmd2's check_parser_uninstallable(). - @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command", aliases="sub_alias") + @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command", aliases=["sub_alias"]) def subcmd_func(self, args: argparse.Namespace) -> None: self._cmd.poutput("Subcommand Ran")