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

From: Nicolas Vigier <[email-masked]>
Date: Wed, 16 Oct 2013 01:47:01 +0200

On Tue, 15 Oct 2013, Junio C Hamano wrote:

> Nicolas Vigier <[email-masked]> writes:
> 
> > git rev-parse --parseopt does not allow us to see the difference
> > between an option with an optional argument starting with a dash, and an
> > option with an unset optional argument followed by an other option.
> >
> > If I use this script :
> >
> >   $ cat /tmp/opt.sh
> >   #!/bin/sh
> >   OPTIONS_SPEC="\
> >   git [options]
> >   --
> >   q,quiet         be quiet
> >   S,gpg-sign?     GPG-sign commit"
> >   echo "$OPTIONS_SPEC" | git rev-parse --parseopt $parseopt_extra -- "$@"
> >
> > Then the following two commands give us the same result :
> >
> >   $ /tmp/opt.sh -S -q
> >   set -- -S -q --
> >   $ /tmp/opt.sh -S-q
> >   set -- -S '-q' --
> >
> > We cannot know if '-q' is an argument to '-S' or a new option.
> >
> > With this patch, rev-parse --parseopt will always give an argument to
> > optional options, as an empty string if the argument is unset.
> >
> > The same two commands now give us :
> >
> >   $ /tmp/opt.sh -S -q
> >   set -- -S '' -q --
> >   $ /tmp/opt.sh -S-q
> >   set -- -S '-q' --
> 
> Two are different, but the former "set -- -S '' -q --" is not what
> you want, either, no?  -S with an explicit empty argument and -S
> alone without argument may mean two totally different things, which
> is the whole point of "option with an optional parameter".  If some
> code that have been using "rev-parse --parseopt" was happy with
> 
> 	$ /tmp/opt.sh -S
>         set -- -S --
> 
> and then your updated version gave it this instead:
> 
> 	$ /tmp/opt.sh -S
>         set -- -S '' --
> 
> wouldn't it be a regression to them?

Indeed, this could be a regression to them. I couldn't find any script
using "rev-parse --parseopt" with an option with an optional argument,
but yes, it doesn't mean that nobody uses that.