Re: [PATCH 1/6] parse-options: add early_scan_options()

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 2, 2026, 22:11 UTC
Message-ID: <xmqqy0djfgmt.fsf@gitster.g>
In-Reply-To: <20260902161047.476753-2-christian.couder@gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

> So users must spell these specific options in full. This restriction
> could be lifted in the future though, once the scanner is adapted to
> accept a command's full option array, as this would give it the
> complete context needed for safe abbreviation matching.

It is unfortunate that end-users cannot tell if they are dealing with a system before of after "once the scanner is adapted" happened, so they must be trained to always spell the options in full to make use of the commands that use this feature. It at least does not regress relative to the ad-hoc early scanners these selected commands have that do not even understand what they are parsing, so it may not be too bad.

Stepping back a bit, the burden on programmers to use this would be to write in a separate notation what options there are in addition to what they feed the real parse_options(), which cuts both ways in the sense that because this does not take parse_options(), commands that do not use parse_options() can still use it, but those that do already use parse_options() need additional work to use eary_scan.

And then once the scanner is adapted to accept the full option array, the programmers only need to discard the struct early_scan_option[] they wrote and replace it with the struct option[] they already have? Or would the calling convention to the scanner also change when it happens (oother than replacing the pointer to struct early_scan_option[] with another pointer to struct option[])?

> +static const struct early_scan_option *
> +find_early_scan_option(const char *arg,
> +		       const struct early_scan_option *options,
> +		       const char **value)

Because you return one single element from the incoming array of options, it is mildly misleading to call the variable/parameter "options" here and everywhere else. Let's stick to "arrays are named singular, so that option[4] names 4th option" convention.

Show 9 quoted lines

> +{
> +	if (!skip_prefix(arg, "--", &arg))
> +		return NULL;
> +
> +	for (; options->name; options++) {
> +		const char *rest;
> +
> +		if (!skip_prefix(arg, options->name, &rest))
> +			continue;

"--option" on the command line, after getting stripped the leading "--", may begin with "option", and that name may be in the option[] table, in which case ...

> +		if (!*rest) {
> +			*value = NULL;
> +			return options;
> +		}

... we found a hit. But shouldn't option->takes_value be consulted before we return to signal the caller that the next arg is an option value before we return from here? It looks a bit uneven as we do that for stuck form "--option=value" here.

Show 5 quoted lines

> +		/* Only an option taking a value can be stuck to one. */
> +		if (*rest == '=' && options->takes_value) {
> +			*value = rest + 1;
> +			return options;
> +		}

And if the option[] table had "opt", then "--option" on the command line may begin with "--opt" but "ion" is an excess that is not a stuck value, so we do not consider it as a match. OK.

> +	}
> +	return NULL;
> +}

If we are to write a separate function anyway, I wonder how much more work to write a early_scan_option() parser that does take a real "struct option[]" array. Its elements already know if they take a value or not. For expediency, it may be OK to start by simplified parser that does not handle unique prefix and other complexities like callback functions of the real parser, but at least it would reduce the burden on the programmers quite a bit if we used the real struct option[] array, I suspect.