Merge pull request #988 from python-cmd2/add_help

Removed need to set add_help to False for @as_subcommand_to decorator
python-cmd2 · Aug 28, 2020 · 36b0b75 · 36b0b75
2 parents 47f8652 + e3ed15e
commit 36b0b75
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,12 @@
+## 1.3.8 (August 28, 2020)
+* Bug Fixes
+    * Fixed issue where subcommand added with `@as_subcommand_to` decorator did not display help
+    when called with `-h/--help`.
+* Enhancements
+    * `add_help=False` no longer has to be passed to parsers used in `@as_subcommand_to` decorator.
+      Only pass this if your subcommand should not have the `-h/--help` help option (as stated in
+      argparse documentation).
+
 ## 1.3.7 (August 27, 2020)
 * Bug Fixes
     * Fixes an issue introduced in 1.3.0 with processing command strings containing terminator/separator 

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
@@ -701,9 +701,11 @@ def find_subcommand(action: argparse.ArgumentParser, subcmd_names: List[str]) ->
                     add_parser_kwargs['fromfile_prefix_chars'] = subcmd_parser.fromfile_prefix_chars
                     add_parser_kwargs['argument_default'] = subcmd_parser.argument_default
                     add_parser_kwargs['conflict_handler'] = subcmd_parser.conflict_handler
-                    add_parser_kwargs['add_help'] = subcmd_parser.add_help
                     add_parser_kwargs['allow_abbrev'] = subcmd_parser.allow_abbrev
 
+                    # Set add_help to False and use whatever help option subcmd_parser already has
+                    add_parser_kwargs['add_help'] = False
+
                     attached_parser = action.add_parser(subcommand_name, **add_parser_kwargs)
                     setattr(attached_parser, constants.PARSER_ATTR_COMMANDSET, cmdset)
                     break
@@ -2703,8 +2705,7 @@ def do_alias(self, args: argparse.Namespace) -> None:
                            "  alias create show_log !cat \"log file.txt\"\n"
                            "  alias create save_results print_results \">\" out.txt\n")
 
-    alias_create_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=alias_create_description,
-                                                  epilog=alias_create_epilog)
+    alias_create_parser = DEFAULT_ARGUMENT_PARSER(description=alias_create_description, epilog=alias_create_epilog)
     alias_create_parser.add_argument('name', help='name of this alias')
     alias_create_parser.add_argument('command', help='what the alias resolves to',
                                      choices_method=_get_commands_aliases_and_macros_for_completion)
@@ -2748,7 +2749,7 @@ def _alias_create(self, args: argparse.Namespace) -> None:
     alias_delete_help = "delete aliases"
     alias_delete_description = "Delete specified aliases or all aliases if --all is used"
 
-    alias_delete_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=alias_delete_description)
+    alias_delete_parser = DEFAULT_ARGUMENT_PARSER(description=alias_delete_description)
     alias_delete_parser.add_argument('names', nargs=argparse.ZERO_OR_MORE, help='alias(es) to delete',
                                      choices_method=_get_alias_completion_items, descriptive_header='Value')
     alias_delete_parser.add_argument('-a', '--all', action='store_true', help="delete all aliases")
@@ -2776,7 +2777,7 @@ def _alias_delete(self, args: argparse.Namespace) -> None:
                               "\n"
                               "Without arguments, all aliases will be listed.")
 
-    alias_list_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=alias_list_description)
+    alias_list_parser = DEFAULT_ARGUMENT_PARSER(description=alias_list_description)
     alias_list_parser.add_argument('names', nargs=argparse.ZERO_OR_MORE, help='alias(es) to list',
                                    choices_method=_get_alias_completion_items, descriptive_header='Value')
 
@@ -2854,8 +2855,7 @@ def do_macro(self, args: argparse.Namespace) -> None:
                            "  Because macros do not resolve until after hitting Enter, tab completion\n"
                            "  will only complete paths while typing a macro.")
 
-    macro_create_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=macro_create_description,
-                                                  epilog=macro_create_epilog)
+    macro_create_parser = DEFAULT_ARGUMENT_PARSER(description=macro_create_description, epilog=macro_create_epilog)
     macro_create_parser.add_argument('name', help='name of this macro')
     macro_create_parser.add_argument('command', help='what the macro resolves to',
                                      choices_method=_get_commands_aliases_and_macros_for_completion)
@@ -2945,7 +2945,7 @@ def _macro_create(self, args: argparse.Namespace) -> None:
     # macro -> delete
     macro_delete_help = "delete macros"
     macro_delete_description = "Delete specified macros or all macros if --all is used"
-    macro_delete_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=macro_delete_description)
+    macro_delete_parser = DEFAULT_ARGUMENT_PARSER(description=macro_delete_description)
     macro_delete_parser.add_argument('names', nargs=argparse.ZERO_OR_MORE, help='macro(s) to delete',
                                      choices_method=_get_macro_completion_items, descriptive_header='Value')
     macro_delete_parser.add_argument('-a', '--all', action='store_true', help="delete all macros")
@@ -2973,7 +2973,7 @@ def _macro_delete(self, args: argparse.Namespace) -> None:
                               "\n"
                               "Without arguments, all macros will be listed.")
 
-    macro_list_parser = DEFAULT_ARGUMENT_PARSER(add_help=False, description=macro_list_description)
+    macro_list_parser = DEFAULT_ARGUMENT_PARSER(description=macro_list_description)
     macro_list_parser.add_argument('names', nargs=argparse.ZERO_OR_MORE, help='macro(s) to list',
                                    choices_method=_get_macro_completion_items, descriptive_header='Value')
 

diff --git a/docs/features/modular_commands.rst b/docs/features/modular_commands.rst
@@ -244,7 +244,7 @@ command and each CommandSet
         def do_apple(self, _: cmd2.Statement):
             self._cmd.poutput('Apple')
 
-        banana_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+        banana_parser = cmd2.Cmd2ArgumentParser()
         banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
 
         @cmd2.as_subcommand_to('cut', 'banana', banana_parser)
@@ -261,7 +261,7 @@ command and each CommandSet
         def do_arugula(self, _: cmd2.Statement):
             self._cmd.poutput('Arugula')
 
-        bokchoy_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+        bokchoy_parser = cmd2.Cmd2ArgumentParser()
         bokchoy_parser.add_argument('style', choices=['quartered', 'diced'])
 
         @cmd2.as_subcommand_to('cut', 'bokchoy', bokchoy_parser)

diff --git a/examples/modular_subcommands.py b/examples/modular_subcommands.py
@@ -24,7 +24,7 @@ def do_apple(self, _: cmd2.Statement):
         self._cmd.poutput('Apple')
 
     banana_description = "Cut a banana"
-    banana_parser = cmd2.Cmd2ArgumentParser(add_help=False, description=banana_description)
+    banana_parser = cmd2.Cmd2ArgumentParser(description=banana_description)
     banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
 
     @cmd2.as_subcommand_to('cut', 'banana', banana_parser, help=banana_description.lower())
@@ -42,7 +42,7 @@ def do_arugula(self, _: cmd2.Statement):
         self._cmd.poutput('Arugula')
 
     bokchoy_description = "Cut some bokchoy"
-    bokchoy_parser = cmd2.Cmd2ArgumentParser(add_help=False, description=bokchoy_description)
+    bokchoy_parser = cmd2.Cmd2ArgumentParser(description=bokchoy_description)
     bokchoy_parser.add_argument('style', choices=['quartered', 'diced'])
 
     @cmd2.as_subcommand_to('cut', 'bokchoy', bokchoy_parser, help=bokchoy_description.lower())

diff --git a/tests/test_argparse.py b/tests/test_argparse.py
@@ -289,24 +289,32 @@ def do_base(self, args):
         func = getattr(args, 'func')
         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
+    # Add subcommands using as_subcommand_to decorator
+    has_subcmds_parser = cmd2.Cmd2ArgumentParser(description="Tests as_subcmd_to decorator")
+    has_subcmds_subparsers = has_subcmds_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND')
+    has_subcmds_subparsers.required = True
 
-    @cmd2.with_argparser(has_subcmd_parser)
+    @cmd2.with_argparser(has_subcmds_parser)
     def do_test_subcmd_decorator(self, args: argparse.Namespace):
         handler = args.cmd2_handler.get()
         handler(args)
 
-    subcmd_parser = cmd2.Cmd2ArgumentParser(add_help=False, description="The subcommand")
+    subcmd_parser = cmd2.Cmd2ArgumentParser(description="A subcommand")
 
-    @cmd2.as_subcommand_to('test_subcmd_decorator', 'subcmd', subcmd_parser, help='the subcommand')
+    @cmd2.as_subcommand_to('test_subcmd_decorator', 'subcmd', subcmd_parser, help=subcmd_parser.description.lower())
     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.
+        # Make sure printing the Namespace works. The way we originally added cmd2_hander to it resulted in a RecursionError.
         self.poutput(args)
 
+    helpless_subcmd_parser = cmd2.Cmd2ArgumentParser(add_help=False, description="A subcommand with no help")
+
+    @cmd2.as_subcommand_to('test_subcmd_decorator', 'helpless_subcmd', helpless_subcmd_parser,
+                           help=helpless_subcmd_parser.description.lower())
+    def helpless_subcmd_func(self, args: argparse.Namespace):
+        # Make sure vars(Namespace) works. The way we originally added cmd2_hander to it resulted in a RecursionError.
+        self.poutput(vars(args))
+
+
 @pytest.fixture
 def subcommand_app():
     app = SubcommandApp()
@@ -391,9 +399,29 @@ def test_add_another_subcommand(subcommand_app):
 
 
 def test_subcmd_decorator(subcommand_app):
+    # Test subcommand that has help option
     out, err = run_cmd(subcommand_app, 'test_subcmd_decorator subcmd')
     assert out[0].startswith('Namespace(')
 
+    out, err = run_cmd(subcommand_app, 'help test_subcmd_decorator subcmd')
+    assert out[0] == 'Usage: test_subcmd_decorator subcmd [-h]'
+
+    out, err = run_cmd(subcommand_app, 'test_subcmd_decorator subcmd -h')
+    assert out[0] == 'Usage: test_subcmd_decorator subcmd [-h]'
+
+    # Test subcommand that has no help option
+    out, err = run_cmd(subcommand_app, 'test_subcmd_decorator helpless_subcmd')
+    assert "'subcommand': 'helpless_subcmd'" in out[0]
+
+    out, err = run_cmd(subcommand_app, 'help test_subcmd_decorator helpless_subcmd')
+    assert out[0] == 'Usage: test_subcmd_decorator helpless_subcmd'
+    assert not err
+
+    out, err = run_cmd(subcommand_app, 'test_subcmd_decorator helpless_subcmd -h')
+    assert not out
+    assert err[0] == 'Usage: test_subcmd_decorator [-h] SUBCOMMAND ...'
+    assert err[1] == 'Error: unrecognized arguments: -h'
+
 
 def test_unittest_mock():
     from unittest import mock

diff --git a/tests_isolated/test_commandset/test_commandset.py b/tests_isolated/test_commandset/test_commandset.py
@@ -76,7 +76,7 @@ def do_main(self, args: argparse.Namespace) -> None:
         handler(args)
 
     # main -> sub
-    subcmd_parser = cmd2.Cmd2ArgumentParser(add_help=False, description="Sub Command")
+    subcmd_parser = cmd2.Cmd2ArgumentParser(description="Sub Command")
 
     @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command")
     def subcmd_func(self, args: argparse.Namespace) -> None:
@@ -339,7 +339,7 @@ def do_stir(self, ns: argparse.Namespace):
             self._cmd.pwarning('This command does nothing without sub-parsers registered')
             self._cmd.do_help('stir')
 
-    stir_pasta_parser = cmd2.Cmd2ArgumentParser('pasta', add_help=False)
+    stir_pasta_parser = cmd2.Cmd2ArgumentParser()
     stir_pasta_parser.add_argument('--option', '-o')
     stir_pasta_parser.add_subparsers(title='style', help='Stir style')
 
@@ -379,7 +379,7 @@ def __init__(self, dummy):
     def do_apple(self, _: cmd2.Statement):
         self._cmd.poutput('Apple')
 
-    banana_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+    banana_parser = cmd2.Cmd2ArgumentParser()
     banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
 
     @cmd2.as_subcommand_to('cut', 'banana', banana_parser, help='Cut banana', aliases=['bananer'])
@@ -393,7 +393,7 @@ def __init__(self, dummy):
         super(LoadablePastaStir, self).__init__()
         self._dummy = dummy  # prevents autoload
 
-    stir_pasta_vigor_parser = cmd2.Cmd2ArgumentParser('vigor', add_help=False)
+    stir_pasta_vigor_parser = cmd2.Cmd2ArgumentParser()
     stir_pasta_vigor_parser.add_argument('frequency')
 
     @cmd2.as_subcommand_to('stir pasta', 'vigorously', stir_pasta_vigor_parser)
@@ -413,7 +413,7 @@ def do_arugula(self, _: cmd2.Statement):
     def complete_style_arg(self, text: str, line: str, begidx: int, endidx: int) -> List[str]:
         return ['quartered', 'diced']
 
-    bokchoy_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+    bokchoy_parser = cmd2.Cmd2ArgumentParser()
     bokchoy_parser.add_argument('style', completer_method=complete_style_arg)
 
     @cmd2.as_subcommand_to('cut', 'bokchoy', bokchoy_parser)
@@ -561,7 +561,7 @@ def __init__(self, dummy):
             super(BadNestedSubcommands, self).__init__()
             self._dummy = dummy  # prevents autoload
 
-        stir_pasta_vigor_parser = cmd2.Cmd2ArgumentParser('vigor', add_help=False)
+        stir_pasta_vigor_parser = cmd2.Cmd2ArgumentParser()
         stir_pasta_vigor_parser.add_argument('frequency')
 
         # stir sauce doesn't exist anywhere, this should fail
@@ -607,7 +607,7 @@ def do_cut(self, ns: argparse.Namespace):
             self.poutput('This command does nothing without sub-parsers registered')
             self.do_help('cut')
 
-    banana_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+    banana_parser = cmd2.Cmd2ArgumentParser()
     banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
 
     @cmd2.as_subcommand_to('cut', 'banana', banana_parser, help='Cut banana', aliases=['bananer'])
@@ -618,7 +618,7 @@ def cut_banana(self, ns: argparse.Namespace):
     def complete_style_arg(self, text: str, line: str, begidx: int, endidx: int) -> List[str]:
         return ['quartered', 'diced']
 
-    bokchoy_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+    bokchoy_parser = cmd2.Cmd2ArgumentParser()
     bokchoy_parser.add_argument('style', completer_method=complete_style_arg)
 
     @cmd2.as_subcommand_to('cut', 'bokchoy', bokchoy_parser)
@@ -861,7 +861,7 @@ def do_cut(self, ns: argparse.Namespace):
             """Cut something"""
             pass
 
-        banana_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+        banana_parser = cmd2.Cmd2ArgumentParser()
         banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
 
         @cmd2.as_subcommand_to('cut', 'bad name', banana_parser, help='This should fail')