From 40bf34a37003d62d5483675579584dcc4beae202 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 17 Apr 2026 14:13:30 -0400 Subject: [PATCH 1/4] Enforcing that a subparser's type match the _SubparsersAction's parser class. --- cmd2/argparse_custom.py | 23 ++++++++++++++++------- cmd2/cmd2.py | 8 +++++--- tests/test_argparse_custom.py | 27 +++++++++++++++++++++++++++ tests/test_cmd2.py | 6 ++++++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 1db0f858c..7420b9542 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -890,7 +890,7 @@ 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 + :return: the discovered parser :raises ValueError: if any subcommand in the path is not found or a level doesn't support subcommands """ parser = self @@ -905,7 +905,7 @@ def attach_subcommand( self, subcommand_path: Iterable[str], subcommand: str, - parser: 'Cmd2ArgumentParser', + subcommand_parser: 'Cmd2ArgumentParser', **add_parser_kwargs: Any, ) -> None: """Attach a parser as a subcommand to a command at the specified path. @@ -913,26 +913,35 @@ def attach_subcommand( :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 subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :raises TypeError: if the subcommand parser's type does not match the 'parser_class' configured + for the target subcommand group. :raises ValueError: if the command path is invalid or doesn't support subcommands """ target_parser = self._find_parser(subcommand_path) subparsers_action = target_parser._get_subparsers_action() + if type(subcommand_parser) is not subparsers_action._parser_class: + raise TypeError( + f"The attached parser must be of type '{subparsers_action._parser_class.__name__}' " + f"to match the 'parser_class' configured for this subparsers action. " + f"Received '{type(subcommand_parser).__name__}'." + ) + # Use add_parser to register the subcommand name and any aliases - new_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs) + placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs) # 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) + subcommand_parser.update_prog(placeholder_parser.prog) # Replace the parser created by add_parser() with our pre-configured one - subparsers_action._name_parser_map[subcommand] = parser + subparsers_action._name_parser_map[subcommand] = 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 + subparsers_action._name_parser_map[alias] = subcommand_parser def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) -> 'Cmd2ArgumentParser': """Detach a subcommand from a command at the specified path. diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index cbfefdfa1..df54b8bb7 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1179,7 +1179,7 @@ def attach_subcommand( self, command: str, subcommand: str, - parser: Cmd2ArgumentParser, + subcommand_parser: Cmd2ArgumentParser, **add_parser_kwargs: Any, ) -> None: """Attach a parser as a subcommand to a command at the specified path. @@ -1187,12 +1187,14 @@ def attach_subcommand( :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 subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :raises TypeError: if the subcommand parser's type does not match the 'parser_class' configured + for the target subcommand group. :raises ValueError: if the command path is invalid or doesn't support subcommands """ root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) - root_parser.attach_subcommand(subcommand_path, subcommand, parser, **add_parser_kwargs) + root_parser.attach_subcommand(subcommand_path, subcommand, subcommand_parser, **add_parser_kwargs) def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser: """Detach a subcommand from a command at the specified path. diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 7a333295b..749787763 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -425,6 +425,33 @@ def test_subcommand_attachment_errors() -> None: with pytest.raises(ValueError, match="Subcommand 'fake' not found in 'root'"): root_parser.detach_subcommand([], "fake") + # Verify TypeError when attaching a parser of a different type + class SubParser(Cmd2ArgumentParser): + pass + + subclass_parser = SubParser(prog="sub") + with pytest.raises(TypeError, match="The attached parser must be of type 'Cmd2ArgumentParser'"): + root_parser.attach_subcommand([], "sub", subclass_parser) + + +def test_subcommand_attachment_parser_class_override() -> None: + class MyParser(Cmd2ArgumentParser): + pass + + root_parser = Cmd2ArgumentParser(prog="root") + + # Explicitly override parser_class for this subparsers action + root_parser.add_subparsers(parser_class=MyParser) + + # Attaching a MyParser instance should succeed + my_parser = MyParser(prog="sub") + root_parser.attach_subcommand([], "sub", my_parser) + + # Attaching a standard Cmd2ArgumentParser instance should fail + standard_parser = Cmd2ArgumentParser(prog="standard") + with pytest.raises(TypeError, match="The attached parser must be of type 'MyParser'"): + root_parser.attach_subcommand([], "fail", standard_parser) + def test_completion_items_as_choices(capsys) -> None: """Test cmd2's patch to Argparse._check_value() which supports CompletionItems as choices. diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index d17427f46..6d1ef56a9 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4534,3 +4534,9 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None: # 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()) + + # Test type mismatch + import argparse + + with pytest.raises(TypeError, match="to match the 'parser_class' configured for this subparsers action"): + app.attach_subcommand("alias", "sub", argparse.ArgumentParser()) # type: ignore[arg-type] From bbfab98dd33ceae7976bb2b4d6048d7452c44791 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 17 Apr 2026 14:23:02 -0400 Subject: [PATCH 2/4] Removed Cmd2ArgumentParser.add_subparsers() override. --- CHANGELOG.md | 3 +++ cmd2/argparse_custom.py | 19 ------------------- cmd2/cmd2.py | 4 ++-- cmd2/decorators.py | 2 +- 4 files changed, 6 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5bd378bd..131f1074b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,9 @@ prompt is displayed. the Enhancements section below for details). - Removed `Cmd.undoc_header` since all commands are now considered categorized. - Renamed `Cmd.cmd_func()` to `Cmd.get_command_func()`. + - `cmd2` no longer sets a default title for a subparsers group. If you desire a title, you will + need to pass one in like this `parser.add_subparsers(title="subcommands")`. This is standard + `argparse` behavior. - 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 7420b9542..086bfe233 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -772,25 +772,6 @@ def __init__( self.description: RenderableType | None # type: ignore[assignment] self.epilog: RenderableType | None # type: ignore[assignment] - def add_subparsers( # type: ignore[override] - self, - **kwargs: Any, - ) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": - """Override for improved defaults and type safety. - - 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: _SubParsersAction which stores Cmd2ArgumentParsers - """ - if 'title' not in kwargs: - kwargs['title'] = 'subcommands' - - return super().add_subparsers(**kwargs) - def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]": """Get the _SubParsersAction for this parser if it exists. diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index df54b8bb7..37aeb9e75 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -3728,7 +3728,7 @@ def _build_alias_parser() -> Cmd2ArgumentParser: "See Also", "macro", ) - alias_parser.add_subparsers(metavar='SUBCOMMAND', required=True) + alias_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True) return alias_parser @@ -3944,7 +3944,7 @@ def _build_macro_parser() -> Cmd2ArgumentParser: "See Also", "alias", ) - macro_parser.add_subparsers(metavar='SUBCOMMAND', required=True) + macro_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True) return macro_parser diff --git a/cmd2/decorators.py b/cmd2/decorators.py index e66b1a729..41e1d391b 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -370,7 +370,7 @@ def as_subcommand_to( Example: ```py base_parser = cmd2.Cmd2ArgumentParser() - base_parser.add_subparsers(metavar='SUBCOMMAND', required=True) + base_parser.add_subparsers(title="subcommands", metavar="SUBCOMMAND", required=True) sub_parser = cmd2.Cmd2ArgumentParser() class MyApp(cmd2.Cmd): From 42371e699de6b7aa17033fe36bf6ba875ed7671f Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 17 Apr 2026 15:44:22 -0400 Subject: [PATCH 3/4] Confirm attached parser is a Cmd2ArgumentParser-derived class. --- cmd2/argparse_custom.py | 12 ++++++++++-- cmd2/cmd2.py | 5 +++-- tests/test_argparse_custom.py | 9 +++++++-- tests/test_cmd2.py | 6 ------ 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 086bfe233..e49307c5f 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -896,13 +896,21 @@ def attach_subcommand( :param subcommand: name of the new subcommand :param subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) - :raises TypeError: if the subcommand parser's type does not match the 'parser_class' configured - for the target subcommand group. + :raises TypeError: if the subcommand parser is not an instance of 'Cmd2ArgumentParser' + (or one of its subclasses), or if its type does not match the 'parser_class' + configured for the target subcommand group. :raises ValueError: if the command path is invalid or doesn't support subcommands """ + if not isinstance(subcommand_parser, Cmd2ArgumentParser): + raise TypeError( + f"The attached parser must be an instance of 'Cmd2ArgumentParser' (or a subclass). " + f"Received: '{type(subcommand_parser).__name__}'." + ) + target_parser = self._find_parser(subcommand_path) subparsers_action = target_parser._get_subparsers_action() + # Mirror argparse's add_parser() behavior by requiring an exact type match with _parser_class if type(subcommand_parser) is not subparsers_action._parser_class: raise TypeError( f"The attached parser must be of type '{subparsers_action._parser_class.__name__}' " diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 37aeb9e75..e2bd17c22 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1189,8 +1189,9 @@ def attach_subcommand( :param subcommand: name of the new subcommand :param subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) - :raises TypeError: if the subcommand parser's type does not match the 'parser_class' configured - for the target subcommand group. + :raises TypeError: if the subcommand parser is not an instance of 'Cmd2ArgumentParser' + (or one of its subclasses), or if its type does not match the 'parser_class' + configured for the target subcommand group. :raises ValueError: if the command path is invalid or doesn't support subcommands """ root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 749787763..c214bdc28 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -425,12 +425,17 @@ def test_subcommand_attachment_errors() -> None: with pytest.raises(ValueError, match="Subcommand 'fake' not found in 'root'"): root_parser.detach_subcommand([], "fake") + # Verify TypeError when attaching a non-Cmd2ArgumentParser type + ap_parser = argparse.ArgumentParser(prog="non-cmd2-parser") + with pytest.raises(TypeError, match=r"must be an instance of 'Cmd2ArgumentParser' \(or a subclass\)"): + root_parser.attach_subcommand([], "sub", ap_parser) # type: ignore[arg-type] + # Verify TypeError when attaching a parser of a different type class SubParser(Cmd2ArgumentParser): pass - subclass_parser = SubParser(prog="sub") - with pytest.raises(TypeError, match="The attached parser must be of type 'Cmd2ArgumentParser'"): + subclass_parser = SubParser(prog="subclass") + with pytest.raises(TypeError, match="to match the 'parser_class' configured for this subparsers action"): root_parser.attach_subcommand([], "sub", subclass_parser) diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index 6d1ef56a9..d17427f46 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4534,9 +4534,3 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None: # 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()) - - # Test type mismatch - import argparse - - with pytest.raises(TypeError, match="to match the 'parser_class' configured for this subparsers action"): - app.attach_subcommand("alias", "sub", argparse.ArgumentParser()) # type: ignore[arg-type] From 156d158d28efac31b750d2fe9e5f8a0d0e2371a7 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Fri, 17 Apr 2026 17:01:44 -0400 Subject: [PATCH 4/4] Allow attached parser to be a subclass of _SubparsersAction's parser class. --- cmd2/argparse_custom.py | 19 +++++++++++-------- cmd2/cmd2.py | 6 +++--- tests/test_argparse_custom.py | 17 ++++++++--------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index e49307c5f..8cb7c7e95 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -896,9 +896,9 @@ def attach_subcommand( :param subcommand: name of the new subcommand :param subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) - :raises TypeError: if the subcommand parser is not an instance of 'Cmd2ArgumentParser' - (or one of its subclasses), or if its type does not match the 'parser_class' - configured for the target subcommand group. + :raises TypeError: if subcommand_parser is not an instance of the following or their subclasses: + 1. Cmd2ArgumentParser + 2. The parser_class configured for the target subcommand group :raises ValueError: if the command path is invalid or doesn't support subcommands """ if not isinstance(subcommand_parser, Cmd2ArgumentParser): @@ -910,12 +910,15 @@ def attach_subcommand( target_parser = self._find_parser(subcommand_path) subparsers_action = target_parser._get_subparsers_action() - # Mirror argparse's add_parser() behavior by requiring an exact type match with _parser_class - if type(subcommand_parser) is not subparsers_action._parser_class: + # Verify the parser is compatible with the 'parser_class' configured for this + # subcommand group. We use isinstance() here to allow for subclasses, providing + # more flexibility than the standard add_parser() factory approach which enforces + # a specific class. + if not isinstance(subcommand_parser, subparsers_action._parser_class): raise TypeError( - f"The attached parser must be of type '{subparsers_action._parser_class.__name__}' " - f"to match the 'parser_class' configured for this subparsers action. " - f"Received '{type(subcommand_parser).__name__}'." + f"The attached parser must be an instance of '{subparsers_action._parser_class.__name__}' " + f"(or a subclass) to match the 'parser_class' configured for this subcommand group. " + f"Received: '{type(subcommand_parser).__name__}'." ) # Use add_parser to register the subcommand name and any aliases diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index e2bd17c22..0dcc5c6c3 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1189,9 +1189,9 @@ def attach_subcommand( :param subcommand: name of the new subcommand :param subcommand_parser: the parser to attach :param add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) - :raises TypeError: if the subcommand parser is not an instance of 'Cmd2ArgumentParser' - (or one of its subclasses), or if its type does not match the 'parser_class' - configured for the target subcommand group. + :raises TypeError: if subcommand_parser is not an instance of the following or their subclasses: + 1. Cmd2ArgumentParser + 2. The parser_class configured for the target subcommand group :raises ValueError: if the command path is invalid or doesn't support subcommands """ root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index c214bdc28..0ac393b49 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -430,19 +430,14 @@ def test_subcommand_attachment_errors() -> None: with pytest.raises(TypeError, match=r"must be an instance of 'Cmd2ArgumentParser' \(or a subclass\)"): root_parser.attach_subcommand([], "sub", ap_parser) # type: ignore[arg-type] - # Verify TypeError when attaching a parser of a different type - class SubParser(Cmd2ArgumentParser): - pass - - subclass_parser = SubParser(prog="subclass") - with pytest.raises(TypeError, match="to match the 'parser_class' configured for this subparsers action"): - root_parser.attach_subcommand([], "sub", subclass_parser) - def test_subcommand_attachment_parser_class_override() -> None: class MyParser(Cmd2ArgumentParser): pass + class MySubParser(MyParser): + pass + root_parser = Cmd2ArgumentParser(prog="root") # Explicitly override parser_class for this subparsers action @@ -452,9 +447,13 @@ class MyParser(Cmd2ArgumentParser): my_parser = MyParser(prog="sub") root_parser.attach_subcommand([], "sub", my_parser) + # Attaching a MySubParser instance should also succeed (isinstance check) + my_sub_parser = MySubParser(prog="sub2") + root_parser.attach_subcommand([], "sub2", my_sub_parser) + # Attaching a standard Cmd2ArgumentParser instance should fail standard_parser = Cmd2ArgumentParser(prog="standard") - with pytest.raises(TypeError, match="The attached parser must be of type 'MyParser'"): + with pytest.raises(TypeError, match=r"must be an instance of 'MyParser' \(or a subclass\)"): root_parser.attach_subcommand([], "fail", standard_parser)