From 7a4d7f4797966146bc32500d0cdbe0cbde58de08 Mon Sep 17 00:00:00 2001 From: Eric Lin Date: Mon, 25 Sep 2023 17:44:21 -0400 Subject: [PATCH] Changed with_argparser() and as_subcommand_to() to accept either an ArgumentParser or a factory callable that returns an ArgumentParser. Changed Cmd constructor to construct an instance-specific ArgumentParser using either the factory callable or by deep-copying the provided ArgumentParser. With this change a new argparse instance should be created for each instance of Cmd. Addresses #1002 --- cmd2/cmd2.py | 113 ++++++++++++++++++++--------- cmd2/decorators.py | 72 +++++++++++++----- cmd2/utils.py | 4 + tests/conftest.py | 14 ++++ tests/test_argparse.py | 12 ++- tests/transcripts/from_cmdloop.txt | 2 +- 6 files changed, 159 insertions(+), 58 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index f1e6c847..363c58f1 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -30,6 +30,7 @@ # setting is True import argparse import cmd +import copy import functools import glob import inspect @@ -140,6 +141,7 @@ from .utils import ( Settable, get_defining_class, + is_callable, strip_doc_annotations, suggest_similar, ) @@ -510,6 +512,16 @@ def __init__( # This does not affect self.formatted_completions. self.matches_sorted = False + # Command parsers for this Cmd instance. + self._command_parsers: Dict[str, argparse.ArgumentParser] = {} + + # Locates the command parser template or factory and creates an instance-specific parser + for command in self.get_all_commands(): + self._register_command_parser(command, self.cmd_func(command)) # type: ignore[arg-type] + + # Add functions decorated to be subcommands + self._register_subcommands(self) + ############################################################################################################ # The following code block loads CommandSets, verifies command names, and registers subcommands. # This block should appear after all attributes have been created since the registration code @@ -529,9 +541,6 @@ def __init__( if not valid: raise ValueError(f"Invalid command name '{cur_cmd}': {errmsg}") - # Add functions decorated to be subcommands - self._register_subcommands(self) - self.suggest_similar_command = suggest_similar_command self.default_suggestion_message = "Did you mean {}?" @@ -659,6 +668,38 @@ def register_command_set(self, cmdset: CommandSet) -> None: cmdset.on_unregistered() raise + def _register_command_parser(self, command: str, command_method: Callable[..., Any]) -> None: + if command not in self._command_parsers: + parser_builder = getattr(command_method, constants.CMD_ATTR_ARGPARSER, None) + if is_callable(parser_builder): + if isinstance(parser_builder, staticmethod): + parser = cast(argparse.ArgumentParser, parser_builder.__func__()) + elif isinstance(parser_builder, classmethod): + parent = self.find_commandset_for_command(command) + parser = cast(argparse.ArgumentParser, parser_builder.__func__(parent if not None else self)) + else: + parser = cast(argparse.ArgumentParser, parser_builder()) # type: ignore[misc] + elif isinstance(parser_builder, argparse.ArgumentParser): + if sys.version_info >= (3, 6, 4): + parser = copy.deepcopy(parser_builder) + else: + parser = parser_builder + else: + return + + # argparser defaults the program name to sys.argv[0], but we want it to be the name of our command + from .decorators import ( + _set_parser_prog, + ) + + _set_parser_prog(parser, command) + + # If the description has not been set, then use the method docstring if one exists + if parser.description is None and hasattr(command_method, '__wrapped__') and command_method.__wrapped__.__doc__: + parser.description = strip_doc_annotations(command_method.__wrapped__.__doc__) + + self._command_parsers[command] = parser + def _install_command_function(self, command: str, command_wrapper: Callable[..., Any], context: str = '') -> None: cmd_func_name = COMMAND_FUNC_PREFIX + command @@ -681,6 +722,8 @@ def _install_command_function(self, command: str, command_wrapper: Callable[..., self.pwarning(f"Deleting macro '{command}' because it shares its name with a new command") del self.macros[command] + self._register_command_parser(command, command_wrapper) + setattr(self, cmd_func_name, command_wrapper) def _install_completer_function(self, cmd_name: str, cmd_completer: CompleterFunc) -> None: @@ -727,6 +770,8 @@ def unregister_command_set(self, cmdset: CommandSet) -> None: del self._cmd_to_command_sets[cmd_name] delattr(self, COMMAND_FUNC_PREFIX + cmd_name) + if cmd_name in self._command_parsers: + del self._command_parsers[cmd_name] if hasattr(self, COMPLETER_FUNC_PREFIX + cmd_name): delattr(self, COMPLETER_FUNC_PREFIX + cmd_name) @@ -746,14 +791,7 @@ def _check_uninstallable(self, cmdset: CommandSet) -> None: for method in methods: command_name = method[0][len(COMMAND_FUNC_PREFIX) :] - - # 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) - - command_parser = cast(argparse.ArgumentParser, getattr(command_func, constants.CMD_ATTR_ARGPARSER, None)) + command_parser = self._command_parsers.get(command_name, None) def check_parser_uninstallable(parser: argparse.ArgumentParser) -> None: for action in parser._actions: @@ -792,7 +830,7 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: for method_name, method in methods: subcommand_name: str = getattr(method, constants.SUBCMD_ATTR_NAME) full_command_name: str = getattr(method, constants.SUBCMD_ATTR_COMMAND) - subcmd_parser = getattr(method, constants.CMD_ATTR_ARGPARSER) + subcmd_parser_builder = getattr(method, constants.CMD_ATTR_ARGPARSER) subcommand_valid, errmsg = self.statement_parser.is_valid_command(subcommand_name, is_subcommand=True) if not subcommand_valid: @@ -812,7 +850,7 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: raise CommandSetRegistrationError( f"Could not find command '{command_name}' needed by subcommand: {str(method)}" ) - command_parser = getattr(command_func, constants.CMD_ATTR_ARGPARSER, None) + command_parser = self._command_parsers.get(command_name, None) if command_parser is None: raise CommandSetRegistrationError( f"Could not find argparser for command '{command_name}' needed by subcommand: {str(method)}" @@ -832,16 +870,20 @@ def find_subcommand(action: argparse.ArgumentParser, subcmd_names: List[str]) -> target_parser = find_subcommand(command_parser, subcommand_names) + if is_callable(subcmd_parser_builder): + subcmd_parser = subcmd_parser_builder() + else: + subcmd_parser = copy.deepcopy(subcmd_parser_builder) + from .decorators import ( + _set_parser_prog, + ) + + _set_parser_prog(subcmd_parser, f'{command_name} {subcommand_name}') + if subcmd_parser.description is None and method.__doc__: + subcmd_parser.description = strip_doc_annotations(method.__doc__) + for action in target_parser._actions: if isinstance(action, argparse._SubParsersAction): - # Temporary workaround for avoiding subcommand help text repeatedly getting added to - # action._choices_actions. Until we have instance-specific parser objects, we will remove - # any existing subcommand which has the same name before replacing it. This problem is - # exercised when more than one cmd2.Cmd-based object is created and the same subcommands - # get added each time. Argparse overwrites the previous subcommand but keeps growing the help - # text which is shown by running something like 'alias -h'. - action.remove_parser(subcommand_name) # type: ignore[arg-type,attr-defined] - # Get the kwargs for add_parser() add_parser_kwargs = getattr(method, constants.SUBCMD_ATTR_ADD_PARSER_KWARGS, {}) @@ -913,7 +955,7 @@ def _unregister_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: raise CommandSetRegistrationError( f"Could not find command '{command_name}' needed by subcommand: {str(method)}" ) - command_parser = getattr(command_func, constants.CMD_ATTR_ARGPARSER, None) + command_parser = self._command_parsers.get(command_name, None) 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 @@ -2034,7 +2076,7 @@ def _perform_completion( else: # There's no completer function, next see if the command uses argparse func = self.cmd_func(command) - argparser: Optional[argparse.ArgumentParser] = getattr(func, constants.CMD_ATTR_ARGPARSER, None) + argparser = self._command_parsers.get(command, None) if func is not None and argparser is not None: # Get arguments for complete() @@ -3259,14 +3301,19 @@ def _cmdloop(self) -> None: ############################################################# # Top-level parser for alias - alias_description = "Manage aliases\n" "\n" "An alias is a command that enables replacement of a word by another string." - alias_epilog = "See also:\n" " macro" - alias_parser = argparse_custom.DEFAULT_ARGUMENT_PARSER(description=alias_description, epilog=alias_epilog) - alias_subparsers = alias_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') - alias_subparsers.required = True + @staticmethod + def _build_alias_parser() -> argparse.ArgumentParser: + alias_description = ( + "Manage aliases\n" "\n" "An alias is a command that enables replacement of a word by another string." + ) + alias_epilog = "See also:\n" " macro" + alias_parser = argparse_custom.DEFAULT_ARGUMENT_PARSER(description=alias_description, epilog=alias_epilog) + alias_subparsers = alias_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') + alias_subparsers.required = True + return alias_parser # Preserve quotes since we are passing strings to other commands - @with_argparser(alias_parser, preserve_quotes=True) + @with_argparser(_build_alias_parser, preserve_quotes=True) def do_alias(self, args: argparse.Namespace) -> None: """Manage aliases""" # Call handler for whatever subcommand was selected @@ -3681,7 +3728,7 @@ def complete_help_subcommands( # Check if this command uses argparse func = self.cmd_func(command) - argparser = getattr(func, constants.CMD_ATTR_ARGPARSER, None) + argparser = self._command_parsers.get(command, None) if func is None or argparser is None: return [] @@ -3717,7 +3764,7 @@ def do_help(self, args: argparse.Namespace) -> None: # Getting help for a specific command func = self.cmd_func(args.command) help_func = getattr(self, constants.HELP_FUNC_PREFIX + args.command, None) - argparser = getattr(func, constants.CMD_ATTR_ARGPARSER, None) + argparser = self._command_parsers.get(args.command, None) # If the command function uses argparse, then use argparse's help if func is not None and argparser is not None: @@ -3853,7 +3900,7 @@ def _build_command_info(self) -> Tuple[Dict[str, List[str]], List[str], List[str help_topics.remove(command) # Non-argparse commands can have help_functions for their documentation - if not hasattr(func, constants.CMD_ATTR_ARGPARSER): + if command not in self._command_parsers: has_help_func = True if hasattr(func, constants.CMD_ATTR_HELP_CATEGORY): @@ -3899,7 +3946,7 @@ def _print_topics(self, header: str, cmds: List[str], verbose: bool) -> None: doc: Optional[str] # Non-argparse commands can have help_functions for their documentation - if not hasattr(cmd_func, constants.CMD_ATTR_ARGPARSER) and command in topics: + if command not in self._command_parsers and command in topics: help_func = getattr(self, constants.HELP_FUNC_PREFIX + command) result = io.StringIO() diff --git a/cmd2/decorators.py b/cmd2/decorators.py index da09f176..a6203dc7 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -12,6 +12,7 @@ Tuple, TypeVar, Union, + overload, ) from . import ( @@ -30,9 +31,6 @@ from .parsing import ( Statement, ) -from .utils import ( - strip_doc_annotations, -) if TYPE_CHECKING: # pragma: no cover import cmd2 @@ -261,12 +259,34 @@ def _set_parser_prog(parser: argparse.ArgumentParser, prog: str) -> None: ] +@overload def with_argparser( parser: argparse.ArgumentParser, *, ns_provider: Optional[Callable[..., argparse.Namespace]] = None, preserve_quotes: bool = False, with_unknown_args: bool = False, +) -> Callable[[ArgparseCommandFunc[CommandParent]], RawCommandFuncOptionalBoolReturn[CommandParent]]: + ... + + +@overload +def with_argparser( + parser: Callable[[], argparse.ArgumentParser], + *, + ns_provider: Optional[Callable[..., argparse.Namespace]] = None, + preserve_quotes: bool = False, + with_unknown_args: bool = False, +) -> Callable[[ArgparseCommandFunc[CommandParent]], RawCommandFuncOptionalBoolReturn[CommandParent]]: + ... + + +def with_argparser( + parser: Union[argparse.ArgumentParser, Callable[[], argparse.ArgumentParser]], + *, + ns_provider: Optional[Callable[..., argparse.Namespace]] = None, + preserve_quotes: bool = False, + with_unknown_args: bool = False, ) -> Callable[[ArgparseCommandFunc[CommandParent]], RawCommandFuncOptionalBoolReturn[CommandParent]]: """A decorator to alter a cmd2 method to populate its ``args`` argument by parsing arguments with the given instance of argparse.ArgumentParser. @@ -339,6 +359,9 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]: statement, parsed_arglist = cmd2_app.statement_parser.get_command_arg_list( command_name, statement_arg, preserve_quotes ) + arg_parser = cmd2_app._command_parsers.get(command_name, None) + if arg_parser is None: + raise ValueError(f'No argument parser found for {command_name}') if ns_provider is None: namespace = None @@ -352,9 +375,9 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]: try: new_args: Union[Tuple[argparse.Namespace], Tuple[argparse.Namespace, List[str]]] if with_unknown_args: - new_args = parser.parse_known_args(parsed_arglist, namespace) + new_args = arg_parser.parse_known_args(parsed_arglist, namespace) else: - new_args = (parser.parse_args(parsed_arglist, namespace),) + new_args = (arg_parser.parse_args(parsed_arglist, namespace),) ns = new_args[0] except SystemExit: raise Cmd2ArgparseError @@ -374,16 +397,7 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]: args_list = _arg_swap(args, statement_arg, *new_args) return func(*args_list, **kwargs) # type: ignore[call-arg] - # argparser defaults the program name to sys.argv[0], but we want it to be the name of our command command_name = func.__name__[len(constants.COMMAND_FUNC_PREFIX) :] - _set_parser_prog(parser, command_name) - - # If the description has not been set, then use the method docstring if one exists - if parser.description is None and func.__doc__: - parser.description = strip_doc_annotations(func.__doc__) - - # Set the command's help text as argparser.description (which can be None) - cmd_wrapper.__doc__ = parser.description # Set some custom attributes for this command setattr(cmd_wrapper, constants.CMD_ATTR_ARGPARSER, parser) @@ -395,6 +409,7 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]: return arg_decorator +@overload def as_subcommand_to( command: str, subcommand: str, @@ -402,6 +417,29 @@ def as_subcommand_to( *, help: Optional[str] = None, aliases: Optional[List[str]] = None, +) -> Callable[[ArgparseCommandFunc[CommandParent]], ArgparseCommandFunc[CommandParent]]: + ... + + +@overload +def as_subcommand_to( + command: str, + subcommand: str, + parser: Callable[[], argparse.ArgumentParser], + *, + help: Optional[str] = None, + aliases: Optional[List[str]] = None, +) -> Callable[[ArgparseCommandFunc[CommandParent]], ArgparseCommandFunc[CommandParent]]: + ... + + +def as_subcommand_to( + command: str, + subcommand: str, + parser: Union[argparse.ArgumentParser, Callable[[], argparse.ArgumentParser]], + *, + help: Optional[str] = None, + aliases: Optional[List[str]] = None, ) -> Callable[[ArgparseCommandFunc[CommandParent]], ArgparseCommandFunc[CommandParent]]: """ Tag this method as a subcommand to an existing argparse decorated command. @@ -417,12 +455,6 @@ def as_subcommand_to( """ def arg_decorator(func: ArgparseCommandFunc[CommandParent]) -> ArgparseCommandFunc[CommandParent]: - _set_parser_prog(parser, command + ' ' + subcommand) - - # If the description has not been set, then use the method docstring if one exists - if parser.description is None and func.__doc__: - parser.description = func.__doc__ - # Set some custom attributes for this command setattr(func, constants.SUBCMD_ATTR_COMMAND, command) setattr(func, constants.CMD_ATTR_ARGPARSER, parser) diff --git a/cmd2/utils.py b/cmd2/utils.py index b29649a1..28586575 100644 --- a/cmd2/utils.py +++ b/cmd2/utils.py @@ -1298,3 +1298,7 @@ def suggest_similar( best_simil = simil proposed_command = each return proposed_command + + +def is_callable(candidate: Any) -> bool: + return callable(candidate) or isinstance(candidate, (staticmethod, classmethod)) diff --git a/tests/conftest.py b/tests/conftest.py index 07039504..dca3f70b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,7 @@ """ Cmd2 unit/functional testing """ +import argparse import sys from contextlib import ( redirect_stderr, @@ -190,3 +191,16 @@ def get_endidx(): with mock.patch.object(readline, 'get_begidx', get_begidx): with mock.patch.object(readline, 'get_endidx', get_endidx): return app.complete(text, 0) + + +def find_subcommand(action: argparse.ArgumentParser, subcmd_names: List[str]) -> argparse.ArgumentParser: + 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 ValueError(f"Could not find subcommand '{subcmd_names}'") diff --git a/tests/test_argparse.py b/tests/test_argparse.py index 070b506a..99ca813c 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -13,6 +13,7 @@ import cmd2 from .conftest import ( + find_subcommand, run_cmd, ) @@ -243,9 +244,6 @@ def test_preservelist(argparse_app): class SubcommandApp(cmd2.Cmd): """Example cmd2 application where we a base command which has a couple subcommands.""" - def __init__(self): - cmd2.Cmd.__init__(self) - # subcommand functions for the base command def base_foo(self, args): """foo subcommand of base command""" @@ -396,7 +394,13 @@ def test_add_another_subcommand(subcommand_app): 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. """ - new_parser = subcommand_app.base_subparsers.add_parser('new_sub', help="stuff") + base_parser = subcommand_app._command_parsers.get('base') + subcommand_parser = find_subcommand(subcommand_app._command_parsers.get('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" diff --git a/tests/transcripts/from_cmdloop.txt b/tests/transcripts/from_cmdloop.txt index f1c68d81..a615d243 100644 --- a/tests/transcripts/from_cmdloop.txt +++ b/tests/transcripts/from_cmdloop.txt @@ -2,7 +2,7 @@ # so you can see where they are. (Cmd) help say -Usage: speak [-h] [-p] [-s] [-r REPEAT]/ */ +Usage: say [-h] [-p] [-s] [-r REPEAT]/ */ Repeats what you tell me to./ */