From cb300d7af3a044cc1e075be82b7afd1d5fa89f5a Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Wed, 22 Feb 2017 21:21:19 +0100 Subject: [PATCH 1/7] Ah, the item list is getting recycled, which is causing the merging of options across groups. --- sphinxarg/ext.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinxarg/ext.py b/sphinxarg/ext.py index 3dd2d62..117a898 100644 --- a/sphinxarg/ext.py +++ b/sphinxarg/ext.py @@ -67,10 +67,10 @@ def print_arg_list(data, nested_content): def print_opt_list(data, nested_content): definitions = map_nested_definitions(nested_content) - items = [] nodes_list = [] # dictionary to hold the group options, the group title is used as the key if 'action_groups' in data: for action_group in data['action_groups']: + items = [] if 'options' in action_group: for opt in action_group['options']: names = [] From 319ae85027bbb79232b5789e88e51dea05cdd3d7 Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Thu, 23 Feb 2017 00:38:03 +0100 Subject: [PATCH 2/7] Arguments in argument groups are no longer duplicated. Positional arguments are no longer specially treated. --- sphinxarg/parser.py | 115 ++++++++++++++++++------------------- test/test_parser.py | 136 +++++++++++++++++++++++++++++--------------- 2 files changed, 146 insertions(+), 105 deletions(-) diff --git a/sphinxarg/parser.py b/sphinxarg/parser.py index 880a7f0..55622dc 100644 --- a/sphinxarg/parser.py +++ b/sphinxarg/parser.py @@ -60,70 +60,50 @@ def parse_parser(parser, data=None, **kwargs): _try_add_parser_attribute(data, parser, 'description') _try_add_parser_attribute(data, parser, 'epilog') for action in parser._get_positional_actions(): - if isinstance(action, _HelpAction): + if not isinstance(action, _SubParsersAction): continue - if isinstance(action, _SubParsersAction): - helps = {} - for item in action._choices_actions: - helps[item.dest] = item.help - - # commands which share an existing parser are an alias, - # don't duplicate docs - subsection_alias = {} - subsection_alias_names = set() - for name, subaction in action._name_parser_map.items(): - if subaction not in subsection_alias: - subsection_alias[subaction] = [] - else: - subsection_alias[subaction].append(name) - subsection_alias_names.add(name) - - for name, subaction in action._name_parser_map.items(): - if name in subsection_alias_names: - continue - subalias = subsection_alias[subaction] - subaction.prog = '%s %s' % (parser.prog, name) - subdata = { - 'name': name if not subalias else '%s (%s)' % (name, ', '.join(subalias)), - 'help': helps.get(name, ''), - 'usage': subaction.format_usage().strip(), - 'bare_usage': _format_usage_without_prefix(subaction), - } - parse_parser(subaction, subdata, **kwargs) - data.setdefault('children', []).append(subdata) - continue - if 'args' not in data: - data['args'] = [] - - # Fill in things like %(prog)s in the help section. - # Note that if any keyword is missing, then nothing is filled in. - formatDict = dict(vars(action), prog=data.get('prog', '')) - helpStr = action.help or '' # Ensure we don't print None - try: - helpStr = helpStr % formatDict - except: - pass - - arg = { - 'name': action.dest, - 'help': helpStr, - 'metavar': action.metavar - } - if action.choices: - arg['choices'] = action.choices - data['args'].append(arg) - show_defaults = ( - ('skip_default_values' not in kwargs) or - (kwargs['skip_default_values'] is False)) + helps = {} + for item in action._choices_actions: + helps[item.dest] = item.help + + # commands which share an existing parser are an alias, + # don't duplicate docs + subsection_alias = {} + subsection_alias_names = set() + for name, subaction in action._name_parser_map.items(): + if subaction not in subsection_alias: + subsection_alias[subaction] = [] + else: + subsection_alias[subaction].append(name) + subsection_alias_names.add(name) + + for name, subaction in action._name_parser_map.items(): + if name in subsection_alias_names: + continue + subalias = subsection_alias[subaction] + subaction.prog = '%s %s' % (parser.prog, name) + subdata = { + 'name': name if not subalias else '%s (%s)' % (name, ', '.join(subalias)), + 'help': helps.get(name, ''), + 'usage': subaction.format_usage().strip(), + 'bare_usage': _format_usage_without_prefix(subaction), + } + parse_parser(subaction, subdata, **kwargs) + data.setdefault('children', []).append(subdata) + + show_defaults = True + if 'skip_default_values' in kwargs and kwargs['skip_default_values'] is True: + show_defaults = False show_defaults_const = show_defaults if 'skip_default_const_values' in kwargs and kwargs['skip_default_const_values'] is True: show_defaults_const = False - # argparse stores the different groups as a lint in parser._action_groups - # the first element of the list are the positional arguments, hence the - # parser._actions_groups[1:] + # argparse stores the different groups as a list in parser._action_groups + # the first element of the list holds the positional arguments, the + # second the option arguments not in groups, and subsequent elements + # argument groups with positional and optional parameters action_groups = [] - for action_group in parser._action_groups[1:]: + for action_group in parser._action_groups: options_list = [] for action in action_group._group_actions: if isinstance(action, _HelpAction): @@ -141,15 +121,26 @@ def parse_parser(parser, data=None, **kwargs): except: pass + # Options have the option_strings set, positional arguments don't + name = action.option_strings + if name == []: + if action.metavar is None: + name = [action.dest] + else: + name = [action.metavar] + # Skip lines for subcommands + if name == ['==SUPPRESS==']: + continue + if isinstance(action, _StoreConstAction): option = { - 'name': action.option_strings, + 'name': name, 'default': action.default if show_defaults_const else '==SUPPRESS==', 'help': helpStr } else: option = { - 'name': action.option_strings, + 'name': name, 'default': action.default if show_defaults else '==SUPPRESS==', 'help': helpStr } @@ -161,6 +152,10 @@ def parse_parser(parser, data=None, **kwargs): if len(options_list) == 0: continue + # Upper case "Positional Arguments" and "Optional Arguments" titles + if action_group.title in ['positional arguments', 'optional arguments']: + action_group.title = action_group.title.title() + group = {'title': action_group.title, 'description': action_group.description, 'options': options_list} diff --git a/test/test_parser.py b/test/test_parser.py index 58bfae5..7955ae4 100755 --- a/test/test_parser.py +++ b/test/test_parser.py @@ -43,12 +43,12 @@ def test_parse_arg_choices(): data = parse_parser(parser) - assert data['args'] == [ + assert data['action_groups'][0]['options'] == [ { - 'name': 'move', + 'name': ['move'], 'help': '', 'choices': ['rock', 'paper', 'scissors'], - 'metavar': None + 'default': None } ] @@ -91,15 +91,15 @@ def test_parse_positional(): data = parse_parser(parser) - assert data['args'] == [ + assert data['action_groups'][0]['options'] == [ { - 'name': 'foo', + 'name': ['foo'], 'help': 'foo help', - 'metavar': None + 'default': False }, { - 'name': 'bar', + 'name': ['bar'], 'help': '', - 'metavar': None + 'default': False } ] @@ -115,15 +115,15 @@ def test_parse_description(): assert data['epilog'] == 'epilogged' - assert data['args'] == [ + assert data['action_groups'][0]['options'] == [ { - 'name': 'foo', + 'name': ['foo'], 'help': 'foo help', - 'metavar': None + 'default': False }, { - 'name': 'bar', + 'name': ['bar'], 'help': '', - 'metavar': None + 'default': False } ] @@ -141,15 +141,15 @@ def test_parse_nested(): data = parse_parser(parser) - assert data['args'] == [ + assert data['action_groups'][0]['options'] == [ { - 'name': 'foo', + 'name': ['foo'], 'help': 'foo help', - 'metavar': None + 'default': False }, { - 'name': 'bar', + 'name': ['bar'], 'help': '', - 'metavar': None + 'default': False } ] @@ -159,23 +159,27 @@ def test_parse_nested(): 'help': 'install help', 'usage': 'usage: py.test install [-h] [--upgrade] ref', 'bare_usage': 'py.test install [-h] [--upgrade] ref', - 'args': [ + 'action_groups': [ { - 'name': 'ref', - 'help': 'foo1 help', - 'metavar': None + 'title': 'Positional Arguments', + 'description': None, + 'options': [ + { + 'name': ['ref'], + 'help': 'foo1 help', + 'default': None + } + ] }, - ], - 'action_groups': [ { 'description': None, - 'title': 'optional arguments', + 'title': 'Optional Arguments', 'options': [ { 'name': ['--upgrade'], 'default': False, 'help': 'foo2 help' - }, + } ] } ] @@ -202,15 +206,15 @@ def test_parse_nested_traversal(): data3 = parser_navigate(data, 'level1 level2 level3') - assert data3['args'] == [ + assert data3['action_groups'][0]['options'] == [ { - 'name': 'foo', + 'name': ['foo'], 'help': 'foo help', - 'metavar': None + 'default': None }, { - 'name': 'bar', + 'name': ['bar'], 'help': '', - 'metavar': None + 'default': None } ] @@ -221,16 +225,21 @@ def test_parse_nested_traversal(): 'help': '', 'usage': 'usage: py.test level1 level2 level3 [-h] foo bar', 'bare_usage': 'py.test level1 level2 level3 [-h] foo bar', - 'args': [ - { - 'name': 'foo', - 'help': 'foo help', - 'metavar': None - }, + 'action_groups': [ { - 'name': 'bar', - 'help': '', - 'metavar': None + 'title': 'Positional Arguments', + 'description': None, + 'options': [ + { + 'default': None, + 'name': ['foo'], + 'help': 'foo help' + }, { + 'name': ['bar'], + 'help': '', + 'default': None + } + ] } ] } @@ -247,11 +256,11 @@ def test_fill_in_default_prog(): parser.add_argument('bar', default='foo', help='%(prog)s (default: %(default)s)') data = parse_parser(parser) - assert data['args'] == [ + assert data['action_groups'][0]['options'] == [ { - 'metavar': None, - 'name': 'bar', - 'help': 'test_fill_in_default_prog (default: foo)' + 'default': '"foo"', + 'name': ['bar'], + 'help': 'test_fill_in_default_prog (default: "foo")' } ] @@ -289,7 +298,7 @@ def test_parse_groups(): 'options': [ {'default': False, 'help': 'foo help', 'name': ['--foo']}, {'default': False, 'help': '', 'name': ['--bar']}], - 'title': 'optional arguments'}, + 'title': 'Optional Arguments'}, { 'description': None, 'options': [ @@ -298,3 +307,40 @@ def test_parse_groups(): 'title': 'Group 1' } ] + + +def test_action_groups_with_subcommands(): + """ + This is a somewhat overly complicated example incorporating both action + groups (with optional AND positional arguments) and subcommands (again + with both optional and positional arguments) + """ + parser = argparse.ArgumentParser('foo') + subparsers = parser.add_subparsers() + parserA = subparsers.add_parser('A', help='A subparser') + parserA.add_argument('baz', type=int, help='An integer') + parserB = subparsers.add_parser('B', help='B subparser') + parserB.add_argument('--barg', choices='XYZ', help='A list of choices') + + parser.add_argument('--foo', help='foo help') + parser.add_argument('foo2', metavar='foo2 metavar', help='foo2 help') + grp1 = parser.add_argument_group('bar options') + grp1.add_argument('--bar', help='bar help') + grp1.add_argument('quux', help='quux help') + grp2 = parser.add_argument_group('bla options') + grp2.add_argument('--blah', help='blah help') + grp2.add_argument('sniggly', help='sniggly help') + + data = parse_parser(parser) + + assert data['action_groups'] == [ + {'options': [{'default': None, 'name': ['foo2 metavar'], 'help': 'foo2 help'}], 'description': None, 'title': 'Positional Arguments'}, + {'options': [{'default': None, 'name': ['--foo'], 'help': 'foo help'}], 'description': None, 'title': 'Optional Arguments'}, + {'options': [{'default': None, 'name': ['--bar'], 'help': 'bar help'}, {'default': None, 'name': ['quux'], 'help': 'quux help'}], 'description': None, 'title': 'bar options'}, + {'options': [{'default': None, 'name': ['--blah'], 'help': 'blah help'}, {'default': None, 'name': ['sniggly'], 'help': 'sniggly help'}], 'description': None, 'title': 'bla options'} + ] + + assert data['children'] == [ + {'usage': 'usage: foo A [-h] baz', 'action_groups': [{'options': [{'default': None, 'name': ['baz'], 'help': 'An integer'}], 'description': None, 'title': 'Positional Arguments'}], 'bare_usage': 'foo A [-h] baz', 'name': 'A', 'help': 'A subparser'}, + {'usage': 'usage: foo B [-h] [--barg {X,Y,Z}]', 'action_groups': [{'options': [{'default': None, 'choices': 'XYZ', 'name': ['--barg'], 'help': 'A list of choices'}], 'description': None, 'title': 'Optional Arguments'}], 'bare_usage': 'foo B [-h] [--barg {X,Y,Z}]', 'name': 'B', 'help': 'B subparser'} + ] From ceb93ea424e214ea97294dd8be96e6b32527fb9a Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Thu, 23 Feb 2017 00:38:50 +0100 Subject: [PATCH 3/7] Test python 3.6 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 973f70c..74f1659 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ python: - "3.3" - "3.4" - "3.5" + - "3.6" install: - pip install . - pip install flake8 From 116013709ebc84ecd03ad8e71b6208e8c393f1e8 Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Thu, 23 Feb 2017 00:40:15 +0100 Subject: [PATCH 4/7] PEP8 --- test/test_parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_parser.py b/test/test_parser.py index 7955ae4..53c24b8 100755 --- a/test/test_parser.py +++ b/test/test_parser.py @@ -149,7 +149,7 @@ def test_parse_nested(): }, { 'name': ['bar'], 'help': '', - 'default': False + 'default': False } ] @@ -312,7 +312,7 @@ def test_parse_groups(): def test_action_groups_with_subcommands(): """ This is a somewhat overly complicated example incorporating both action - groups (with optional AND positional arguments) and subcommands (again + groups (with optional AND positional arguments) and subcommands (again with both optional and positional arguments) """ parser = argparse.ArgumentParser('foo') From 4836eb76cb7f21cf72da36719fe6572c6dde3155 Mon Sep 17 00:00:00 2001 From: dpryan79 Date: Thu, 23 Feb 2017 13:45:08 +0100 Subject: [PATCH 5/7] Rename Positional Arguments to Required Arguments for consistency --- .gitignore | 1 + sphinxarg/parser.py | 6 ++++-- test/test_parser.py | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index c4ae962..681d0f1 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,4 @@ nosetests.xml .env/ docs/_build/ +.cache/ diff --git a/sphinxarg/parser.py b/sphinxarg/parser.py index 55622dc..a82cbc2 100644 --- a/sphinxarg/parser.py +++ b/sphinxarg/parser.py @@ -153,8 +153,10 @@ def parse_parser(parser, data=None, **kwargs): continue # Upper case "Positional Arguments" and "Optional Arguments" titles - if action_group.title in ['positional arguments', 'optional arguments']: - action_group.title = action_group.title.title() + if action_group.title == 'optional arguments': + action_group.title = 'Optional Arguments' + if action_group.title == 'positional arguments': + action_group.title = 'Required Arguments' group = {'title': action_group.title, 'description': action_group.description, diff --git a/test/test_parser.py b/test/test_parser.py index 53c24b8..ef2af25 100755 --- a/test/test_parser.py +++ b/test/test_parser.py @@ -161,7 +161,7 @@ def test_parse_nested(): 'bare_usage': 'py.test install [-h] [--upgrade] ref', 'action_groups': [ { - 'title': 'Positional Arguments', + 'title': 'Required Arguments', 'description': None, 'options': [ { @@ -227,7 +227,7 @@ def test_parse_nested_traversal(): 'bare_usage': 'py.test level1 level2 level3 [-h] foo bar', 'action_groups': [ { - 'title': 'Positional Arguments', + 'title': 'Required Arguments', 'description': None, 'options': [ { @@ -334,13 +334,13 @@ def test_action_groups_with_subcommands(): data = parse_parser(parser) assert data['action_groups'] == [ - {'options': [{'default': None, 'name': ['foo2 metavar'], 'help': 'foo2 help'}], 'description': None, 'title': 'Positional Arguments'}, + {'options': [{'default': None, 'name': ['foo2 metavar'], 'help': 'foo2 help'}], 'description': None, 'title': 'Required Arguments'}, {'options': [{'default': None, 'name': ['--foo'], 'help': 'foo help'}], 'description': None, 'title': 'Optional Arguments'}, {'options': [{'default': None, 'name': ['--bar'], 'help': 'bar help'}, {'default': None, 'name': ['quux'], 'help': 'quux help'}], 'description': None, 'title': 'bar options'}, {'options': [{'default': None, 'name': ['--blah'], 'help': 'blah help'}, {'default': None, 'name': ['sniggly'], 'help': 'sniggly help'}], 'description': None, 'title': 'bla options'} ] assert data['children'] == [ - {'usage': 'usage: foo A [-h] baz', 'action_groups': [{'options': [{'default': None, 'name': ['baz'], 'help': 'An integer'}], 'description': None, 'title': 'Positional Arguments'}], 'bare_usage': 'foo A [-h] baz', 'name': 'A', 'help': 'A subparser'}, + {'usage': 'usage: foo A [-h] baz', 'action_groups': [{'options': [{'default': None, 'name': ['baz'], 'help': 'An integer'}], 'description': None, 'title': 'Required Arguments'}], 'bare_usage': 'foo A [-h] baz', 'name': 'A', 'help': 'A subparser'}, {'usage': 'usage: foo B [-h] [--barg {X,Y,Z}]', 'action_groups': [{'options': [{'default': None, 'choices': 'XYZ', 'name': ['--barg'], 'help': 'A list of choices'}], 'description': None, 'title': 'Optional Arguments'}], 'bare_usage': 'foo B [-h] [--barg {X,Y,Z}]', 'name': 'B', 'help': 'B subparser'} ] From 93296fb308528cdc43b40a5d56ce5b41c7cdc44c Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Fri, 24 Feb 2017 13:58:31 +0100 Subject: [PATCH 6/7] Bump version to 0.1.17 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d693bfd..ec00838 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ setup( name='sphinx-argparse', - version='0.1.16', + version='0.1.17', packages=[ 'sphinxarg', ], From e7672e31c7d8387061f770e2cd1a0a36f4071d43 Mon Sep 17 00:00:00 2001 From: Devon Ryan Date: Fri, 24 Feb 2017 14:01:44 +0100 Subject: [PATCH 7/7] Update README.md --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index fcbc073..6f71dff 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,11 @@ http://sphinx-argparse.readthedocs.org/en/latest/ Changelog & contributors ======================== +0.1.17 +------ + +- Fixed handling of argument groups (this was bug #49). Thanks to @croth1 for reporting this bug. Note that now position arguments (also known as required arguments) within argument groups are now also handled correctly. + 0.1.16 ------