Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions cmd2/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,9 @@ def as_subcommand_to(
:param subcommand: Subcommand name
:param parser: instance of Cmd2ArgumentParser or a callable that returns a Cmd2ArgumentParser for this subcommand
:param help: Help message for this subcommand which displays in the list of subcommands of the command we are adding to.
This is passed as the help argument to subparsers.add_parser().
:param aliases: Alternative names for this subcommand. This is passed as the alias argument to
subparsers.add_parser().
If not None, this is passed as the 'help' argument to subparsers.add_parser().
:param aliases: Alternative names for this subcommand. If a non-empty sequence is provided, it is passed
as the 'aliases' argument to subparsers.add_parser().
:param add_parser_kwargs: other registration-specific kwargs for add_parser()
(e.g. deprecated [Python 3.13+])
:return: a decorator which configures the target function to be a subcommand handler
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