summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Van Brunt <kmvanbrunt@gmail.com>2020-04-23 13:57:30 -0400
committerKevin Van Brunt <kmvanbrunt@gmail.com>2020-04-23 13:57:30 -0400
commitdcafd4e6ee7069009c8ea87f69cd8d6f9fe99067 (patch)
tree12ae06e0d334699b3abdf5afbe43766da9c71ea4
parent0bc9217858dccaf3f0f6490c46a2f831a739ecf9 (diff)
downloadcmd2-git-dcafd4e6ee7069009c8ea87f69cd8d6f9fe99067.tar.gz
Fixed issue where subcommand usage text could contain a subcommand alias instead of the actual name
-rw-r--r--CHANGELOG.md2
-rw-r--r--cmd2/decorators.py25
-rw-r--r--tests/test_argparse.py62
3 files changed, 74 insertions, 15 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..7d097534 100644
--- a/cmd2/decorators.py
+++ b/cmd2/decorators.py
@@ -85,6 +85,7 @@ 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
"""
@@ -94,11 +95,25 @@ 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)
+ # 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..f9ee0d07 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,11 +295,44 @@ 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')