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

[Junio C Hamano <gitster@pobox.com>]

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

> 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.

>> > +             /* 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.