Merge pull request #928 from python-cmd2/subcmd_aliases

Fixed argparse usage text

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"