From e68867df1b377666d0cd02ebcd0b0112a79a7699 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 | 115 ++++++++++++++++++++--------- cmd2/decorators.py | 75 +++++++++++++------ cmd2/utils.py | 4 + tests/conftest.py | 14 ++++ tests/test_argparse.py | 53 ++++++++----- tests/transcripts/from_cmdloop.txt | 2 +- 6 files changed, 188 insertions(+), 75 deletions(-) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index f1e6c847..a4edb7f2 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,43 @@ def register_command_set(self, cmdset: CommandSet) -> None: cmdset.on_unregistered() raise + def _build_parser(self, parent: Union['Cmd', CommandSet], parser_builder: Any) -> Optional[argparse.ArgumentParser]: + parser: Optional[argparse.ArgumentParser] = None + if is_callable(parser_builder): + if isinstance(parser_builder, staticmethod): + parser = cast(argparse.ArgumentParser, parser_builder.__func__()) + elif isinstance(parser_builder, classmethod): + 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: # pragma: no cover + parser = parser_builder + return parser + + 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) + parent = self.find_commandset_for_command(command) or self + parser = self._build_parser(parent, parser_builder) + if parser is None: + 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 +727,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 +775,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 +796,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 +835,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 +855,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 +875,17 @@ def find_subcommand(action: argparse.ArgumentParser, subcmd_names: List[str]) -> target_parser = find_subcommand(command_parser, subcommand_names) + subcmd_parser = cast(argparse.ArgumentParser, self._build_parser(cmdset, 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 +957,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 +2078,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 +3303,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 +3730,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 +3766,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 +3902,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 +3948,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..3a163fda 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,17 +259,39 @@ 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]]: + ... # pragma: no cover + + +@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]]: + ... # pragma: no cover + + +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. - :param parser: unique instance of ArgumentParser + :param parser: unique instance of ArgumentParser or a callable that returns an ArgumentParser :param ns_provider: An optional function that accepts a cmd2.Cmd or cmd2.CommandSet object as an argument and returns an argparse.Namespace. This is useful if the Namespace needs to be prepopulated with state data that affects parsing. @@ -339,6 +359,10 @@ 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: + # This shouldn't be possible to reach + raise ValueError(f'No argument parser found for {command_name}') # pragma: no cover if ns_provider is None: namespace = None @@ -352,9 +376,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 +398,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 +410,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 +418,29 @@ def as_subcommand_to( *, help: Optional[str] = None, aliases: Optional[List[str]] = None, +) -> Callable[[ArgparseCommandFunc[CommandParent]], ArgparseCommandFunc[CommandParent]]: + ... # pragma: no cover + + +@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]]: + ... # pragma: no cover + + +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 +456,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..19512aff 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -13,6 +13,7 @@ import cmd2 from .conftest import ( + find_subcommand, run_cmd, ) @@ -27,13 +28,16 @@ def namespace_provider(self) -> argparse.Namespace: ns.custom_stuff = "custom" return ns - say_parser = cmd2.Cmd2ArgumentParser() - say_parser.add_argument('-p', '--piglatin', action='store_true', help='atinLay') - say_parser.add_argument('-s', '--shout', action='store_true', help='N00B EMULATION MODE') - say_parser.add_argument('-r', '--repeat', type=int, help='output [n] times') - say_parser.add_argument('words', nargs='+', help='words to say') + @staticmethod + def _say_parser_builder() -> cmd2.Cmd2ArgumentParser: + say_parser = cmd2.Cmd2ArgumentParser() + say_parser.add_argument('-p', '--piglatin', action='store_true', help='atinLay') + say_parser.add_argument('-s', '--shout', action='store_true', help='N00B EMULATION MODE') + say_parser.add_argument('-r', '--repeat', type=int, help='output [n] times') + say_parser.add_argument('words', nargs='+', help='words to say') + return say_parser - @cmd2.with_argparser(say_parser) + @cmd2.with_argparser(_say_parser_builder) def do_say(self, args, *, keyword_arg: Optional[str] = None): """ Repeat what you @@ -86,12 +90,15 @@ def do_arglist(self, arglist, *, keyword_arg: Optional[str] = None): def do_preservelist(self, arglist): self.stdout.write('{}'.format(arglist)) - known_parser = cmd2.Cmd2ArgumentParser() - known_parser.add_argument('-p', '--piglatin', action='store_true', help='atinLay') - known_parser.add_argument('-s', '--shout', action='store_true', help='N00B EMULATION MODE') - known_parser.add_argument('-r', '--repeat', type=int, help='output [n] times') + @classmethod + def _speak_parser_builder(cls) -> cmd2.Cmd2ArgumentParser: + known_parser = cmd2.Cmd2ArgumentParser() + known_parser.add_argument('-p', '--piglatin', action='store_true', help='atinLay') + known_parser.add_argument('-s', '--shout', action='store_true', help='N00B EMULATION MODE') + known_parser.add_argument('-r', '--repeat', type=int, help='output [n] times') + return known_parser - @cmd2.with_argparser(known_parser, with_unknown_args=True) + @cmd2.with_argparser(_speak_parser_builder, with_unknown_args=True) def do_speak(self, args, extra, *, keyword_arg: Optional[str] = None): """Repeat what you tell me to.""" words = [] @@ -240,12 +247,16 @@ def test_preservelist(argparse_app): assert out[0] == "['foo', '\"bar baz\"']" +def _build_has_subcmd_parser() -> cmd2.Cmd2ArgumentParser: + has_subcmds_parser = cmd2.Cmd2ArgumentParser(description="Tests as_subcmd_to decorator") + has_subcmds_subparsers = has_subcmds_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') + has_subcmds_subparsers.required = True + return has_subcmds_parser + + 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""" @@ -291,11 +302,7 @@ def do_base(self, args): func(self, args) # Add subcommands using as_subcommand_to decorator - has_subcmds_parser = cmd2.Cmd2ArgumentParser(description="Tests as_subcmd_to decorator") - has_subcmds_subparsers = has_subcmds_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') - has_subcmds_subparsers.required = True - - @cmd2.with_argparser(has_subcmds_parser) + @cmd2.with_argparser(_build_has_subcmd_parser) def do_test_subcmd_decorator(self, args: argparse.Namespace): handler = args.cmd2_handler.get() handler(args) @@ -396,7 +403,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./ */