From 34884b2a2c6513436af854f6eb6beb663c4d05ab Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 19 Sep 2024 10:36:23 +0300 Subject: [PATCH 1/2] gh-81691: Fix handling of multiple "--" (double dashes) in argparse Only the first one has now been removed, all subsequent ones are now taken literally. --- Lib/argparse.py | 15 ++--- Lib/test/test_argparse.py | 60 ++++++++++++++++++- ...4-09-19-10-36-18.gh-issue-81691.Hyhp_U.rst | 3 + 3 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-09-19-10-36-18.gh-issue-81691.Hyhp_U.rst diff --git a/Lib/argparse.py b/Lib/argparse.py index a88a8c65c40a1d..2d899551255bcf 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1916,12 +1916,14 @@ def _parse_known_args(self, arg_strings, namespace): # which has an 'O' if there is an option at an index, # an 'A' if there is an argument, or a '-' if there is a '--' option_string_indices = {} + double_dash_index = len(arg_strings) arg_string_pattern_parts = [] arg_strings_iter = iter(arg_strings) for i, arg_string in enumerate(arg_strings_iter): # all args after -- are non-options if arg_string == '--': + double_dash_index = i arg_string_pattern_parts.append('-') for arg_string in arg_strings_iter: arg_string_pattern_parts.append('A') @@ -2070,6 +2072,12 @@ def consume_positionals(start_index): # and add the Positional and its args to the list for action, arg_count in zip(positionals, arg_counts): args = arg_strings[start_index: start_index + arg_count] + # Strip out the first '--' if it is not in PARSER or REMAINDER arg. + if (arg_count + and start_index <= double_dash_index < start_index + arg_count + and action.nargs not in [PARSER, REMAINDER]): + assert args.index('--') == double_dash_index - start_index + args.remove('--') start_index += arg_count if args and action.deprecated and action.dest not in warned: self._warning(_("argument '%(argument_name)s' is deprecated") % @@ -2470,13 +2478,6 @@ def parse_known_intermixed_args(self, args=None, namespace=None): # Value conversion methods # ======================== def _get_values(self, action, arg_strings): - # for everything but PARSER, REMAINDER args, strip out first '--' - if not action.option_strings and action.nargs not in [PARSER, REMAINDER]: - try: - arg_strings.remove('--') - except ValueError: - pass - # optional argument produces a default when not present if not arg_strings and action.nargs == OPTIONAL: if action.option_strings: diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py index 584462b741ee99..3192c6f73c2836 100644 --- a/Lib/test/test_argparse.py +++ b/Lib/test/test_argparse.py @@ -5721,7 +5721,30 @@ def test_zero_or_more_optional(self): self.assertEqual(NS(x=[]), args) def test_double_dash(self): - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(exit_on_error=False) + parser.add_argument('-f', '--foo') + parser.add_argument('bar', nargs='*') + + args = parser.parse_args(['--foo=--']) + self.assertEqual(NS(foo='--', bar=[]), args) + self.assertRaisesRegex(argparse.ArgumentError, + 'argument -f/--foo: expected one argument', + parser.parse_args, ['--foo', '--']) + args = parser.parse_args(['-f--']) + self.assertEqual(NS(foo='--', bar=[]), args) + self.assertRaisesRegex(argparse.ArgumentError, + 'argument -f/--foo: expected one argument', + parser.parse_args, ['-f', '--']) + args = parser.parse_args(['--foo', 'a', '--', 'b', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', 'b', '--foo', 'c']) + self.assertEqual(NS(foo='c', bar=['a', 'b']), args) + args = parser.parse_args(['a', '--', 'b', '--foo', 'c']) + self.assertEqual(NS(foo=None, bar=['a', 'b', '--foo', 'c']), args) + args = parser.parse_args(['a', '--', 'b', '--', 'c', '--foo', 'd']) + self.assertEqual(NS(foo=None, bar=['a', 'b', '--', 'c', '--foo', 'd']), args) + + parser = argparse.ArgumentParser(exit_on_error=False) parser.add_argument('-f', '--foo', nargs='*') parser.add_argument('bar', nargs='*') @@ -5735,6 +5758,41 @@ def test_double_dash(self): self.assertEqual(NS(foo=[], bar=[]), args) args = parser.parse_args(['--foo', 'a', 'b', '--', 'c', 'd']) self.assertEqual(NS(foo=['a', 'b'], bar=['c', 'd']), args) + args = parser.parse_args(['a', 'b', '--foo', 'c', 'd']) + self.assertEqual(NS(foo=['c', 'd'], bar=['a', 'b']), args) + args = parser.parse_args(['a', '--', 'b', '--foo', 'c', 'd']) + self.assertEqual(NS(foo=None, bar=['a', 'b', '--foo', 'c', 'd']), args) + args, argv = parser.parse_known_args(['a', 'b', '--foo', 'c', '--', 'd']) + self.assertEqual(NS(foo=['c'], bar=['a', 'b']), args) + self.assertEqual(argv, ['--', 'd']) + + parser = argparse.ArgumentParser(exit_on_error=False) + parser.add_argument('foo') + parser.add_argument('bar', nargs='*') + + args = parser.parse_args(['--', 'a', 'b', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', '--', 'b', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', 'b', '--', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', '--', 'b', '--', 'c']) + self.assertEqual(NS(foo='a', bar=['b', '--', 'c']), args) + args = parser.parse_args(['--', '--', 'a', '--', 'b', 'c']) + self.assertEqual(NS(foo='--', bar=['a', '--', 'b', 'c']), args) + + parser = argparse.ArgumentParser(exit_on_error=False) + parser.add_argument('foo') + parser.add_argument('bar', nargs=argparse.REMAINDER) + + args = parser.parse_args(['--', 'a', 'b', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', '--', 'b', 'c']) + self.assertEqual(NS(foo='a', bar=['b', 'c']), args) + args = parser.parse_args(['a', 'b', '--', 'c']) + self.assertEqual(NS(foo='a', bar=['b', '--', 'c']), args) + args = parser.parse_args(['a', '--', 'b', '--', 'c']) + self.assertEqual(NS(foo='a', bar=['b', '--', 'c']), args) # =========================== diff --git a/Misc/NEWS.d/next/Library/2024-09-19-10-36-18.gh-issue-81691.Hyhp_U.rst b/Misc/NEWS.d/next/Library/2024-09-19-10-36-18.gh-issue-81691.Hyhp_U.rst new file mode 100644 index 00000000000000..8f0108502efde6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-09-19-10-36-18.gh-issue-81691.Hyhp_U.rst @@ -0,0 +1,3 @@ +Fix handling of multiple ``"--"`` (double dashes) in :mod:`argparse`. Only +the first one has now been removed, all subsequent ones are now taken +literally. From 2560f6e32f2736fdf5d52ec7f34794e006d67c3f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 20 Sep 2024 08:46:54 +0300 Subject: [PATCH 2/2] Minimize diff. --- Lib/argparse.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Lib/argparse.py b/Lib/argparse.py index 2d899551255bcf..7988c447d03584 100644 --- a/Lib/argparse.py +++ b/Lib/argparse.py @@ -1916,14 +1916,12 @@ def _parse_known_args(self, arg_strings, namespace): # which has an 'O' if there is an option at an index, # an 'A' if there is an argument, or a '-' if there is a '--' option_string_indices = {} - double_dash_index = len(arg_strings) arg_string_pattern_parts = [] arg_strings_iter = iter(arg_strings) for i, arg_string in enumerate(arg_strings_iter): # all args after -- are non-options if arg_string == '--': - double_dash_index = i arg_string_pattern_parts.append('-') for arg_string in arg_strings_iter: arg_string_pattern_parts.append('A') @@ -2073,10 +2071,9 @@ def consume_positionals(start_index): for action, arg_count in zip(positionals, arg_counts): args = arg_strings[start_index: start_index + arg_count] # Strip out the first '--' if it is not in PARSER or REMAINDER arg. - if (arg_count - and start_index <= double_dash_index < start_index + arg_count - and action.nargs not in [PARSER, REMAINDER]): - assert args.index('--') == double_dash_index - start_index + if (action.nargs not in [PARSER, REMAINDER] + and arg_strings_pattern.find('-', start_index, + start_index + arg_count) >= 0): args.remove('--') start_index += arg_count if args and action.deprecated and action.dest not in warned: