diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | cmd2/decorators.py | 31 | ||||
| -rw-r--r-- | tests/test_argparse.py | 69 |
3 files changed, 86 insertions, 16 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 23596a4b..c6fbd466 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ ## 1.0.3 (TBD, 2020) +* Bug Fixes + * Fixed issue where subcommand usage text could contain a subcommand alias instead of the actual name * Enhancements * Made `ipy` consistent with `py` in the following ways * `ipy` returns whether any of the commands run in it returned True to stop command loop diff --git a/cmd2/decorators.py b/cmd2/decorators.py index dc196032..d2fdf9c7 100644 --- a/cmd2/decorators.py +++ b/cmd2/decorators.py @@ -85,8 +85,9 @@ def _set_parser_prog(parser: argparse.ArgumentParser, prog: str): """ Recursively set prog attribute of a parser and all of its subparsers so that the root command is a command name and not sys.argv[0]. + :param parser: the parser being edited - :param prog: value for the current parsers prog attribute + :param prog: new value for the parser's prog attribute """ # Set the prog value for this parser parser.prog = prog @@ -94,11 +95,29 @@ def _set_parser_prog(parser: argparse.ArgumentParser, prog: str): # Set the prog value for the parser's subcommands for action in parser._actions: if isinstance(action, argparse._SubParsersAction): - - # Set the prog value for each subcommand - for sub_cmd, sub_cmd_parser in action.choices.items(): - sub_cmd_prog = parser.prog + ' ' + sub_cmd - _set_parser_prog(sub_cmd_parser, sub_cmd_prog) + # 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 + ' ' + 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 diff --git a/tests/test_argparse.py b/tests/test_argparse.py index 9510e8f2..db6dea21 100644 --- a/tests/test_argparse.py +++ b/tests/test_argparse.py @@ -233,9 +233,14 @@ class SubcommandApp(cmd2.Cmd): """bar subcommand of base command""" self.poutput('((%s))' % args.z) + def base_helpless(self, args): + """helpless subcommand of base command""" + self.poutput('((%s))' % args.z) + # create the top-level parser for the base command base_parser = argparse.ArgumentParser() - base_subparsers = base_parser.add_subparsers(title='subcommands', help='subcommand help') + base_subparsers = base_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND') + base_subparsers.required = True # create the parser for the "foo" subcommand parser_foo = base_subparsers.add_parser('foo', help='foo help') @@ -244,20 +249,24 @@ class SubcommandApp(cmd2.Cmd): parser_foo.set_defaults(func=base_foo) # create the parser for the "bar" subcommand - parser_bar = base_subparsers.add_parser('bar', help='bar help') + parser_bar = base_subparsers.add_parser('bar', help='bar help', aliases=['bar_1', 'bar_2']) + parser_bar.add_argument('z', help='string') + 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. + parser_bar = base_subparsers.add_parser('helpless', aliases=['helpless_1', 'helpless_2']) parser_bar.add_argument('z', help='string') parser_bar.set_defaults(func=base_bar) @cmd2.with_argparser(base_parser) def do_base(self, args): """Base command help""" - func = getattr(args, 'func', None) - if func is not None: - # Call whatever subcommand function was selected - func(self, args) - else: - # No subcommand was provided, so call help - self.do_help('base') + # Call whatever subcommand function was selected + func = getattr(args, 'func') + func(self, args) @pytest.fixture def subcommand_app(): @@ -277,7 +286,7 @@ def test_subcommand_bar(subcommand_app): def test_subcommand_invalid(subcommand_app): out, err = run_cmd(subcommand_app, 'base baz') assert err[0].startswith('usage: base') - assert err[1].startswith("base: error: invalid choice: 'baz'") + assert err[1].startswith("base: error: argument SUBCOMMAND: invalid choice: 'baz'") def test_subcommand_base_help(subcommand_app): out, err = run_cmd(subcommand_app, 'help base') @@ -286,12 +295,52 @@ def test_subcommand_base_help(subcommand_app): assert out[2] == 'Base command help' def test_subcommand_help(subcommand_app): + # foo has no aliases out, err = run_cmd(subcommand_app, 'help base foo') assert out[0].startswith('usage: base foo') assert out[1] == '' assert out[2] == 'positional arguments:' + # bar has aliases (usage should never show alias name) + out, err = run_cmd(subcommand_app, 'help base bar') + assert out[0].startswith('usage: base bar') + assert out[1] == '' + assert out[2] == 'positional arguments:' + + out, err = run_cmd(subcommand_app, 'help base bar_1') + assert out[0].startswith('usage: base bar') + assert out[1] == '' + assert out[2] == 'positional arguments:' + + out, err = run_cmd(subcommand_app, 'help base bar_2') + assert out[0].startswith('usage: base bar') + assert out[1] == '' + assert out[2] == 'positional arguments:' + + # helpless has aliases and no help text (usage should never show alias name) + out, err = run_cmd(subcommand_app, 'help base helpless') + assert out[0].startswith('usage: base helpless') + assert out[1] == '' + assert out[2] == 'positional arguments:' + + out, err = run_cmd(subcommand_app, 'help base helpless_1') + assert out[0].startswith('usage: base helpless') + assert out[1] == '' + assert out[2] == 'positional arguments:' + + out, err = run_cmd(subcommand_app, 'help base helpless_2') + assert out[0].startswith('usage: base helpless') + assert out[1] == '' + assert out[2] == 'positional arguments:' def test_subcommand_invalid_help(subcommand_app): out, err = run_cmd(subcommand_app, 'help base baz') assert out[0].startswith('usage: base') + +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") + assert new_parser.prog == "base new_sub" |