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 1db0f858c..8cb7c7e95 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. @@ -890,7 +871,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 +886,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 +894,46 @@ 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 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): + 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() + # 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 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 - 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..0dcc5c6c3 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,15 @@ 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 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) - 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. @@ -3726,7 +3729,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 @@ -3942,7 +3945,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): diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 7a333295b..0ac393b49 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -425,6 +425,37 @@ 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] + + +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 + 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 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=r"must be an instance of 'MyParser' \(or a subclass\)"): + 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.