Re: [PATCH] rev-parse --parseopt: fix handling of optional arguments

[Jeff King <peff@peff.net>]

On Wed, Oct 16, 2013 at 02:40:07PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > ... But what is the normalized form for an
> > optional argument? It either needs to be consistently "sticked" or
> > "unsticked", either:
> >
> >   set -- -S '' --     ;# default
> >   set -- -S 'foo' --  ;# not default
> >
> > or
> >
> >   set -- -S --    ;# default
> >   set -- -Sfoo -- ;# not default
> >
> > so that reading the normalized form is unambiguous.
> 
> The analysis makes sense.  Either form do not let you distinguish
> between the case where the end user wanted to explicitly pass "" as
> the optional parameter to -S and the case where she gave -S without
> any optional parameter, though.

I almost mentioned that, but I am not sure that it matters. Keep in mind
that:

  git my-script -S foo

already does not involve "foo" with S, because it is not "sticked". So
there is no way for the _user_ to distinguish between "I want the
default" and "I passed you an empty string"; because the argument must
be sticked they both look like "-S". And that distinction is already
impossible in the definition of optional arguments, and is not a problem
with going through the "git rev-parse --parseopt" channel at all.

So the only bug is the ambiguity in the current normalized form. Of the
two forms above, I think I prefer:

  set -- -S '' --

because it more closely matches the non-optional form we produce
today, and because it is slightly less work to parse (you can check that
$1 is "-S", and then store or check "$2", rather than having to match
"-S*" and parse off the beginning).

> Which pretty much agrees with j6t's (and my earlier) comment that
> there is no way to solve this issue completely, I think.

I guess it depends on what the issue is. :)

No, I do not think you can ever "fix" the options to let those two cases
be distinguishable. But I do not think anybody is really asking for
that; the real concern is that the "rev-parse --parseopt" normalization
is ambiguous, and that is easily fixable.

-Peff