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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 23, 2026, 17:25 UTC
Message-ID: <xmqqqzijc22h.fsf@gitster.g>
In-Reply-To: <CAP8UFD0KP+e4EYVAKW1+6n3og1nzi_+Utr59Vgo8Fz0G=WZ-Qw@mail.gmail.com>

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

Show 13 quoted lines

> Whether the next argument has to be skipped is decided by the caller:
>
>   if (parse_options_takes_argument(opt) && !value && i + 1 < argc)
>       value = argv[++i];
>
> find_early_scan_option() cannot do that itself, as it has neither
> argv, argc nor the current index.
>
> So signalling to the caller would be redundant, because the caller
> already holds the matched option and can ask directly.
>
> But maybe I should add a comment on the line before `if (!*rest) {`
> saying that skipping a separate value is the caller's job?

Not really. I was hinting if it is cleaner to have the callee do the skipping so that caller does not have to worry about it. After all, the job of the early-scan machinery is to scan the options reliably to find something later in the command line argument array. The less the caller needs to do, the easier the machinery is to use.

Show 16 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.
>
> Now using `takes_value` in the `*rest == '='` case wasn't quite right,
> as parse_options_takes_argument() returns 0 for PARSE_OPT_OPTARG and
> PARSE_OPT_LASTARG_DEFAULT, but parse_options() does accept a stuck
> value for both.
>
> So in v2 we use the same condition parse_options() uses:

My giving an opaque hint pays off sometimes ;-)

Thanks.