Removed monkey patches and improved type hints.

Author: kmvanbrunt

cmd2/argparse_custom.py

@@ -222,19 +222,6 @@ def get_choices(self) -> Choices:
 
 - ``action.get_<name>()``
 - ``action.set_<name>(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."""

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__)
 

tests/test_argparse.py

@@ -248,15 +248,15 @@ 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):
     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:

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."""