git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 5/6] parse-options: normalize arg and long_name before comparison

From
Josh Steadmon <steadmon@google.com>
Date
Mar 12, 2024, 21:43 UTC
Message-ID
<ZfDMn-3dKDQwYx_v@google.com>
In-Reply-To
<20240303121944.20627-6-l.s.r@web.de>
On 2024.03.03 13:19, René Scharfe wrote:
Show 54 quoted lines
> Strip "no-" from arg and long_name before comparing them.  This way we
> no longer have to repeat the comparison with an offset of 3 for negated
> arguments.
> 
> Note that we must not modify the "flags" value, which tracks whether arg
> is negated, inside the loop.  When registering "--n", "--no" or "--no-"
> as abbreviation for any negative option, we used to OR it with OPT_UNSET
> and end the loop.  We can simply hard-code OPT_UNSET and leave flags
> unchanged instead.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  parse-options.c | 44 ++++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/parse-options.c b/parse-options.c
> index 008c0f32cf..d45efa4e5c 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -382,28 +382,42 @@ static enum parse_opt_result parse_long_opt(
>  	const struct option *options)
>  {
>  	const char *arg_end = strchrnul(arg, '=');
> +	const char *arg_start = arg;
> +	enum opt_parsed flags = OPT_LONG;
> +	int arg_starts_with_no_no = 0;
>  	struct parsed_option abbrev = { .option = NULL, .flags = OPT_LONG };
>  	struct parsed_option ambiguous = { .option = NULL, .flags = OPT_LONG };
> 
> +	if (skip_prefix(arg_start, "no-", &arg_start)) {
> +		if (skip_prefix(arg_start, "no-", &arg_start))
> +			arg_starts_with_no_no = 1;
> +		else
> +			flags |= OPT_UNSET;
> +	}
> +
>  	for (; options->type != OPTION_END; options++) {
>  		const char *rest, *long_name = options->long_name;
> -		enum opt_parsed flags = OPT_LONG, opt_flags = OPT_LONG;
> +		enum opt_parsed opt_flags = OPT_LONG;
> +		int allow_unset = !(options->flags & PARSE_OPT_NONEG);
> 
>  		if (options->type == OPTION_SUBCOMMAND)
>  			continue;
>  		if (!long_name)
>  			continue;
> 
> -		if (!starts_with(arg, "no-") &&
> -		    !(options->flags & PARSE_OPT_NONEG) &&
> -		    skip_prefix(long_name, "no-", &long_name))
> +		if (skip_prefix(long_name, "no-", &long_name))
>  			opt_flags |= OPT_UNSET;
> +		else if (arg_starts_with_no_no)
> +			continue;

We only allow double negation ("--no-no-foo") if the canonical form starts with "--no-". Makes sense.

> +		if (((flags ^ opt_flags) & OPT_UNSET) && !allow_unset)
> +			continue;

I don't think the "& OPT_UNSET" is required here, as we never set any flags other than OPT_UNSET, but I think it's fine to keep it for clarity.

Show 41 quoted lines
> -		if (!skip_prefix(arg, long_name, &rest))
> +		if (!skip_prefix(arg_start, long_name, &rest))
>  			rest = NULL;
>  		if (!rest) {
>  			/* abbreviated? */
> -			if (!strncmp(long_name, arg, arg_end - arg)) {
> +			if (!strncmp(long_name, arg_start, arg_end - arg_start)) {
>  				register_abbrev(p, options, flags ^ opt_flags,
>  						&abbrev, &ambiguous);
>  			}
> @@ -412,24 +426,10 @@ static enum parse_opt_result parse_long_opt(
>  				continue;
>  			/* negated and abbreviated very much? */
>  			if (starts_with("no-", arg)) {
> -				flags |= OPT_UNSET;
> -				register_abbrev(p, options, flags ^ opt_flags,
> +				register_abbrev(p, options, OPT_UNSET ^ opt_flags,
>  						&abbrev, &ambiguous);
> -				continue;
> -			}
> -			/* negated? */
> -			if (!starts_with(arg, "no-"))
> -				continue;
> -			flags |= OPT_UNSET;
> -			if (!skip_prefix(arg + 3, long_name, &rest)) {
> -				/* abbreviated and negated? */
> -				if (!strncmp(long_name, arg + 3,
> -					     arg_end - arg - 3))
> -					register_abbrev(p, options,
> -							flags ^ opt_flags,
> -							&abbrev, &ambiguous);
> -				continue;
>  			}
> +			continue;
>  		}
>  		if (*rest) {
>  			if (*rest != '=')
> --
> 2.44.0
> 
> 
Previous: René ScharfeNext: René Scharfe
Message 14 of 23 in “parse-options: long option parsing fixes and cleanup”
  1. 0/6 parse-options: long option parsing fixes and cleanupRené Scharfe, Feb 24, 2024
  2. 2/6 parse-options: set arg of abbreviated option lazilyRené Scharfe, Feb 24, 2024
  3. 3/6 parse-options: factor out register_abbrev() and struct parsed_optionRené Scharfe, Feb 24, 2024
  4. Junio C HamanoFeb 24, 2024
  5. 1/6 parse-options: recognize abbreviated negated option with argRené Scharfe, Feb 24, 2024
  6. 6/6 parse-options: rearrange long_name matching codeRené Scharfe, Feb 24, 2024
  7. 5/6 parse-options: normalize arg and long_name before comparisonRené Scharfe, Feb 24, 2024
  8. 4/6 parse-options: detect ambiguous self-negationRené Scharfe, Feb 24, 2024
  9. René ScharfeFeb 24, 2024
  10. 0/6 parse-options: long option parsing fixes and cleanupRené Scharfe, Mar 3, 2024
  11. 6/6 parse-options: rearrange long_name matching codeRené Scharfe, Mar 3, 2024
  12. Josh SteadmonMar 12, 2024
  13. 5/6 parse-options: normalize arg and long_name before comparisonRené Scharfe, Mar 3, 2024
  14. Josh SteadmonMar 12, 2024
  15. 2/6 parse-options: set arg of abbreviated option lazilyRené Scharfe, Mar 3, 2024
  16. 1/6 parse-options: recognize abbreviated negated option with argRené Scharfe, Mar 3, 2024
  17. Josh SteadmonMar 12, 2024
  18. 4/6 parse-options: detect ambiguous self-negationRené Scharfe, Mar 3, 2024
  19. 3/6 parse-options: factor out register_abbrev() and struct parsed_optionRené Scharfe, Mar 3, 2024
  20. Josh SteadmonMar 12, 2024
  21. Junio C HamanoMar 13, 2024
  22. René ScharfeMar 13, 2024
  23. Josh SteadmonMar 12, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.