diff --git a/CHANGELOG.md b/CHANGELOG.md index 368a487e0..5f7d3d380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ prompt is displayed. class of it. - Removed `set_ap_completer_type()` and `get_ap_completer_type()` since `ap_completer_type` is now a public member of `Cmd2ArgumentParser`. + - Moved `set_parser_prog()` to `Cmd2ArgumentParser.update_prog()`. - Enhancements - New `cmd2.Cmd` parameters - **auto_suggest**: (boolean) if `True`, provide fish shell style auto-suggestions. These diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py index 26e96e27d..763d88538 100644 --- a/cmd2/argparse_completer.py +++ b/cmd2/argparse_completer.py @@ -27,7 +27,7 @@ from .argparse_custom import ( Cmd2ArgumentParser, - generate_range_error, + build_range_error, ) from .command_definition import CommandSet from .completion import ( @@ -137,7 +137,7 @@ def __init__(self, flag_arg_state: _ArgumentState) -> None: :param flag_arg_state: information about the unfinished flag action. """ arg = f'{argparse._get_action_name(flag_arg_state.action)}' - err = f'{generate_range_error(flag_arg_state.min, flag_arg_state.max)}' + err = f'{build_range_error(flag_arg_state.min, flag_arg_state.max)}' error = f"Error: argument {arg}: {err} ({flag_arg_state.count} entered)" super().__init__(error) diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 5711ffb68..23b6ad2ba 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -222,19 +222,6 @@ def get_choices(self) -> Choices: - ``action.get_()`` - ``action.set_(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 @@ -243,6 +230,7 @@ def get_choices(self) -> Choices: from argparse import ArgumentError from collections.abc import ( Callable, + Iterable, Iterator, Sequence, ) @@ -283,8 +271,8 @@ def get_choices(self) -> Choices: from .argparse_completer import ArgparseCompleter -def generate_range_error(range_min: int, range_max: float) -> str: - """Generate an error message when the the number of arguments provided is not within the expected range.""" +def build_range_error(range_min: int, range_max: float) -> str: + """Build an error message when the the number of arguments provided is not within the expected range.""" err_msg = "expected " if range_max == constants.INFINITY: @@ -302,56 +290,6 @@ def generate_range_error(range_min: int, range_max: float) -> str: return err_msg -def set_parser_prog(parser: argparse.ArgumentParser, prog: str) -> None: - """Recursively set prog attribute of a parser and all of its subparsers. - - Does so that the root command is a command name and not sys.argv[0]. - - :param parser: the parser being edited - :param prog: new value for the parser's prog attribute - """ - # Set the prog value for this parser - parser.prog = prog - req_args: list[str] = [] - - # Set the prog value for the parser's subcommands - for action in parser._actions: - if isinstance(action, argparse._SubParsersAction): - # Set the _SubParsersAction's _prog_prefix value. That way if its add_parser() method is called later, - # the correct prog value will be set on the parser being added. - action._prog_prefix = parser.prog - - # The keys of action.choices are subcommand names as well as subcommand aliases. The aliases point to the - # same parser as the actual subcommand. We want to avoid placing an alias into a parser's prog value. - # Unfortunately there is nothing about an action.choices entry which tells us it's an alias. In most cases - # we can filter out the aliases by checking the contents of action._choices_actions. This list only contains - # help information and names for the subcommands and not aliases. However, subcommands without help text - # won't show up in that list. Since dictionaries are ordered in Python 3.6 and above and argparse inserts the - # subcommand name into choices dictionary before aliases, we should be OK assuming the first time we see a - # parser, the dictionary key is a subcommand and not alias. - processed_parsers = [] - - # Set the prog value for each subcommand's parser - for subcmd_name, subcmd_parser in action.choices.items(): - # Check if we've already edited this parser - if subcmd_parser in processed_parsers: - continue - - subcmd_prog = parser.prog - if req_args: - subcmd_prog += " " + " ".join(req_args) - subcmd_prog += " " + subcmd_name - set_parser_prog(subcmd_parser, subcmd_prog) - processed_parsers.append(subcmd_parser) - - # We can break since argparse only allows 1 group of subcommands per level - break - - # Need to save required args so they can be prepended to the subcommand usage - if action.required: - req_args.append(action.dest) - - ############################################################################################################ # Allow developers to add custom action attributes ############################################################################################################ @@ -571,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 ArgumentParser 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. @@ -720,7 +577,7 @@ def _set_color(self, color: bool, **kwargs: Any) -> None: super()._set_color(color) def _build_nargs_range_str(self, nargs_range: tuple[int, int | float]) -> str: - """Generate nargs range string for help text.""" + """Build nargs range string for help text.""" if nargs_range[1] == constants.INFINITY: # {min+} range_str = f"{{{nargs_range[0]}+}}" @@ -877,7 +734,7 @@ def __init__( *, ap_completer_type: type['ArgparseCompleter'] | None = None, ) -> None: - """Initialize the Cmd2ArgumentParser instance, a custom ArgumentParser added by cmd2. + """Initialize the Cmd2ArgumentParser instance. :param ap_completer_type: optional parameter which specifies a subclass of ArgparseCompleter for custom completion behavior on this parser. If this is None or not present, then cmd2 will use @@ -904,29 +761,211 @@ def __init__( conflict_handler=conflict_handler, add_help=add_help, allow_abbrev=allow_abbrev, - exit_on_error=exit_on_error, # added in Python 3.9 + exit_on_error=exit_on_error, **kwargs, # added in Python 3.14 ) - # Recast to assist type checkers since these can be Rich renderables in a Cmd2HelpFormatter. + self.ap_completer_type = ap_completer_type + + # To assist type checkers, recast these to reflect our usage of rich-argparse. + self.formatter_class: type[Cmd2HelpFormatter] self.description: RenderableType | None # type: ignore[assignment] self.epilog: RenderableType | None # type: ignore[assignment] - 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[Cmd2ArgumentParser]": + """Get the _SubParsersAction for this parser if it exists. + + :return: the _SubParsersAction for this parser + :raises ValueError: if this parser does not support subcommands + """ + if self._subparsers is not None: + for action in self._subparsers._group_actions: + if isinstance(action, argparse._SubParsersAction): + return action + raise ValueError(f"Command '{self.prog}' does not support subcommands") + + def _build_subparsers_prog_prefix(self, positionals: list[argparse.Action]) -> str: + """Build the 'prog' prefix for a subparsers action. + + This prefix is stored in the _SubParsersAction's '_prog_prefix' attribute and + is used to construct the 'prog' attribute for its child parsers. It + typically consists of the current parser's 'prog' name followed by any + positional arguments that appear before the _SubParsersAction. + + This method uses a temporary Cmd2ArgumentParser to leverage argparse's + functionality for generating these strings. Subclasses can override this if + they need to change how subcommand 'prog' values are constructed (e.g., if + add_subparsers() was overridden with custom naming logic or if a different + formatting style is desired). + + Note: This method explicitly instantiates Cmd2ArgumentParser rather than + type(self) to avoid potential side effects or mandatory constructor + arguments in user-defined subclasses. + + :param positionals: positional arguments which appear before the _SubParsersAction + :return: the built 'prog' prefix + """ + # 1. usage=None: In Python < 3.14, this prevents the default usage + # string from affecting subparser prog strings. This was fixed in 3.14: + # https://github.com/python/cpython/commit/0cb4d6c6549d2299f7518f083bbe7d10314ecd66 + # + # 2. add_help=False: No need for a help action since we already know which + # actions are needed to build the prefix and have passed them in + # via the 'positionals' argument. + temp_parser = Cmd2ArgumentParser( + prog=self.prog, + usage=None, + formatter_class=self.formatter_class, + add_help=False, + ) + + # Inject the current positional state so add_subparsers() has the right context + temp_parser._actions = positionals + temp_parser._mutually_exclusive_groups = self._mutually_exclusive_groups + + # Call add_subparsers() to build _prog_prefix + return temp_parser.add_subparsers()._prog_prefix + + def update_prog(self, prog: str) -> None: + """Recursively update the prog attribute of this parser and all of its subparsers. + + :param prog: new value for this parser's prog attribute + """ + # Set the prog value for this parser + self.prog = prog + + try: + subparsers_action = self._get_subparsers_action() + except ValueError: + # This parser has no subcommands + return + + # Get all positional arguments which appear before the subcommand. + positionals: list[argparse.Action] = [] + for action in self._actions: + if action is subparsers_action: + break + + # Save positional argument + if not action.option_strings: + positionals.append(action) + + # Update _prog_prefix. This ensures that any subcommands added later via + # 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'. + updated_parsers: set[Cmd2ArgumentParser] = set() + + # Set the prog value for each subcommand's parser + for subcmd_name, subcmd_parser in subparsers_action.choices.items(): + if subcmd_parser in updated_parsers: + continue + + subcmd_prog = f"{subparsers_action._prog_prefix} {subcmd_name}" + subcmd_parser.update_prog(subcmd_prog) + updated_parsers.add(subcmd_parser) + + 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 + :raises ValueError: if any subcommand in the path is not found or a level doesn't support subcommands + """ + 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] + return parser + + def attach_subcommand( + self, + subcommand_path: Iterable[str], + subcommand: str, + parser: 'Cmd2ArgumentParser', + **add_parser_kwargs: Any, + ) -> None: + """Attach a parser as a subcommand to a command at the specified path. + + :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 add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :raises ValueError: if the command path is invalid or doesn't support subcommands + """ + target_parser = self._find_parser(subcommand_path) + subparsers_action = target_parser._get_subparsers_action() + + # Use add_parser to register the subcommand name and any aliases + new_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) + + # 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. + + :param subcommand_path: sequence of subcommand names leading to the parser hosting the + subcommand to be detached. An empty sequence indicates this parser. + :param subcommand: name of the subcommand to detach + :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 + + return subparser + def error(self, message: str) -> NoReturn: """Override that applies custom formatting to the error message.""" lines = message.split('\n') @@ -983,7 +1022,7 @@ def _match_argument(self, action: argparse.Action, arg_strings_pattern: str) -> if match is None: nargs_range = action.get_nargs_range() # type: ignore[attr-defined] if nargs_range is not None: - raise ArgumentError(action, generate_range_error(nargs_range[0], nargs_range[1])) + raise ArgumentError(action, build_range_error(nargs_range[0], nargs_range[1])) return super()._match_argument(action, arg_strings_pattern) @@ -992,7 +1031,6 @@ def _check_value(self, action: argparse.Action, value: Any) -> None: When displaying choices, use CompletionItem.value instead of the CompletionItem instance. - :param self: ArgumentParser instance :param action: the action being populated :param value: value from command line already run through conversion function by argparse """ @@ -1035,7 +1073,7 @@ def set(self, new_val: Any) -> None: def set_default_argument_parser_type(parser_type: type[Cmd2ArgumentParser]) -> None: - """Set the default ArgumentParser class for cmd2's built-in commands. + """Set the default Cmd2ArgumentParser class for cmd2's built-in commands. Since built-in commands rely on customizations made in Cmd2ArgumentParser, your custom parser class should inherit from Cmd2ArgumentParser. diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index 9181f01e1..d661509d7 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -49,7 +49,6 @@ Callable, Iterable, Mapping, - MutableSequence, Sequence, ) from dataclasses import ( @@ -274,7 +273,11 @@ 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) + + # To ensure accurate usage strings, recursively update 'prog' values + # within the parser to match the command name. + 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__: @@ -889,7 +892,6 @@ def _build_parser( self, parent: CmdOrSet, parser_builder: Cmd2ArgumentParser | Callable[[], Cmd2ArgumentParser] | StaticArgParseBuilder | ClassArgParseBuilder, - prog: str, ) -> Cmd2ArgumentParser: """Build argument parser for a command/subcommand. @@ -898,7 +900,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 @@ -921,8 +922,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") - argparse_custom.set_parser_prog(parser, prog) - return parser def _install_command_function(self, command_func_name: str, command_method: CommandFunc, context: str = '') -> None: @@ -1026,18 +1025,29 @@ def unregister_command_set(self, cmdset: CommandSet) -> None: self._installed_command_sets.remove(cmdset) def _check_uninstallable(self, cmdset: CommandSet) -> None: + cmdset_id = id(cmdset) + def check_parser_uninstallable(parser: Cmd2ArgumentParser) -> None: - cmdset_id = id(cmdset) - for action in parser._actions: - if isinstance(action, argparse._SubParsersAction): - for subparser in action.choices.values(): - attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) - if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: - raise CommandSetRegistrationError( - 'Cannot uninstall CommandSet when another CommandSet depends on it' - ) - check_parser_uninstallable(subparser) - break + try: + subparsers_action = parser._get_subparsers_action() + except ValueError: + # No subcommands to check + return + + # Prevent redundant traversal of parser aliases + checked_parsers: set[Cmd2ArgumentParser] = set() + + for subparser in subparsers_action.choices.values(): + if subparser in checked_parsers: + continue + checked_parsers.add(subparser) + + attached_cmdset_id = getattr(subparser, constants.PARSER_ATTR_COMMANDSET_ID, None) + if attached_cmdset_id is not None and attached_cmdset_id != cmdset_id: + raise CommandSetRegistrationError( + f"Cannot uninstall CommandSet: '{subparser.prog}' is required by another CommandSet" + ) + check_parser_uninstallable(subparser) methods: list[tuple[str, Callable[..., Any]]] = inspect.getmembers( cmdset, @@ -1085,40 +1095,8 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: if not subcommand_valid: raise CommandSetRegistrationError(f'Subcommand {subcommand_name} is not valid: {errmsg}') - command_tokens = full_command_name.split() - command_name = command_tokens[0] - subcommand_names = command_tokens[1:] - - # Search for the base command function and verify it has an argparser defined - if command_name in self.disabled_commands: - command_func = self.disabled_commands[command_name].command_function - else: - command_func = self.cmd_func(command_name) - - if command_func is None: - raise CommandSetRegistrationError(f"Could not find command '{command_name}' needed by subcommand: {method}") - command_parser = self._command_parsers.get(command_func) - if command_parser is None: - raise CommandSetRegistrationError( - f"Could not find argparser for command '{command_name}' needed by subcommand: {method}" - ) - - def find_subcommand(action: Cmd2ArgumentParser, subcmd_names: MutableSequence[str]) -> Cmd2ArgumentParser: - if not subcmd_names: - return action - cur_subcmd = subcmd_names.pop(0) - for sub_action in action._actions: - if isinstance(sub_action, argparse._SubParsersAction): - for choice_name, choice in sub_action.choices.items(): - if choice_name == cur_subcmd: - return find_subcommand(choice, subcmd_names) - break - raise CommandSetRegistrationError(f"Could not find subcommand '{action}'") - - target_parser = find_subcommand(command_parser, subcommand_names) - # Create the subcommand parser and configure it - subcmd_parser = self._build_parser(cmdset, subcmd_parser_builder, f'{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__) @@ -1129,19 +1107,14 @@ def find_subcommand(action: Cmd2ArgumentParser, subcmd_names: MutableSequence[st # Set what instance the handler is bound to setattr(subcmd_parser, constants.PARSER_ATTR_COMMANDSET_ID, id(cmdset)) - # Find the argparse action that handles subcommands - for action in target_parser._actions: - if isinstance(action, argparse._SubParsersAction): - # Get add_parser() kwargs (aliases, help, etc.) defined by the decorator - add_parser_kwargs = getattr(method, constants.SUBCMD_ATTR_ADD_PARSER_KWARGS, {}) - - # Attach existing parser as a subcommand - action.attach_parser( # type: ignore[attr-defined] - subcommand_name, - subcmd_parser, - **add_parser_kwargs, - ) - break + # Get add_parser() kwargs (aliases, help, etc.) defined by the decorator + add_parser_kwargs = getattr(method, constants.SUBCMD_ATTR_ADD_PARSER_KWARGS, {}) + + # Attach existing parser as a subcommand + try: + self.attach_subcommand(full_command_name, subcommand_name, subcmd_parser, **add_parser_kwargs) + except ValueError as ex: + raise CommandSetRegistrationError(str(ex)) from ex def _unregister_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: """Unregister subcommands from their base command. @@ -1165,30 +1138,77 @@ def _unregister_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: # iterate through all matching methods for _method_name, method in methods: subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME) - command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND) + full_command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND) - # Search for the base command function and verify it has an argparser defined - if command_name in self.disabled_commands: - command_func = self.disabled_commands[command_name].command_function - else: - command_func = self.cmd_func(command_name) - - if command_func is None: # pragma: no cover - # This really shouldn't be possible since _register_subcommands would prevent this from happening - # but keeping in case it does for some strange reason - raise CommandSetRegistrationError(f"Could not find command '{command_name}' needed by subcommand: {method}") - command_parser = self._command_parsers.get(command_func) - if command_parser is None: # pragma: no cover - # This really shouldn't be possible since _register_subcommands would prevent this from happening - # but keeping in case it does for some strange reason - raise CommandSetRegistrationError( - f"Could not find argparser for command '{command_name}' needed by subcommand: {method}" - ) + with contextlib.suppress(ValueError): + self.detach_subcommand(full_command_name, subcommand_name) - for action in command_parser._actions: - if isinstance(action, argparse._SubParsersAction): - action.detach_parser(subcommand_name) # type: ignore[attr-defined] - break + def _get_root_parser_and_subcmd_path(self, command: str) -> tuple[Cmd2ArgumentParser, list[str]]: + """Tokenize a command string and resolve the associated root parser and relative subcommand path. + + This helper handles the initial resolution of a command string (e.g., 'foo bar baz') by + identifying 'foo' as the root command (even if disabled), retrieving its associated + parser, and returning any remaining tokens (['bar', 'baz']) as a path relative + to that parser for further traversal. + + :param command: full space-delimited command path leading to a parser (e.g. 'foo' or 'foo bar') + :return: a tuple containing the Cmd2ArgumentParser for the root command and a list of + strings representing the relative path to the desired hosting parser. + :raises ValueError: if the command is empty, the root command is not found, or + the root command does not use an argparse parser. + """ + tokens = command.split() + if not tokens: + raise ValueError("Command path cannot be empty") + + root_command = tokens[0] + subcommand_path = tokens[1:] + + # Search for the base command function and verify it has an argparser defined + if root_command in self.disabled_commands: + command_func = self.disabled_commands[root_command].command_function + else: + command_func = self.cmd_func(root_command) + + if command_func is None: + raise ValueError(f"Root command '{root_command}' not found") + + root_parser = self._command_parsers.get(command_func) + if root_parser is None: + raise ValueError(f"Command '{root_command}' does not use argparse") + + return root_parser, subcommand_path + + def attach_subcommand( + self, + command: str, + subcommand: str, + parser: Cmd2ArgumentParser, + **add_parser_kwargs: Any, + ) -> None: + """Attach a parser as a subcommand to a command at the specified path. + + :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 add_parser_kwargs: additional arguments for the subparser registration (e.g. help, aliases) + :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) + + def detach_subcommand(self, command: str, subcommand: str) -> Cmd2ArgumentParser: + """Detach a subcommand from a command at the specified path. + + :param command: full command path (space-delimited) leading to the parser hosting the + subcommand to be detached (e.g. 'foo bar') + :param subcommand: name of the subcommand to detach + :return: the detached parser + :raises ValueError: if the command path is invalid or the subcommand doesn't exist + """ + root_parser, subcommand_path = self._get_root_parser_and_subcmd_path(command) + return root_parser.detach_subcommand(subcommand_path, subcommand) @property def always_prefix_settables(self) -> bool: diff --git a/examples/scripts/save_help_text.py b/examples/scripts/save_help_text.py index 41636b085..0a4ecb393 100644 --- a/examples/scripts/save_help_text.py +++ b/examples/scripts/save_help_text.py @@ -2,29 +2,36 @@ This is meant to be run within a cmd2 session using run_pyscript. """ -import argparse import os import sys from typing import TextIO +from cmd2 import Cmd2ArgumentParser + ASTERISKS = "********************************************************" -def get_sub_commands(parser: argparse.ArgumentParser) -> list[str]: - """Get a list of subcommands for an ArgumentParser.""" +def get_sub_commands(parser: Cmd2ArgumentParser) -> list[str]: + """Get a list of subcommands for a Cmd2ArgumentParser.""" + try: + subparsers_action = parser._get_subparsers_action() + except ValueError: + # No subcommands + return [] + + # Prevent redundant traversal of parser aliases + checked_parsers: set[Cmd2ArgumentParser] = set() + sub_cmds = [] + for subcmd, subcmd_parser in subparsers_action.choices.items(): + if subcmd_parser in checked_parsers: + continue + checked_parsers.add(subcmd_parser) - # Check if this is parser has subcommands - if parser is not None and parser._subparsers is not None: - # Find the _SubParsersAction for the subcommands of this parser - for action in parser._subparsers._actions: - if isinstance(action, argparse._SubParsersAction): - for sub_cmd, sub_cmd_parser in action.choices.items(): - sub_cmds.append(sub_cmd) + sub_cmds.append(subcmd) - # Look for nested subcommands - sub_cmds.extend(f'{sub_cmd} {nested_sub_cmd}' for nested_sub_cmd in get_sub_commands(sub_cmd_parser)) - break + # Look for nested subcommands + sub_cmds.extend(f'{subcmd} {nested_subcmd}' for nested_subcmd in get_sub_commands(subcmd_parser)) sub_cmds.sort() return sub_cmds @@ -60,8 +67,7 @@ def main() -> None: # Open the output file outfile_path = os.path.expanduser(sys.argv[1]) try: - with open(outfile_path, 'w') as outfile: - pass + outfile = open(outfile_path, 'w') # noqa: SIM115 except OSError as e: print(f"Error opening {outfile_path} because: {e}") return @@ -83,11 +89,18 @@ def main() -> None: is_command = item in all_commands add_help_to_file(item, outfile, is_command) - if is_command: - # Add any subcommands - for subcmd in get_sub_commands(getattr(self.cmd_func(item), 'argparser', None)): - full_cmd = f'{item} {subcmd}' - add_help_to_file(full_cmd, outfile, is_command) + if not is_command: + continue + + cmd_func = self.cmd_func(item) + parser = self._command_parsers.get(cmd_func) + if parser is None: + continue + + # Add any subcommands + for subcmd in get_sub_commands(parser): + full_cmd = f'{item} {subcmd}' + add_help_to_file(full_cmd, outfile, is_command) outfile.close() print(f"Output written to {outfile_path}") diff --git a/tests/test_argparse.py b/tests/test_argparse.py index d1fed524d..13d567bf5 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -248,7 +248,7 @@ 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): @@ -256,7 +256,7 @@ 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: @@ -335,9 +335,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to set_parser_prog() which - # use an approach which relies on action._choices_actions list. See comment in that function for more - # details. + # This subcommand has aliases and no help text. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) parser_helpless.add_argument('z', help='string') parser_helpless.set_defaults(func=base_helpless) @@ -445,19 +443,6 @@ def test_subcommand_invalid_help(subcommand_app) -> None: assert out[0].startswith('Usage: base') -def test_add_another_subcommand(subcommand_app) -> None: - """This tests makes sure set_parser_prog() sets _prog_prefix on every _SubParsersAction so that all future calls - to add_parser() write the correct prog value to the parser being added. - """ - base_parser = subcommand_app._command_parsers.get(subcommand_app.do_base) - for sub_action in base_parser._actions: - if isinstance(sub_action, argparse._SubParsersAction): - new_parser = sub_action.add_parser('new_sub', help='stuff') - break - - assert new_parser.prog == "base new_sub" - - def test_subcmd_decorator(subcommand_app) -> None: # Test subcommand that has help option out, err = run_cmd(subcommand_app, 'test_subcmd_decorator subcmd') diff --git a/tests/test_argparse_custom.py b/tests/test_argparse_custom.py index 95f5527c7..d4f29688e 100644 --- a/tests/test_argparse_custom.py +++ b/tests/test_argparse_custom.py @@ -14,10 +14,10 @@ ) from cmd2.argparse_custom import ( Cmd2HelpFormatter, - Cmd2RichArgparseConsole, - generate_range_error, + build_range_error, register_argparse_argument_parameter, ) +from cmd2.rich_utils import Cmd2RichArgparseConsole from .conftest import run_cmd @@ -258,26 +258,26 @@ def test_apcustom_print_message(capsys) -> None: assert test_message in err -def test_generate_range_error() -> None: +def test_build_range_error() -> None: # max is INFINITY - err_msg = generate_range_error(1, constants.INFINITY) + err_msg = build_range_error(1, constants.INFINITY) assert err_msg == "expected at least 1 argument" - err_msg = generate_range_error(2, constants.INFINITY) + err_msg = build_range_error(2, constants.INFINITY) assert err_msg == "expected at least 2 arguments" # min and max are equal - err_msg = generate_range_error(1, 1) + err_msg = build_range_error(1, 1) assert err_msg == "expected 1 argument" - err_msg = generate_range_error(2, 2) + err_msg = build_range_error(2, 2) assert err_msg == "expected 2 arguments" # min and max are not equal - err_msg = generate_range_error(0, 1) + err_msg = build_range_error(0, 1) assert err_msg == "expected 0 to 1 argument" - err_msg = generate_range_error(0, 2) + err_msg = build_range_error(0, 2) assert err_msg == "expected 0 to 2 arguments" @@ -342,46 +342,98 @@ def test_register_argparse_argument_parameter() -> None: delattr(argparse.Action, attr_name) -def test_parser_attachment() -> None: - # Attach a parser as a subcommand - root_parser = Cmd2ArgumentParser(description="root command") +def test_subcommand_attachment() -> None: + """Test Cmd2ArgumentParser convenience methods for attaching and detaching subcommands.""" + + ############################### + # Set up parsers + ############################### + root_parser = Cmd2ArgumentParser(prog="root", description="root command") root_subparsers = root_parser.add_subparsers() - child_parser = Cmd2ArgumentParser(description="child command") - root_subparsers.attach_parser( # type: ignore[attr-defined] + child_parser = Cmd2ArgumentParser(prog="child", description="child command") + child_subparsers = child_parser.add_subparsers() # Must have subparsers to host grandchild + + grandchild_parser = Cmd2ArgumentParser(prog="grandchild", description="grandchild command") + + ############################### + # Attach subcommands + ############################### + + # Attach child to root + root_parser.attach_subcommand( + [], "child", child_parser, help="a child command", aliases=["child_alias"], ) - # Verify the same parser instance was used + # Attach grandchild to child + root_parser.attach_subcommand( + ["child"], + "grandchild", + grandchild_parser, + help="a grandchild command", + ) + + ############################### + # 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 + + ############################### + # Verify attachments + ############################### + + # Verify child attachment and aliases 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" + # Verify grandchild attachment + assert child_subparsers._name_parser_map["grandchild"] is grandchild_parser + + ############################### + # Detach subcommands + ############################### - # Detatch the subcommand - detached_parser = root_subparsers.detach_parser("child") # type: ignore[attr-defined] + # Detach grandchild from child + detached_grandchild = root_parser.detach_subcommand(["child"], "grandchild") + assert detached_grandchild is grandchild_parser + assert "grandchild" not in child_subparsers._name_parser_map - # Verify subcommand and its aliases were removed - assert detached_parser is child_parser + # 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 - # 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_errors() -> None: + root_parser = Cmd2ArgumentParser(prog="root", description="root command") + child_parser = Cmd2ArgumentParser(prog="child", description="child command") + + # Verify ValueError when subcommands are not supported + with pytest.raises(ValueError, match="Command 'root' does not support subcommands"): + root_parser.attach_subcommand([], "anything", child_parser) + with pytest.raises(ValueError, match="Command 'root' does not support subcommands"): + root_parser.detach_subcommand([], "anything") + + # Allow subcommands for the next tests + root_parser.add_subparsers() + + # 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"): + 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'"): + root_parser.detach_subcommand([], "fake") def test_completion_items_as_choices(capsys) -> None: @@ -476,3 +528,67 @@ def side_effect(color, **kwargs): assert mock_set_color.call_count == 2 mock_set_color.assert_any_call(True, file=sys.stdout) mock_set_color.assert_any_call(True) + + +def test_update_prog() -> None: + """Test Cmd2ArgumentParser.update_prog() across various scenarios.""" + + # Set up a complex parser hierarchy + old_app = 'old_app' + root = Cmd2ArgumentParser(prog=old_app) + + # Positionals before subcommand + root.add_argument('pos1') + + # Mutually exclusive group with positionals + group = root.add_mutually_exclusive_group(required=True) + group.add_argument('posA', nargs='?') + group.add_argument('posB', nargs='?') + + # Subparsers with aliases and no help text + root_subparsers = root.add_subparsers(dest='cmd') + + # Subcommand with aliases + sub1 = root_subparsers.add_parser('sub1', aliases=['s1', 'alias1'], help='help for sub1') + + # Subcommand with no help text + sub2 = root_subparsers.add_parser('sub2') + + # Nested subparser + sub2.add_argument('inner_pos') + sub2_subparsers = sub2.add_subparsers(dest='sub2_cmd') + leaf = sub2_subparsers.add_parser('leaf', help='leaf help') + + # Save initial prog values + orig_root_prog = root.prog + orig_sub1_prog = sub1.prog + orig_sub2_prog = sub2.prog + orig_leaf_prog = leaf.prog + + # Perform update + new_app = 'new_app' + root.update_prog(new_app) + + # Verify updated prog values + assert root.prog.startswith(new_app) + assert root.prog == orig_root_prog.replace(old_app, new_app, 1) + + assert sub1.prog.startswith(new_app) + assert sub1.prog == orig_sub1_prog.replace(old_app, new_app, 1) + + assert sub2.prog.startswith(new_app) + assert sub2.prog == orig_sub2_prog.replace(old_app, new_app, 1) + + assert leaf.prog.startswith(new_app) + assert leaf.prog == orig_leaf_prog.replace(old_app, new_app, 1) + + # Verify that action._prog_prefix was updated by adding a new subparser + sub3 = root_subparsers.add_parser('sub3') + assert sub3.prog.startswith(new_app) + assert sub3.prog == root_subparsers._prog_prefix + ' sub3' + + # Verify aliases still point to the correct parser + for action in root._actions: + if isinstance(action, argparse._SubParsersAction): + assert action.choices['s1'].prog == sub1.prog + assert action.choices['alias1'].prog == sub1.prog diff --git a/tests/test_argparse_subcommands.py b/tests/test_argparse_subcommands.py index 558924d1e..653f3fdcd 100644 --- a/tests/test_argparse_subcommands.py +++ b/tests/test_argparse_subcommands.py @@ -45,9 +45,7 @@ def base_helpless(self, args) -> None: parser_bar.set_defaults(func=base_bar) # create the parser for the "helpless" subcommand - # This subcommand has aliases and no help text. It exists to prevent changes to set_parser_prog() which - # use an approach which relies on action._choices_actions list. See comment in that function for more - # details. + # This subcommand has aliases and no help text. parser_helpless = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) parser_helpless.add_argument('z', help='string') parser_helpless.set_defaults(func=base_helpless) diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index d0998f30a..e7293c30b 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -4419,3 +4419,75 @@ def test_auto_suggest_default(): """Test that auto_suggest defaults to True.""" app = cmd2.Cmd() assert isinstance(app.main_session.auto_suggest, AutoSuggestFromHistory) + + +def test_subcommand_attachment() -> None: + import argparse + + class SubcmdApp(cmd2.Cmd): + def __init__(self) -> None: + super().__init__() + + root_parser = cmd2.Cmd2ArgumentParser() + root_parser.add_subparsers() + + @cmd2.with_argparser(root_parser) + def do_root(self, _args: argparse.Namespace) -> None: + pass + + app = SubcmdApp() + + # Verify root exists and uses argparse + root_parser = app._command_parsers.get(app.do_root) + assert root_parser is not None + + # Attach child to root + child_parser = cmd2.Cmd2ArgumentParser(prog="child") + child_parser.add_subparsers() + app.attach_subcommand("root", "child", child_parser, help="child help") + + # Verify child was attached + 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 + + # Attach grandchild to child + grandchild_parser = cmd2.Cmd2ArgumentParser(prog="grandchild") + app.attach_subcommand("root child", "grandchild", grandchild_parser) + + # Verify grandchild was attached + child_subparsers_action = child_parser._get_subparsers_action() + assert "grandchild" in child_subparsers_action._name_parser_map + + # Detach grandchild + detached_grandchild = app.detach_subcommand("root child", "grandchild") + assert detached_grandchild is grandchild_parser + assert "grandchild" not in child_subparsers_action._name_parser_map + + # Detach child + detached_child = app.detach_subcommand("root", "child") + assert detached_child is child_parser + assert "child" not in root_subparsers_action._name_parser_map + + +def test_subcommand_attachment_errors() -> None: + class SubcmdErrorApp(cmd2.Cmd): + def __init__(self) -> None: + super().__init__() + + def do_no_argparse(self, _statement: cmd2.Statement) -> None: + pass + + app = SubcmdErrorApp() + + # Test empty command + with pytest.raises(ValueError, match="Command path cannot be empty"): + app.attach_subcommand("", "sub", cmd2.Cmd2ArgumentParser()) + + # Test non-existent command + with pytest.raises(ValueError, match="Root command 'fake' not found"): + 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()) diff --git a/tests/test_commandset.py b/tests/test_commandset.py index 330928f23..69129106d 100644 --- a/tests/test_commandset.py +++ b/tests/test_commandset.py @@ -98,7 +98,8 @@ def do_main(self, args: argparse.Namespace) -> None: # main -> sub subcmd_parser = cmd2.Cmd2ArgumentParser(description="Sub Command") - @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command") + # Include aliases to cover the alias check in cmd2's check_parser_uninstallable(). + @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command", aliases=["sub_alias"]) def subcmd_func(self, args: argparse.Namespace) -> None: self._cmd.poutput("Subcommand Ran") diff --git a/tests/test_completion.py b/tests/test_completion.py index 02018ba3c..992ba4fdd 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -942,7 +942,7 @@ def test_clean_display() -> None: assert completion_item.display == expected assert completion_item.display_meta == expected - # Verify that text derived display is also sanitized + # Verify that text-derived display is also sanitized text = "item\nwith\nnewlines" expected_text_display = "item with newlines" completion_item = CompletionItem(text)