From 9b5a98825a9b00807a40494e8c634c392077ccd2 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Tue, 25 Aug 2020 01:46:04 -0400 Subject: [PATCH] Fixed RecursionError when printing an argparse.Namespace caused by custom attribute cmd2 was adding Added get_statement() function to argparse.Namespace which returns __statement__ attribute --- CHANGELOG.md | 6 ++++++ cmd2/cmd2.py | 4 +++- cmd2/constants.py | 11 ++++------- cmd2/decorators.py | 14 ++++++++------ docs/features/argument_processing.rst | 3 ++- examples/decorator_example.py | 2 +- tests/test_argparse.py | 23 +++++++++++++++++++++++ tests/test_plugin.py | 2 +- 8 files changed, 48 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 161a058d2..f2ba27108 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.3.5 (TBD) +* Bug Fixes + * Fixed `RecursionError` when printing an `argparse.Namespace` caused by custom attribute cmd2 was adding +* Enhancements + * Added `get_statement()` function to `argparse.Namespace` which returns `__statement__` attribute + ## 1.3.4 (August 20, 2020) * Bug Fixes * Fixed `AttributeError` when `CommandSet` that uses `as_subcommand_to` decorator is loaded during diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index da5768058..bc6691f6d 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -663,7 +663,9 @@ def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None: raise CommandSetRegistrationError('Could not find argparser for command "{}" needed by subcommand: {}' .format(command_name, str(method))) - subcmd_parser.set_defaults(cmd2_handler=method) + # Set the subcommand handler function + defaults = {constants.NS_ATTR_SUBCMD_HANDLER: method} + subcmd_parser.set_defaults(**defaults) def find_subcommand(action: argparse.ArgumentParser, subcmd_names: List[str]) -> argparse.ArgumentParser: if not subcmd_names: diff --git a/cmd2/constants.py b/cmd2/constants.py index 9eaa9957c..037a7cabc 100644 --- a/cmd2/constants.py +++ b/cmd2/constants.py @@ -37,10 +37,6 @@ # All command completer functions start with this COMPLETER_FUNC_PREFIX = 'complete_' -############################################################################## -# The following are optional attributes added to do_* command functions -############################################################################## - # The custom help category a command belongs to CMD_ATTR_HELP_CATEGORY = 'help_category' @@ -50,9 +46,6 @@ # Whether or not tokens are unquoted before sending to argparse CMD_ATTR_PRESERVE_QUOTES = 'preserve_quotes' -# optional attribute -SUBCMD_HANDLER = 'cmd2_handler' - # subcommand attributes for the base command name and the subcommand name SUBCMD_ATTR_COMMAND = 'parent_command' SUBCMD_ATTR_NAME = 'subcommand_name' @@ -60,3 +53,7 @@ # arpparse attribute linking to command set instance PARSER_ATTR_COMMANDSET = 'command_set' + +# custom attributes added to argparse Namespaces +NS_ATTR_SUBCMD_HANDLER = '__subcmd_handler__' +NS_ATTR_STATEMENT = '__statement__' diff --git a/cmd2/decorators.py b/cmd2/decorators.py index 689f29c58..ccbbd8320 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -1,7 +1,6 @@ # coding=utf-8 """Decorators for ``cmd2`` commands""" import argparse -import types from typing import TYPE_CHECKING, Any, Callable, Dict, Iterable, List, Optional, Tuple, Union from . import constants @@ -190,6 +189,7 @@ def with_argparser_and_unknown_args(parser: argparse.ArgumentParser, *, of unknown argument strings. A member called ``__statement__`` is added to the ``Namespace`` to provide command functions access to the :class:`cmd2.Statement` object. This can be useful if the command function needs to know the command line. + ``__statement__`` can also be retrieved by calling ``get_statement()`` on the ``Namespace``. :Example: @@ -228,6 +228,7 @@ def with_argparser(parser: argparse.ArgumentParser, *, :return: function that gets passed the argparse-parsed args in a Namespace A member called __statement__ is added to the Namespace to provide command functions access to the Statement object. This can be useful if the command function needs to know the command line. + ``__statement__`` can also be retrieved by calling ``get_statement()`` on the ``Namespace``. :Example: @@ -297,12 +298,13 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]: except SystemExit: raise Cmd2ArgparseError else: - setattr(ns, '__statement__', statement) + # Add statement to Namespace and a getter function for it + setattr(ns, constants.NS_ATTR_STATEMENT, statement) + setattr(ns, 'get_statement', lambda: statement) - def get_handler(ns_self: argparse.Namespace) -> Optional[Callable]: - return getattr(ns_self, constants.SUBCMD_HANDLER, None) - - setattr(ns, 'get_handler', types.MethodType(get_handler, ns)) + # Add getter function for subcmd handler, which can be None + subcmd_handler = getattr(ns, constants.NS_ATTR_SUBCMD_HANDLER, None) + setattr(ns, 'get_handler', lambda: subcmd_handler) args_list = _arg_swap(args, statement, *new_args) return func(*args_list, **kwargs) diff --git a/docs/features/argument_processing.rst b/docs/features/argument_processing.rst index 9e65742eb..161702395 100644 --- a/docs/features/argument_processing.rst +++ b/docs/features/argument_processing.rst @@ -14,7 +14,8 @@ handles the following for you: 3. Passes the resulting ``argparse.Namespace`` object to your command function. The ``Namespace`` includes the ``Statement`` object that was created when parsing the command line. It is stored in the ``__statement__`` attribute of - the ``Namespace``. + the ``Namespace`` and can also be retrieved by calling ``get_statement()`` + on the ``Namespace``. 4. Adds the usage message from the argument parser to your command. diff --git a/examples/decorator_example.py b/examples/decorator_example.py index 0f5374ceb..5b721da66 100755 --- a/examples/decorator_example.py +++ b/examples/decorator_example.py @@ -67,7 +67,7 @@ def do_speak(self, args: argparse.Namespace): def do_tag(self, args: argparse.Namespace): """create an html tag""" # The Namespace always includes the Statement object created when parsing the command line - statement = args.__statement__ + statement = args.get_statement() self.poutput("The command line you ran was: {}".format(statement.command_and_args)) self.poutput("It generated this tag:") diff --git a/tests/test_argparse.py b/tests/test_argparse.py index 0d46b15a8..9806c9b53 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -290,6 +290,24 @@ def do_base(self, args): func(self, args) + # Add a subcommand using as_subcommand_to decorator + has_subcmd_parser = cmd2.Cmd2ArgumentParser(description="Tests as_subcmd_to decorator") + has_subcmd_subparsers = has_subcmd_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') + has_subcmd_subparsers.required = True + + @cmd2.with_argparser(has_subcmd_parser) + def do_test_subcmd_decorator(self, args: argparse.Namespace): + handler = args.get_handler() + handler(args) + + subcmd_parser = cmd2.Cmd2ArgumentParser(add_help=False, description="The subcommand") + + @cmd2.as_subcommand_to('test_subcmd_decorator', 'subcmd', subcmd_parser, help='the subcommand') + def subcmd_func(self, args: argparse.Namespace): + # Make sure printing the Namespace works. The way we originally added get_hander() + # to it resulted in a RecursionError when printing. + print(args) + @pytest.fixture def subcommand_app(): app = SubcommandApp() @@ -373,6 +391,11 @@ def test_add_another_subcommand(subcommand_app): assert new_parser.prog == "base new_sub" +def test_subcmd_decorator(subcommand_app): + out, err = run_cmd(subcommand_app, 'test_subcmd_decorator subcmd') + assert out[0].startswith('Namespace(') + + def test_unittest_mock(): from unittest import mock from cmd2 import CommandSetRegistrationError diff --git a/tests/test_plugin.py b/tests/test_plugin.py index e49cbbfc3..d40a9b61e 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -279,7 +279,7 @@ def do_skip_postcmd_hooks(self, _): @with_argparser(parser) def do_argparse_cmd(self, namespace: argparse.Namespace): """Repeat back the arguments""" - self.poutput(namespace.__statement__) + self.poutput(namespace.get_statement()) ### #