Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions cmd2/argparse_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ def _SubParsersAction_remove_parser( # noqa: N802
:raises ValueError: if the subcommand doesn't exist
"""
if name not in self._name_parser_map:
raise ValueError(f"Subcommand '{name}' not found")
raise ValueError(f"Subcommand '{name}' does not exist")

subparser = self._name_parser_map[name]

Expand Down Expand Up @@ -684,12 +684,12 @@ def update_prog(self, prog: str) -> None:
# 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'.
# subparsers_action._name_parser_map 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():
for subcmd_name, subcmd_parser in subparsers_action._name_parser_map.items():
if subcmd_parser in updated_parsers:
continue

Expand All @@ -707,9 +707,9 @@ def find_parser(self, subcommand_path: Iterable[str]) -> "Cmd2ArgumentParser":
parser = self
for name in subcommand_path:
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]
if name not in subparsers_action._name_parser_map:
raise ValueError(f"Subcommand '{name}' does not exist for '{parser.prog}'")
parser = subparsers_action._name_parser_map[name]
return parser

def attach_subcommand(
Expand All @@ -729,7 +729,8 @@ def attach_subcommand(
: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
:raises ValueError: if the command path is invalid, doesn't support subcommands, or the
subcommand already exists
"""
if not isinstance(subcommand_parser, Cmd2ArgumentParser):
raise TypeError(
Expand All @@ -751,6 +752,12 @@ def attach_subcommand(
f"Received: '{type(subcommand_parser).__name__}'."
)

# Do not overwrite existing subcommands or aliases
all_names = (subcommand, *add_parser_kwargs.get("aliases", ()))
for name in all_names:
if name in subparsers_action._name_parser_map:
raise ValueError(f"Subcommand '{name}' already exists for '{target_parser.prog}'")

# Use add_parser to register the subcommand name and any aliases
placeholder_parser = subparsers_action.add_parser(subcommand, **add_parser_kwargs)

Expand Down Expand Up @@ -783,7 +790,7 @@ def detach_subcommand(self, subcommand_path: Iterable[str], subcommand: str) ->
subparsers_action.remove_parser(subcommand), # type: ignore[attr-defined]
)
except ValueError:
raise ValueError(f"Subcommand '{subcommand}' not found in '{target_parser.prog}'") from None
raise ValueError(f"Subcommand '{subcommand}' does not exist for '{target_parser.prog}'") from None

def error(self, message: str) -> NoReturn:
"""Override that applies custom formatting to the error message."""
Expand Down
5 changes: 3 additions & 2 deletions cmd2/cmd2.py
Original file line number Diff line number Diff line change
Expand Up @@ -1174,7 +1174,7 @@ def get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentPar
# Search for the base command function and verify it has an argparser defined
command_func = self.get_command_func(root_command)
if command_func is None:
raise ValueError(f"Root command '{root_command}' not found")
raise ValueError(f"Root command '{root_command}' does not exist")

root_parser = self.command_parsers.get(command_func)
if root_parser is None:
Expand All @@ -1199,7 +1199,8 @@ def attach_subcommand(
: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
:raises ValueError: if the command path is invalid, doesn't support subcommands, or the
subcommand already exists
"""
root_parser, subcommand_path = self.get_root_parser_and_subcmd_path(command)
root_parser.attach_subcommand(subcommand_path, subcommand, subcommand_parser, **add_parser_kwargs)
Expand Down
12 changes: 9 additions & 3 deletions tests/test_argparse_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,20 +453,26 @@ def test_subcommand_attachment_errors() -> None:
root_parser.add_subparsers()

# Verify ValueError when path is invalid (find_parser() fails)
with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):
with pytest.raises(ValueError, match="Subcommand 'nonexistent' does not exist for 'root'"):
root_parser.attach_subcommand(["nonexistent"], "anything", child_parser)
with pytest.raises(ValueError, match="Subcommand 'nonexistent' not found"):
with pytest.raises(ValueError, match="Subcommand 'nonexistent' does not exist for 'root'"):
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'"):
with pytest.raises(ValueError, match="Subcommand 'fake' does not exist for '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 ValueError when subcommand name already exists
sub_parser = Cmd2ArgumentParser(prog="sub")
root_parser.attach_subcommand([], "sub", sub_parser)
with pytest.raises(ValueError, match="Subcommand 'sub' already exists for 'root'"):
root_parser.attach_subcommand([], "sub", sub_parser)
Comment thread
tleonhardt marked this conversation as resolved.


def test_subcommand_attachment_parser_class_override() -> None:
class MyParser(Cmd2ArgumentParser):
Expand Down
14 changes: 13 additions & 1 deletion tests/test_cmd2.py
Original file line number Diff line number Diff line change
Expand Up @@ -4596,6 +4596,13 @@ class SubcmdErrorApp(cmd2.Cmd):
def __init__(self) -> None:
super().__init__()

test_parser = cmd2.Cmd2ArgumentParser()
test_parser.add_subparsers(required=True)

@cmd2.with_argparser(test_parser)
def do_test(self, _statement: cmd2.Statement) -> None:
pass

def do_no_argparse(self, _statement: cmd2.Statement) -> None:
pass

Expand All @@ -4606,9 +4613,14 @@ def do_no_argparse(self, _statement: cmd2.Statement) -> None:
app.attach_subcommand("", "sub", cmd2.Cmd2ArgumentParser())

# Test non-existent command
with pytest.raises(ValueError, match="Root command 'fake' not found"):
with pytest.raises(ValueError, match="Root command 'fake' does not exist"):
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())

# Test duplicate subcommand
app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())
with pytest.raises(ValueError, match="Subcommand 'sub' already exists for 'test'"):
app.attach_subcommand("test", "sub", cmd2.Cmd2ArgumentParser())
Loading