Fixed issue where subcommand usage text could contain a subcommand alias instead of the actual name

Author: kmvanbrunt

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

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

tests/test_argparse.py

@@ -233,9 +233,14 @@ def base_bar(self, args):
         """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 @@ def base_bar(self, args):
     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')