From 1319da72f2ae0dc14d6f0631ce654007691cd839 Mon Sep 17 00:00:00 2001 From: Nick Stenning Date: Sun, 8 Feb 2015 11:28:06 +0000 Subject: [PATCH] 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 | 68 ++++++++++++++--------------- honcho/test/integration/test_run.py | 10 +++++ 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/honcho/command.py b/honcho/command.py index 75e0115..18f8d53 100644 --- a/honcho/command.py +++ b/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 = { diff --git a/honcho/test/integration/test_run.py b/honcho/test/integration/test_run.py index 3138616..68101aa 100644 --- a/honcho/test/integration/test_run.py +++ b/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')