Skip to content

Commit 1db9f56

Browse files
gh-142346: Fix usage formatting for mutually exclusive groups in argparse (GH-142381)
Support groups preceded by positional arguments or followed or intermixed with other optional arguments. Support empty groups.
1 parent d6d850d commit 1db9f56

File tree

3 files changed

+131
-146
lines changed

3 files changed

+131
-146
lines changed

Lib/argparse.py

Lines changed: 92 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -334,31 +334,15 @@ def _format_usage(self, usage, actions, groups, prefix):
334334
elif usage is None:
335335
prog = '%(prog)s' % dict(prog=self._prog)
336336

337-
# split optionals from positionals
338-
optionals = []
339-
positionals = []
340-
for action in actions:
341-
if action.option_strings:
342-
optionals.append(action)
343-
else:
344-
positionals.append(action)
345-
337+
parts, pos_start = self._get_actions_usage_parts(actions, groups)
346338
# build full usage string
347-
format = self._format_actions_usage
348-
action_usage = format(optionals + positionals, groups)
349-
usage = ' '.join([s for s in [prog, action_usage] if s])
339+
usage = ' '.join(filter(None, [prog, *parts]))
350340

351341
# wrap the usage parts if it's too long
352342
text_width = self._width - self._current_indent
353343
if len(prefix) + len(self._decolor(usage)) > text_width:
354344

355345
# break usage into wrappable parts
356-
# keep optionals and positionals together to preserve
357-
# mutually exclusive group formatting (gh-75949)
358-
all_actions = optionals + positionals
359-
parts, pos_start = self._get_actions_usage_parts_with_split(
360-
all_actions, groups, len(optionals)
361-
)
362346
opt_parts = parts[:pos_start]
363347
pos_parts = parts[pos_start:]
364348

@@ -417,125 +401,114 @@ def get_lines(parts, indent, prefix=None):
417401
# prefix with 'usage:'
418402
return f'{t.usage}{prefix}{t.reset}{usage}\n\n'
419403

420-
def _format_actions_usage(self, actions, groups):
421-
return ' '.join(self._get_actions_usage_parts(actions, groups))
422-
423404
def _is_long_option(self, string):
424405
return len(string) > 2
425406

426407
def _get_actions_usage_parts(self, actions, groups):
427-
parts, _ = self._get_actions_usage_parts_with_split(actions, groups)
428-
return parts
429-
430-
def _get_actions_usage_parts_with_split(self, actions, groups, opt_count=None):
431408
"""Get usage parts with split index for optionals/positionals.
432409
433410
Returns (parts, pos_start) where pos_start is the index in parts
434-
where positionals begin. When opt_count is None, pos_start is None.
411+
where positionals begin.
435412
This preserves mutually exclusive group formatting across the
436413
optionals/positionals boundary (gh-75949).
437414
"""
438-
# find group indices and identify actions in groups
439-
group_actions = set()
440-
inserts = {}
415+
actions = [action for action in actions if action.help is not SUPPRESS]
416+
# group actions by mutually exclusive groups
417+
action_groups = dict.fromkeys(actions)
441418
for group in groups:
442-
if not group._group_actions:
443-
raise ValueError(f'empty group {group}')
444-
445-
if all(action.help is SUPPRESS for action in group._group_actions):
446-
continue
447-
448-
try:
449-
start = min(actions.index(item) for item in group._group_actions)
450-
except ValueError:
451-
continue
452-
else:
453-
end = start + len(group._group_actions)
454-
if set(actions[start:end]) == set(group._group_actions):
455-
group_actions.update(group._group_actions)
456-
inserts[start, end] = group
419+
for action in group._group_actions:
420+
if action in action_groups:
421+
action_groups[action] = group
422+
# positional arguments keep their position
423+
positionals = []
424+
for action in actions:
425+
if not action.option_strings:
426+
group = action_groups.pop(action)
427+
if group:
428+
group_actions = [
429+
action2 for action2 in group._group_actions
430+
if action2.option_strings and
431+
action_groups.pop(action2, None)
432+
] + [action]
433+
positionals.append((group.required, group_actions))
434+
else:
435+
positionals.append((None, [action]))
436+
# the remaining optional arguments are sorted by the position of
437+
# the first option in the group
438+
optionals = []
439+
for action in actions:
440+
if action.option_strings and action in action_groups:
441+
group = action_groups.pop(action)
442+
if group:
443+
group_actions = [action] + [
444+
action2 for action2 in group._group_actions
445+
if action2.option_strings and
446+
action_groups.pop(action2, None)
447+
]
448+
optionals.append((group.required, group_actions))
449+
else:
450+
optionals.append((None, [action]))
457451

458452
# collect all actions format strings
459453
parts = []
460454
t = self._theme
461-
for action in actions:
462-
463-
# suppressed arguments are marked with None
464-
if action.help is SUPPRESS:
465-
part = None
466-
467-
# produce all arg strings
468-
elif not action.option_strings:
469-
default = self._get_default_metavar_for_positional(action)
470-
part = self._format_args(action, default)
471-
# if it's in a group, strip the outer []
472-
if action in group_actions:
473-
if part[0] == '[' and part[-1] == ']':
474-
part = part[1:-1]
475-
part = t.summary_action + part + t.reset
476-
477-
# produce the first way to invoke the option in brackets
478-
else:
479-
option_string = action.option_strings[0]
480-
if self._is_long_option(option_string):
481-
option_color = t.summary_long_option
455+
pos_start = None
456+
for i, (required, group) in enumerate(optionals + positionals):
457+
start = len(parts)
458+
if i == len(optionals):
459+
pos_start = start
460+
in_group = len(group) > 1
461+
for action in group:
462+
# produce all arg strings
463+
if not action.option_strings:
464+
default = self._get_default_metavar_for_positional(action)
465+
part = self._format_args(action, default)
466+
# if it's in a group, strip the outer []
467+
if in_group:
468+
if part[0] == '[' and part[-1] == ']':
469+
part = part[1:-1]
470+
part = t.summary_action + part + t.reset
471+
472+
# produce the first way to invoke the option in brackets
482473
else:
483-
option_color = t.summary_short_option
484-
485-
# if the Optional doesn't take a value, format is:
486-
# -s or --long
487-
if action.nargs == 0:
488-
part = action.format_usage()
489-
part = f"{option_color}{part}{t.reset}"
490-
491-
# if the Optional takes a value, format is:
492-
# -s ARGS or --long ARGS
493-
else:
494-
default = self._get_default_metavar_for_optional(action)
495-
args_string = self._format_args(action, default)
496-
part = (
497-
f"{option_color}{option_string} "
498-
f"{t.summary_label}{args_string}{t.reset}"
499-
)
500-
501-
# make it look optional if it's not required or in a group
502-
if not action.required and action not in group_actions:
503-
part = '[%s]' % part
474+
option_string = action.option_strings[0]
475+
if self._is_long_option(option_string):
476+
option_color = t.summary_long_option
477+
else:
478+
option_color = t.summary_short_option
504479

505-
# add the action string to the list
506-
parts.append(part)
480+
# if the Optional doesn't take a value, format is:
481+
# -s or --long
482+
if action.nargs == 0:
483+
part = action.format_usage()
484+
part = f"{option_color}{part}{t.reset}"
507485

508-
# group mutually exclusive actions
509-
inserted_separators_indices = set()
510-
for start, end in sorted(inserts, reverse=True):
511-
group = inserts[start, end]
512-
group_parts = [item for item in parts[start:end] if item is not None]
513-
group_size = len(group_parts)
514-
if group.required:
515-
open, close = "()" if group_size > 1 else ("", "")
516-
else:
517-
open, close = "[]"
518-
group_parts[0] = open + group_parts[0]
519-
group_parts[-1] = group_parts[-1] + close
520-
for i, part in enumerate(group_parts[:-1], start=start):
521-
# insert a separator if not already done in a nested group
522-
if i not in inserted_separators_indices:
523-
parts[i] = part + ' |'
524-
inserted_separators_indices.add(i)
525-
parts[start + group_size - 1] = group_parts[-1]
526-
for i in range(start + group_size, end):
527-
parts[i] = None
528-
529-
# if opt_count is provided, calculate where positionals start in
530-
# the final parts list (for wrapping onto separate lines).
531-
# Count before filtering None entries since indices shift after.
532-
if opt_count is not None:
533-
pos_start = sum(1 for p in parts[:opt_count] if p is not None)
534-
else:
535-
pos_start = None
536-
537-
# return the usage parts and split point (gh-75949)
538-
return [item for item in parts if item is not None], pos_start
486+
# if the Optional takes a value, format is:
487+
# -s ARGS or --long ARGS
488+
else:
489+
default = self._get_default_metavar_for_optional(action)
490+
args_string = self._format_args(action, default)
491+
part = (
492+
f"{option_color}{option_string} "
493+
f"{t.summary_label}{args_string}{t.reset}"
494+
)
495+
496+
# make it look optional if it's not required or in a group
497+
if not (action.required or required or in_group):
498+
part = '[%s]' % part
499+
500+
# add the action string to the list
501+
parts.append(part)
502+
503+
if in_group:
504+
parts[start] = ('(' if required else '[') + parts[start]
505+
for i in range(start, len(parts) - 1):
506+
parts[i] += ' |'
507+
parts[-1] += ')' if required else ']'
508+
509+
if pos_start is None:
510+
pos_start = len(parts)
511+
return parts, pos_start
539512

540513
def _format_text(self, text):
541514
if '%(prog)' in text:

Lib/test/test_argparse.py

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3398,12 +3398,11 @@ def test_help_subparser_all_mutually_exclusive_group_members_suppressed(self):
33983398
'''
33993399
self.assertEqual(cmd_foo.format_help(), textwrap.dedent(expected))
34003400

3401-
def test_empty_group(self):
3401+
def test_usage_empty_group(self):
34023402
# See issue 26952
3403-
parser = argparse.ArgumentParser()
3403+
parser = ErrorRaisingArgumentParser(prog='PROG')
34043404
group = parser.add_mutually_exclusive_group()
3405-
with self.assertRaises(ValueError):
3406-
parser.parse_args(['-h'])
3405+
self.assertEqual(parser.format_usage(), 'usage: PROG [-h]\n')
34073406

34083407
def test_nested_mutex_groups(self):
34093408
parser = argparse.ArgumentParser(prog='PROG')
@@ -3671,25 +3670,29 @@ def get_parser(self, required):
36713670
group.add_argument('-b', action='store_true', help='b help')
36723671
parser.add_argument('-y', action='store_true', help='y help')
36733672
group.add_argument('-c', action='store_true', help='c help')
3673+
parser.add_argument('-z', action='store_true', help='z help')
36743674
return parser
36753675

36763676
failures = ['-a -b', '-b -c', '-a -c', '-a -b -c']
36773677
successes = [
3678-
('-a', NS(a=True, b=False, c=False, x=False, y=False)),
3679-
('-b', NS(a=False, b=True, c=False, x=False, y=False)),
3680-
('-c', NS(a=False, b=False, c=True, x=False, y=False)),
3681-
('-a -x', NS(a=True, b=False, c=False, x=True, y=False)),
3682-
('-y -b', NS(a=False, b=True, c=False, x=False, y=True)),
3683-
('-x -y -c', NS(a=False, b=False, c=True, x=True, y=True)),
3678+
('-a', NS(a=True, b=False, c=False, x=False, y=False, z=False)),
3679+
('-b', NS(a=False, b=True, c=False, x=False, y=False, z=False)),
3680+
('-c', NS(a=False, b=False, c=True, x=False, y=False, z=False)),
3681+
('-a -x', NS(a=True, b=False, c=False, x=True, y=False, z=False)),
3682+
('-y -b', NS(a=False, b=True, c=False, x=False, y=True, z=False)),
3683+
('-x -y -c', NS(a=False, b=False, c=True, x=True, y=True, z=False)),
36843684
]
36853685
successes_when_not_required = [
3686-
('', NS(a=False, b=False, c=False, x=False, y=False)),
3687-
('-x', NS(a=False, b=False, c=False, x=True, y=False)),
3688-
('-y', NS(a=False, b=False, c=False, x=False, y=True)),
3686+
('', NS(a=False, b=False, c=False, x=False, y=False, z=False)),
3687+
('-x', NS(a=False, b=False, c=False, x=True, y=False, z=False)),
3688+
('-y', NS(a=False, b=False, c=False, x=False, y=True, z=False)),
36893689
]
36903690

3691-
usage_when_required = usage_when_not_required = '''\
3692-
usage: PROG [-h] [-x] [-a] [-b] [-y] [-c]
3691+
usage_when_not_required = '''\
3692+
usage: PROG [-h] [-x] [-a | -b | -c] [-y] [-z]
3693+
'''
3694+
usage_when_required = '''\
3695+
usage: PROG [-h] [-x] (-a | -b | -c) [-y] [-z]
36933696
'''
36943697
help = '''\
36953698
@@ -3700,6 +3703,7 @@ def get_parser(self, required):
37003703
-b b help
37013704
-y y help
37023705
-c c help
3706+
-z z help
37033707
'''
37043708

37053709

@@ -3753,23 +3757,27 @@ def get_parser(self, required):
37533757
group.add_argument('a', nargs='?', help='a help')
37543758
group.add_argument('-b', action='store_true', help='b help')
37553759
group.add_argument('-c', action='store_true', help='c help')
3760+
parser.add_argument('-z', action='store_true', help='z help')
37563761
return parser
37573762

37583763
failures = ['X A -b', '-b -c', '-c X A']
37593764
successes = [
3760-
('X A', NS(a='A', b=False, c=False, x='X', y=False)),
3761-
('X -b', NS(a=None, b=True, c=False, x='X', y=False)),
3762-
('X -c', NS(a=None, b=False, c=True, x='X', y=False)),
3763-
('X A -y', NS(a='A', b=False, c=False, x='X', y=True)),
3764-
('X -y -b', NS(a=None, b=True, c=False, x='X', y=True)),
3765+
('X A', NS(a='A', b=False, c=False, x='X', y=False, z=False)),
3766+
('X -b', NS(a=None, b=True, c=False, x='X', y=False, z=False)),
3767+
('X -c', NS(a=None, b=False, c=True, x='X', y=False, z=False)),
3768+
('X A -y', NS(a='A', b=False, c=False, x='X', y=True, z=False)),
3769+
('X -y -b', NS(a=None, b=True, c=False, x='X', y=True, z=False)),
37653770
]
37663771
successes_when_not_required = [
3767-
('X', NS(a=None, b=False, c=False, x='X', y=False)),
3768-
('X -y', NS(a=None, b=False, c=False, x='X', y=True)),
3772+
('X', NS(a=None, b=False, c=False, x='X', y=False, z=False)),
3773+
('X -y', NS(a=None, b=False, c=False, x='X', y=True, z=False)),
37693774
]
37703775

3771-
usage_when_required = usage_when_not_required = '''\
3772-
usage: PROG [-h] [-y] [-b] [-c] x [a]
3776+
usage_when_not_required = '''\
3777+
usage: PROG [-h] [-y] [-z] x [-b | -c | a]
3778+
'''
3779+
usage_when_required = '''\
3780+
usage: PROG [-h] [-y] [-z] x (-b | -c | a)
37733781
'''
37743782
help = '''\
37753783
@@ -3782,6 +3790,7 @@ def get_parser(self, required):
37823790
-y y help
37833791
-b b help
37843792
-c c help
3793+
-z z help
37853794
'''
37863795

37873796

@@ -4989,9 +4998,9 @@ def test_mutex_groups_with_mixed_optionals_positionals_wrap(self):
49894998
g.add_argument('positional', nargs='?')
49904999

49915000
usage = textwrap.dedent('''\
4992-
usage: PROG [-h] [-v | -q | -x [EXTRA_LONG_OPTION_NAME] |
4993-
-y [YET_ANOTHER_LONG_OPTION] |
4994-
positional]
5001+
usage: PROG [-h]
5002+
[-v | -q | -x [EXTRA_LONG_OPTION_NAME] |
5003+
-y [YET_ANOTHER_LONG_OPTION] | positional]
49955004
''')
49965005
self.assertEqual(parser.format_usage(), usage)
49975006

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix usage formatting for mutually exclusive groups in :mod:`argparse`
2+
when they are preceded by positional arguments or followed or intermixed
3+
with other optional arguments.

0 commit comments

Comments
 (0)