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

From: Jeff King <peff@peff.net>
Date: Oct 16, 2013, 21:50 UTC
Message-ID: <20131016215047.GA10260@sigill.intra.peff.net>
In-Reply-To: <xmqqsiw051zc.fsf@gitster.dls.corp.google.com>

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

Show 20 quoted lines

> 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