Fix argument parsing in Python 2.7.9

From CPython changeset 1a3143752db2:

    Before, if a default was set on the parent parser, any default for
    that variable set via set_defaults on a subparser would be ignored.
    Now the subparser set_defaults is honored.

This is a problem for us as it means that any arguments supplied to the
parent parser will be blatted by the subparser defaults, meaning that
invocations such as

    honcho -e .env.custom run ...

will not work in Python 2.7.9. This change fixes that by adding the
common args such as "-e", "-f", "-d" to each subparser with a default of
argparse.SUPPRESS, so that the default (and value) is inherited from the
parent parser if the argument is not supplied after the subcommand.

Thanks to @Changaco for the original bug report and fix implementation.

honcho/command.py

@@ -35,29 +35,29 @@
 class CommandError(Exception):
     pass
 
-_parent_parser = argparse.ArgumentParser(
-    'honcho',
-    description='Manage Procfile-based applications',
-    add_help=False)
-_parent_parser.add_argument(
-    '-e', '--env',
-    help='environment file[,file]', default='.env')
-_parent_parser.add_argument(
-    '-d', '--app-root',
-    help='procfile directory', default='.')
-_parent_parser.add_argument(
-    '-f', '--procfile',
-    help='procfile path', default='Procfile')
-_parent_parser.add_argument(
-    '-v', '--version',
-    action='version', version='%(prog)s ' + __version__)
-
-_parser_defaults = {
-    'parents': [_parent_parser],
-    'formatter_class': argparse.ArgumentDefaultsHelpFormatter,
-}
 
-parser = argparse.ArgumentParser(**_parser_defaults)
+def _add_common_args(parser, with_defaults=False):
+    suppress = None if with_defaults else argparse.SUPPRESS
+    parser.add_argument('-e', '--env',
+                        default=(suppress or '.env'),
+                        help='environment file[,file] (default: .env)')
+    parser.add_argument('-d', '--app-root',
+                        metavar='DIR',
+                        default=(suppress or '.'),
+                        help='procfile directory (default: .)')
+    parser.add_argument('-f', '--procfile',
+                        metavar='FILE',
+                        default=(suppress or 'Procfile'),
+                        help='procfile path (default: Procfile)')
+    parser.add_argument('-v', '--version',
+                        action='version',
+                        version='%(prog)s ' + __version__)
+
+
+parser = argparse.ArgumentParser(
+    'honcho',
+    description='Manage Procfile-based applications')
+_add_common_args(parser, with_defaults=True)
 
 subparsers = parser.add_subparsers(title='tasks', dest='command')
 subparsers.required = True
@@ -70,8 +70,8 @@ def command_check(args):
 
 parser_check = subparsers.add_parser(
     'check',
-    help="validate a Procfile",
-    **_parser_defaults)
+    help="validate a Procfile")
+_add_common_args(parser_check)
 
 
 def command_export(args):
@@ -109,8 +109,8 @@ def command_export(args):
 
 parser_export = subparsers.add_parser(
     'export',
-    help="export a Procfile to another format",
-    **_parser_defaults)
+    help="export a Procfile to another format")
+_add_common_args(parser_export)
 parser_export.add_argument(
     '-a', '--app',
     help="alternative app name", default=BASENAME, type=str, metavar='APP')
@@ -153,8 +153,7 @@ def command_help(args):
 
 parser_help = subparsers.add_parser(
     'help',
-    help="describe available tasks or one specific task",
-    **_parser_defaults)
+    help="describe available tasks or one specific task")
 parser_help.add_argument('task', help='task to show help for', nargs='?')
 
 
@@ -177,8 +176,8 @@ def command_run(args):
 
 parser_run = subparsers.add_parser(
     'run',
-    help="run a command using your application's environment",
-    **_parser_defaults)
+    help="run a command using your application's environment")
+_add_common_args(parser_run)
 parser_run.add_argument(
     'argv',
     nargs=argparse.REMAINDER,
@@ -220,8 +219,8 @@ def command_start(args):
 
 parser_start = subparsers.add_parser(
     'start',
-    help="start the application (or a specific PROCESS)",
-    **_parser_defaults)
+    help="start the application (or a specific PROCESS)")
+_add_common_args(parser_start)
 parser_start.add_argument(
     '-p', '--port',
     help="starting port number (default: 5000)",
@@ -236,7 +235,7 @@ def command_start(args):
     type=str, metavar='process1,process2,process3')
 parser_start.add_argument(
     'processes', nargs='*',
-    help='process(es) to start (default: all processes will be run)')
+    help='process(es) to start (default: all processes)')
 
 
 def command_version(args):
@@ -247,8 +246,7 @@ def command_version(args):
 
 parser_version = subparsers.add_parser(
     'version',
-    help="display honcho version",
-    **_parser_defaults)
+    help="display honcho version")
 
 
 COMMANDS = {

honcho/test/integration/test_run.py

@@ -30,3 +30,13 @@ def test_run_env(self):
 
         self.assertEqual(ret, 0)
         self.assertEqual(out, 'giraffe\n')
+
+    def test_run_args_before_command(self):
+        # Regression test for #122 -- ensure that common args can be given
+        # before the subcommand.
+        with TestEnv({'.env.x': 'ANIMAL=giraffe', 'test.py': script}) as env:
+            ret, out, err = env.run_honcho(['-e', '.env.x',
+                                            'run', python_bin, 'test.py'])
+
+        self.assertEqual(ret, 0)
+        self.assertEqual(out, 'giraffe\n')