Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 92 additions & 119 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,31 +334,15 @@ def _format_usage(self, usage, actions, groups, prefix):
elif usage is None:
prog = '%(prog)s' % dict(prog=self._prog)

# split optionals from positionals
optionals = []
positionals = []
for action in actions:
if action.option_strings:
optionals.append(action)
else:
positionals.append(action)

parts, pos_start = self._get_actions_usage_parts(actions, groups)
# build full usage string
format = self._format_actions_usage
action_usage = format(optionals + positionals, groups)
usage = ' '.join([s for s in [prog, action_usage] if s])
usage = ' '.join(filter(None, [prog, *parts]))

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

# break usage into wrappable parts
# keep optionals and positionals together to preserve
# mutually exclusive group formatting (gh-75949)
all_actions = optionals + positionals
parts, pos_start = self._get_actions_usage_parts_with_split(
all_actions, groups, len(optionals)
)
opt_parts = parts[:pos_start]
pos_parts = parts[pos_start:]

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

def _format_actions_usage(self, actions, groups):
return ' '.join(self._get_actions_usage_parts(actions, groups))

def _is_long_option(self, string):
return len(string) > 2

def _get_actions_usage_parts(self, actions, groups):
parts, _ = self._get_actions_usage_parts_with_split(actions, groups)
return parts

def _get_actions_usage_parts_with_split(self, actions, groups, opt_count=None):
"""Get usage parts with split index for optionals/positionals.

Returns (parts, pos_start) where pos_start is the index in parts
where positionals begin. When opt_count is None, pos_start is None.
where positionals begin.
This preserves mutually exclusive group formatting across the
optionals/positionals boundary (gh-75949).
"""
# find group indices and identify actions in groups
group_actions = set()
inserts = {}
actions = [action for action in actions if action.help is not SUPPRESS]
# group actions by mutually exclusive groups
action_groups = dict.fromkeys(actions)
for group in groups:
if not group._group_actions:
raise ValueError(f'empty group {group}')

if all(action.help is SUPPRESS for action in group._group_actions):
continue

try:
start = min(actions.index(item) for item in group._group_actions)
except ValueError:
continue
else:
end = start + len(group._group_actions)
if set(actions[start:end]) == set(group._group_actions):
group_actions.update(group._group_actions)
inserts[start, end] = group
for action in group._group_actions:
if action in action_groups:
action_groups[action] = group
# positional arguments keep their position
positionals = []
for action in actions:
if not action.option_strings:
group = action_groups.pop(action)
if group:
group_actions = [
action2 for action2 in group._group_actions
if action2.option_strings and
action_groups.pop(action2, None)
] + [action]
positionals.append((group.required, group_actions))
else:
positionals.append((None, [action]))
# the remaining optional arguments are sorted by the position of
# the first option in the group
optionals = []
for action in actions:
if action.option_strings and action in action_groups:
group = action_groups.pop(action)
if group:
group_actions = [action] + [
action2 for action2 in group._group_actions
if action2.option_strings and
action_groups.pop(action2, None)
]
optionals.append((group.required, group_actions))
else:
optionals.append((None, [action]))

# collect all actions format strings
parts = []
t = self._theme
for action in actions:

# suppressed arguments are marked with None
if action.help is SUPPRESS:
part = None

# produce all arg strings
elif not action.option_strings:
default = self._get_default_metavar_for_positional(action)
part = self._format_args(action, default)
# if it's in a group, strip the outer []
if action in group_actions:
if part[0] == '[' and part[-1] == ']':
part = part[1:-1]
part = t.summary_action + part + t.reset

# produce the first way to invoke the option in brackets
else:
option_string = action.option_strings[0]
if self._is_long_option(option_string):
option_color = t.summary_long_option
pos_start = None
for i, (required, group) in enumerate(optionals + positionals):
start = len(parts)
if i == len(optionals):
pos_start = start
in_group = len(group) > 1
for action in group:
# produce all arg strings
if not action.option_strings:
default = self._get_default_metavar_for_positional(action)
part = self._format_args(action, default)
# if it's in a group, strip the outer []
if in_group:
if part[0] == '[' and part[-1] == ']':
part = part[1:-1]
part = t.summary_action + part + t.reset

# produce the first way to invoke the option in brackets
else:
option_color = t.summary_short_option

# if the Optional doesn't take a value, format is:
# -s or --long
if action.nargs == 0:
part = action.format_usage()
part = f"{option_color}{part}{t.reset}"

# if the Optional takes a value, format is:
# -s ARGS or --long ARGS
else:
default = self._get_default_metavar_for_optional(action)
args_string = self._format_args(action, default)
part = (
f"{option_color}{option_string} "
f"{t.summary_label}{args_string}{t.reset}"
)

# make it look optional if it's not required or in a group
if not action.required and action not in group_actions:
part = '[%s]' % part
option_string = action.option_strings[0]
if self._is_long_option(option_string):
option_color = t.summary_long_option
else:
option_color = t.summary_short_option

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

# group mutually exclusive actions
inserted_separators_indices = set()
for start, end in sorted(inserts, reverse=True):
group = inserts[start, end]
group_parts = [item for item in parts[start:end] if item is not None]
group_size = len(group_parts)
if group.required:
open, close = "()" if group_size > 1 else ("", "")
else:
open, close = "[]"
group_parts[0] = open + group_parts[0]
group_parts[-1] = group_parts[-1] + close
for i, part in enumerate(group_parts[:-1], start=start):
# insert a separator if not already done in a nested group
if i not in inserted_separators_indices:
parts[i] = part + ' |'
inserted_separators_indices.add(i)
parts[start + group_size - 1] = group_parts[-1]
for i in range(start + group_size, end):
parts[i] = None

# if opt_count is provided, calculate where positionals start in
# the final parts list (for wrapping onto separate lines).
# Count before filtering None entries since indices shift after.
if opt_count is not None:
pos_start = sum(1 for p in parts[:opt_count] if p is not None)
else:
pos_start = None

# return the usage parts and split point (gh-75949)
return [item for item in parts if item is not None], pos_start
# if the Optional takes a value, format is:
# -s ARGS or --long ARGS
else:
default = self._get_default_metavar_for_optional(action)
args_string = self._format_args(action, default)
part = (
f"{option_color}{option_string} "
f"{t.summary_label}{args_string}{t.reset}"
)

# make it look optional if it's not required or in a group
if not (action.required or required or in_group):
part = '[%s]' % part

# add the action string to the list
parts.append(part)

if in_group:
parts[start] = ('(' if required else '[') + parts[start]
for i in range(start, len(parts) - 1):
parts[i] += ' |'
parts[-1] += ')' if required else ']'

if pos_start is None:
pos_start = len(parts)
return parts, pos_start

def _format_text(self, text):
if '%(prog)' in text:
Expand Down
63 changes: 36 additions & 27 deletions Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -3398,12 +3398,11 @@ def test_help_subparser_all_mutually_exclusive_group_members_suppressed(self):
'''
self.assertEqual(cmd_foo.format_help(), textwrap.dedent(expected))

def test_empty_group(self):
def test_usage_empty_group(self):
# See issue 26952
parser = argparse.ArgumentParser()
parser = ErrorRaisingArgumentParser(prog='PROG')
group = parser.add_mutually_exclusive_group()
with self.assertRaises(ValueError):
parser.parse_args(['-h'])
self.assertEqual(parser.format_usage(), 'usage: PROG [-h]\n')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValueError was only raised to hide IndexError. Now this is not needed, empty groups are simply ignored (as well as groups with all suppressed arguments).


def test_nested_mutex_groups(self):
parser = argparse.ArgumentParser(prog='PROG')
Expand Down Expand Up @@ -3671,25 +3670,29 @@ def get_parser(self, required):
group.add_argument('-b', action='store_true', help='b help')
parser.add_argument('-y', action='store_true', help='y help')
group.add_argument('-c', action='store_true', help='c help')
parser.add_argument('-z', action='store_true', help='z help')
return parser

failures = ['-a -b', '-b -c', '-a -c', '-a -b -c']
successes = [
('-a', NS(a=True, b=False, c=False, x=False, y=False)),
('-b', NS(a=False, b=True, c=False, x=False, y=False)),
('-c', NS(a=False, b=False, c=True, x=False, y=False)),
('-a -x', NS(a=True, b=False, c=False, x=True, y=False)),
('-y -b', NS(a=False, b=True, c=False, x=False, y=True)),
('-x -y -c', NS(a=False, b=False, c=True, x=True, y=True)),
('-a', NS(a=True, b=False, c=False, x=False, y=False, z=False)),
('-b', NS(a=False, b=True, c=False, x=False, y=False, z=False)),
('-c', NS(a=False, b=False, c=True, x=False, y=False, z=False)),
('-a -x', NS(a=True, b=False, c=False, x=True, y=False, z=False)),
('-y -b', NS(a=False, b=True, c=False, x=False, y=True, z=False)),
('-x -y -c', NS(a=False, b=False, c=True, x=True, y=True, z=False)),
]
successes_when_not_required = [
('', NS(a=False, b=False, c=False, x=False, y=False)),
('-x', NS(a=False, b=False, c=False, x=True, y=False)),
('-y', NS(a=False, b=False, c=False, x=False, y=True)),
('', NS(a=False, b=False, c=False, x=False, y=False, z=False)),
('-x', NS(a=False, b=False, c=False, x=True, y=False, z=False)),
('-y', NS(a=False, b=False, c=False, x=False, y=True, z=False)),
Comment on lines +3678 to +3688
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these changes are not important. They are just the result of adding yet one argument -z (to increase coverage), no other changes.

]

usage_when_required = usage_when_not_required = '''\
usage: PROG [-h] [-x] [-a] [-b] [-y] [-c]
usage_when_not_required = '''\
usage: PROG [-h] [-x] [-a | -b | -c] [-y] [-z]
'''
usage_when_required = '''\
usage: PROG [-h] [-x] (-a | -b | -c) [-y] [-z]
Comment on lines +3691 to +3695
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old result was incorrect (group was not formatted).

'''
help = '''\

Expand All @@ -3700,6 +3703,7 @@ def get_parser(self, required):
-b b help
-y y help
-c c help
-z z help
'''


Expand Down Expand Up @@ -3753,23 +3757,27 @@ def get_parser(self, required):
group.add_argument('a', nargs='?', help='a help')
group.add_argument('-b', action='store_true', help='b help')
group.add_argument('-c', action='store_true', help='c help')
parser.add_argument('-z', action='store_true', help='z help')
return parser

failures = ['X A -b', '-b -c', '-c X A']
successes = [
('X A', NS(a='A', b=False, c=False, x='X', y=False)),
('X -b', NS(a=None, b=True, c=False, x='X', y=False)),
('X -c', NS(a=None, b=False, c=True, x='X', y=False)),
('X A -y', NS(a='A', b=False, c=False, x='X', y=True)),
('X -y -b', NS(a=None, b=True, c=False, x='X', y=True)),
('X A', NS(a='A', b=False, c=False, x='X', y=False, z=False)),
('X -b', NS(a=None, b=True, c=False, x='X', y=False, z=False)),
('X -c', NS(a=None, b=False, c=True, x='X', y=False, z=False)),
('X A -y', NS(a='A', b=False, c=False, x='X', y=True, z=False)),
('X -y -b', NS(a=None, b=True, c=False, x='X', y=True, z=False)),
Comment on lines +3765 to +3769
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be ignored as above.

]
successes_when_not_required = [
('X', NS(a=None, b=False, c=False, x='X', y=False)),
('X -y', NS(a=None, b=False, c=False, x='X', y=True)),
('X', NS(a=None, b=False, c=False, x='X', y=False, z=False)),
('X -y', NS(a=None, b=False, c=False, x='X', y=True, z=False)),
]

usage_when_required = usage_when_not_required = '''\
usage: PROG [-h] [-y] [-b] [-c] x [a]
usage_when_not_required = '''\
usage: PROG [-h] [-y] [-z] x [-b | -c | a]
'''
usage_when_required = '''\
usage: PROG [-h] [-y] [-z] x (-b | -c | a)
Comment on lines +3776 to +3780
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yest one incorrect result in old tests.

'''
help = '''\

Expand All @@ -3782,6 +3790,7 @@ def get_parser(self, required):
-y y help
-b b help
-c c help
-z z help
'''


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

usage = textwrap.dedent('''\
usage: PROG [-h] [-v | -q | -x [EXTRA_LONG_OPTION_NAME] |
-y [YET_ANOTHER_LONG_OPTION] |
positional]
usage: PROG [-h]
[-v | -q | -x [EXTRA_LONG_OPTION_NAME] |
-y [YET_ANOTHER_LONG_OPTION] | positional]
Comment on lines +5001 to +5003
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line break is now forced not before the first positional argument, but before the first group containing a positional argument.

''')
self.assertEqual(parser.format_usage(), usage)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix usage formatting for mutually exclusive groups in :mod:`argparse`
when they are preceded by positional arguments or followed or intermixed
with other optional arguments.
Loading