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

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

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

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