Making more of the subcommand management API public.

Author: kmvanbrunt

cmd2/argparse_utils.py

@@ -494,6 +494,52 @@ def _ActionsContainer_add_argument(  # noqa: N802
 # Overwrite _ActionsContainer.add_argument with our patch
 argparse._ActionsContainer.add_argument = _ActionsContainer_add_argument  # type: ignore[method-assign]
 
+############################################################################################################
+# Patch argparse._SubParsersAction by adding remove_parser() function
+############################################################################################################
+
+
+def _SubParsersAction_remove_parser(  # noqa: N802
+    self: argparse._SubParsersAction,  # type: ignore[type-arg]
+    name: str,
+) -> argparse.ArgumentParser:
+    """Remove a subparser from a subparsers group.
+
+    This function is added by cmd2 as a method called ``remove_parser()``
+    to ``argparse._SubParsersAction`` class.
+
+    To call: ``action.remove_parser(name)``
+
+    :param self: instance of the _SubParsersAction being edited
+    :param name: name of the subcommand for the subparser to remove
+    :return: the removed parser
+    :raises ValueError: if the subcommand doesn't exist
+    """
+    if name not in self._name_parser_map:
+        raise ValueError(f"Subcommand '{name}' not found")
+
+    subparser = self._name_parser_map[name]
+
+    # Find all names (primary and aliases) that map to this subparser
+    all_names = [cur_name for cur_name, cur_parser in self._name_parser_map.items() if cur_parser is subparser]
+
+    # Remove the help entry for this subparser. To handle the case where
+    # name is an alias, we remove the action whose 'dest' matches any of
+    # the names mapped to this subparser.
+    for choice_action in self._choices_actions:
+        if choice_action.dest in all_names:
+            self._choices_actions.remove(choice_action)
+            break
+
+    # Remove all references to this subparser, including aliases.
+    for cur_name in all_names:
+        del self._name_parser_map[cur_name]
+
+    return cast(argparse.ArgumentParser, subparser)
+
+
+argparse._SubParsersAction.remove_parser = _SubParsersAction_remove_parser  # type: ignore[attr-defined]
+
 
 class Cmd2ArgumentParser(argparse.ArgumentParser):
     """Custom ArgumentParser class that improves error and help output."""
@@ -556,7 +602,7 @@ def __init__(
         self.description: RenderableType | None  # type: ignore[assignment]
         self.epilog: RenderableType | None  # type: ignore[assignment]
 
-    def _get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]":
+    def get_subparsers_action(self) -> "argparse._SubParsersAction[Cmd2ArgumentParser]":
         """Get the _SubParsersAction for this parser if it exists.
 
         :return: the _SubParsersAction for this parser
@@ -619,7 +665,7 @@ def update_prog(self, prog: str) -> None:
         self.prog = prog
 
         try:
-            subparsers_action = self._get_subparsers_action()
+            subparsers_action = self.get_subparsers_action()
         except ValueError:
             # This parser has no subcommands
             return
@@ -651,7 +697,7 @@ def update_prog(self, prog: str) -> None:
             subcmd_parser.update_prog(subcmd_prog)
             updated_parsers.add(subcmd_parser)
 
-    def _find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser":
+    def find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser":
         """Find a parser in the hierarchy based on a sequence of subcommand names.
 
         :param subcommand_path: sequence of subcommand names leading to the target parser
@@ -660,7 +706,7 @@ def _find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser":
         """
         parser = self
         for name in subcommand_path:
-            subparsers_action = parser._get_subparsers_action()
+            subparsers_action = parser.get_subparsers_action()
             if name not in subparsers_action.choices:
                 raise ValueError(f"Subcommand '{name}' not found in '{parser.prog}'")
             parser = subparsers_action.choices[name]
@@ -691,8 +737,8 @@ def attach_subcommand(
                 f"Received: '{type(subcommand_parser).__name__}'."
             )
 
-        target_parser = self._find_parser(subcommand_path)
-        subparsers_action = target_parser._get_subparsers_action()
+        target_parser = self.find_parser(subcommand_path)
+        subparsers_action = target_parser.get_subparsers_action()
 
         # Verify the parser is compatible with the 'parser_class' configured for this
         # subcommand group. We use isinstance() here to allow for subclasses, providing
@@ -728,28 +774,16 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) ->
         :return: the detached parser
         :raises ValueError: if the command path is invalid or the subcommand doesn't exist
         """
-        target_parser = self._find_parser(subcommand_path)
-        subparsers_action = target_parser._get_subparsers_action()
-
-        subparser = subparsers_action._name_parser_map.get(subcommand)
-        if subparser is None:
-            raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'")
-
-        # Remove this subcommand and all its aliases from the base command
-        to_remove = []
-        for cur_name, cur_parser in subparsers_action._name_parser_map.items():
-            if cur_parser is subparser:
-                to_remove.append(cur_name)
-        for cur_name in to_remove:
-            del subparsers_action._name_parser_map[cur_name]
-
-        # Remove this subcommand from its base command's help text
-        for choice_action in subparsers_action._choices_actions:
-            if choice_action.dest == subcommand:
-                subparsers_action._choices_actions.remove(choice_action)
-                break
+        target_parser = self.find_parser(subcommand_path)
+        subparsers_action = target_parser.get_subparsers_action()
 
-        return subparser
+        try:
+            return cast(
+                Cmd2ArgumentParser,
+                subparsers_action.remove_parser(subcommand),  # type: ignore[attr-defined]
+            )
+        except ValueError:
+            raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") from None
 
     def error(self, message: str) -> NoReturn:
         """Override that applies custom formatting to the error message."""

cmd2/cmd2.py

@@ -1026,7 +1026,7 @@ def _check_uninstallable(self, cmdset: CommandSet[Any]) -> None:
 
         def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None:
             try:
-                subparsers_action = parser._get_subparsers_action()
+                subparsers_action = parser.get_subparsers_action()
             except ValueError:
                 # No subcommands to check
                 return

examples/scripts/save_help_text.py

@@ -14,7 +14,7 @@
 def get_sub_commands(parser: Cmd2ArgumentParser) -> list[str]:
     """Get a list of subcommands for a Cmd2ArgumentParser."""
     try:
-        subparsers_action = parser._get_subparsers_action()
+        subparsers_action = parser.get_subparsers_action()
     except ValueError:
         # No subcommands
         return []

tests/test_argparse_utils.py

@@ -367,9 +367,9 @@ def test_subcommand_attachment() -> None:
     # Verify hierarchy navigation
     ###############################
 
-    assert root_parser._find_parser(["child", "grandchild"]) is grandchild_parser
-    assert root_parser._find_parser(["child"]) is child_parser
-    assert root_parser._find_parser([]) is root_parser
+    assert root_parser.find_parser(["child", "grandchild"]) is grandchild_parser
+    assert root_parser.find_parser(["child"]) is child_parser
+    assert root_parser.find_parser([]) is root_parser
 
     ###############################
     # Verify attachments
@@ -382,6 +382,10 @@ def test_subcommand_attachment() -> None:
     # Verify grandchild attachment
     assert child_subparsers._name_parser_map["grandchild"] is grandchild_parser
 
+    # Verify help entries are present
+    assert any(action.dest == "child" for action in root_subparsers._choices_actions)
+    assert any(action.dest == "grandchild" for action in child_subparsers._choices_actions)
+
     ###############################
     # Detach subcommands
     ###############################
@@ -390,12 +394,49 @@ def test_subcommand_attachment() -> None:
     detached_grandchild = root_parser.detach_subcommand(["child"], "grandchild")
     assert detached_grandchild is grandchild_parser
     assert "grandchild" not in child_subparsers._name_parser_map
+    assert not any(action.dest == "grandchild" for action in child_subparsers._choices_actions)
 
     # Detach child from root
     detached_child = root_parser.detach_subcommand([], "child")
     assert detached_child is child_parser
     assert "child" not in root_subparsers._name_parser_map
     assert "child_alias" not in root_subparsers._name_parser_map
+    assert not any(action.dest == "child" for action in root_subparsers._choices_actions)
+
+
+def test_detach_subcommand_by_alias() -> None:
+    """Test detaching a subcommand using one of its alias names."""
+    root_parser = Cmd2ArgumentParser(prog="root")
+    root_subparsers = root_parser.add_subparsers()
+
+    child_parser = Cmd2ArgumentParser(prog="child")
+    root_parser.attach_subcommand(
+        [],
+        "child",
+        child_parser,
+        help="a child command",
+        aliases=["alias1", "alias2"],
+    )
+
+    # Verify all names map to the parser
+    assert root_subparsers._name_parser_map["child"] is child_parser
+    assert root_subparsers._name_parser_map["alias1"] is child_parser
+    assert root_subparsers._name_parser_map["alias2"] is child_parser
+
+    # Verify help entry is present
+    assert any(action.dest == "child" for action in root_subparsers._choices_actions)
+
+    # Detach using an alias
+    detached = root_parser.detach_subcommand([], "alias1")
+    assert detached is child_parser
+
+    # Verify all names are gone
+    assert "child" not in root_subparsers._name_parser_map
+    assert "alias1" not in root_subparsers._name_parser_map
+    assert "alias2" not in root_subparsers._name_parser_map
+
+    # Verify help entry is gone
+    assert not any(action.dest == "child" for action in root_subparsers._choices_actions)
 
 
 def test_subcommand_attachment_errors() -> None:
@@ -411,7 +452,7 @@ def test_subcommand_attachment_errors() -> None:
     # Allow subcommands for the next tests
     root_parser.add_subparsers()
 
-    # Verify ValueError when path is invalid (_find_parser() fails)
+    # Verify ValueError when path is invalid (find_parser() fails)
     with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):
         root_parser.attach_subcommand(["nonexistent"], "anything", child_parser)
     with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):

tests/test_cmd2.py

@@ -4490,7 +4490,7 @@ def do_root(self, _args: argparse.Namespace) -> None:
     app.attach_subcommand("root", "child", child_parser, help="child help")
 
     # Verify child was attached
-    root_subparsers_action = root_parser._get_subparsers_action()
+    root_subparsers_action = root_parser.get_subparsers_action()
     assert "child" in root_subparsers_action._name_parser_map
     assert root_subparsers_action._name_parser_map["child"] is child_parser
 
@@ -4499,7 +4499,7 @@ def do_root(self, _args: argparse.Namespace) -> None:
     app.attach_subcommand("root child", "grandchild", grandchild_parser)
 
     # Verify grandchild was attached
-    child_subparsers_action = child_parser._get_subparsers_action()
+    child_subparsers_action = child_parser.get_subparsers_action()
     assert "grandchild" in child_subparsers_action._name_parser_map
 
     # Detach grandchild