threads / patch / 35144

patchrev-parse --parseopt: fix handling of optional arguments

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

## tl;dr

25 messages between Oct 15, 2013 and Oct 31, 2013. Diffs are folded; open one to read it.

replies: 24people: 7as markdown or json

Nicolas Vigier· Oct 15, 2013, 12:00 UTC · lore

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' --
We can now see if '-q' is an argument to '-S' or an other option.
Also adding two tests in t1502.

There does not seem to be any shell script git command included in git sources tree that is currently using optional arguments and could be affected by this change.

Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
---
 builtin/rev-parse.c           |  3 +++
 t/t1502-rev-parse-parseopt.sh | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)
Show changes to 2 files +21 −0

builtin/rev-parse.c, t/t1502-rev-parse-parseopt.sh

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..25e8c74 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -327,6 +327,9 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
 	if (arg) {
 		strbuf_addch(parsed, ' ');
 		sq_quote_buf(parsed, arg);
+	} else if (o->flags & PARSE_OPT_OPTARG) {
+		const char empty_arg[] = " ''";
+		strbuf_add(parsed, empty_arg, strlen(empty_arg));
 	}
 	return 0;
 }
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 13c88c9..abe7c2f 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -99,4 +99,22 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou
 	test_cmp expect output
 '
 
+cat > expect <<EOF
+set -- -C '' --foo --
+EOF
+
+test_expect_success 'test --parseopt -C --foo' '
+	git rev-parse --parseopt -- -C --foo <optionspec >output &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+set -- -C '--foo' --
+EOF
+
+test_expect_success 'test --parseopt -C--foo' '
+	git rev-parse --parseopt -- -C--foo <optionspec >output &&
+	test_cmp expect output
+'
+
 test_done
-- 
1.8.4
Junio C Hamano· Oct 15, 2013, 22:55 UTC · re: Nicolas Vigier · lore

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

Nicolas Vigier <boklm@mars-attacks.org> writes:
Show 33 quoted lines
> 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?
Nicolas Vigier· Oct 15, 2013, 23:47 UTC · re: Junio C Hamano · lore

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

On Tue, 15 Oct 2013, Junio C Hamano wrote:
Show 51 quoted lines
> Nicolas Vigier <boklm@mars-attacks.org> 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.

Jonathan Nieder· Oct 15, 2013, 23:14 UTC · re: Nicolas Vigier · lore

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

Nicolas Vigier wrote:
Show 17 quoted lines
>   $ 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.
Hmph.

As Junio mentioned, inserting '' would be a backward-incompatible change. I don't think it's worth breaking existing scripts. Probably what is needed is a new parseopt special character with the new semantics (e.g.,

	Use ?? to mean the option has an optional argument.  If the
	option is supplied without its argument, the argument is taken
	to be ''.
or something like
	Use ?<default> to mean the option has an optional argument.  If
	the option is supplied without its argument and <default> is
	nonempty, the argument is taken to be <default>.
).
Sensible?

Thanks, Jonathan

Junio C Hamano· Oct 15, 2013, 23:33 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 40 quoted lines
> Nicolas Vigier wrote:
>
>>   $ 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.
>
> Hmph.
>
> As Junio mentioned, inserting '' would be a backward-incompatible
> change.  I don't think it's worth breaking existing scripts.  Probably
> what is needed is a new parseopt special character with the new
> semantics (e.g.,
>
> 	Use ?? to mean the option has an optional argument.  If the
> 	option is supplied without its argument, the argument is taken
> 	to be ''.
>
> or something like
>
> 	Use ?<default> to mean the option has an optional argument.  If
> 	the option is supplied without its argument and <default> is
> 	nonempty, the argument is taken to be <default>.
>
> ).
>
> Sensible?

You just made these two that the user clearly meant to express two different things indistinguishable.

	opt.sh -S
        opt.sh -S ''

So I do not think it is sensible. In fact, I do not think there is any sensible way to handle a shortopt with optional parameter that is not at the end of the command line.

And that is exactly why gitcli.txt tells users to use the 'sticked' form, and ends the bullet point with:

   An option that takes optional option-argument must be written in
   the 'sticked' form.

That still does not give the command line a way to express an option that could take an optional argument without the optional argument in the middle of the command line, though.

Jonathan Nieder· Oct 15, 2013, 23:57 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 5 quoted lines
> You just made these two that the user clearly meant to express two
> different things indistinguishable.
>
> 	opt.sh -S
>       opt.sh -S ''
[...]
Show 5 quoted lines
> And that is exactly why gitcli.txt tells users to use the 'sticked'
> form, and ends the bullet point with:
>
>    An option that takes optional option-argument must be written in
>    the 'sticked' form.

Yes, another possibility in that vein would be to teach rev-parse --parseopt an OPTIONS_LONG_STICKED output format, and then parse with

	while :
	do
		case $1 in
		--gpg-sign)
			... no keyid ...
			;;
		--gpg-sign=*)
			keyid=${1#--gpg-sign=}
			...
			;;
		esac
		shift
	done
This still leaves
	opt.sh -S
	
and
	opt.sh -S''

indistinguishable. Given what the shell passes to execve, I think that's ok.

The analagous method without preferring long options could work almost as well:

	while :
	do
		case $1 in
		-S)
			... no keyid ...
			;;
		-S?*)
			keyid=${1#-S}
			...
			;;
		esac
		shift
	done
but it mishandles "--gpg-sign=" with empty argument.
Jonathan
Johannes Sixt· Oct 16, 2013, 07:04 UTC · re: Jonathan Nieder · lore

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

Am 10/16/2013 1:57, schrieb Jonathan Nieder:
Show 16 quoted lines
> Junio C Hamano wrote:
> 
>> You just made these two that the user clearly meant to express two
>> different things indistinguishable.
>>
>> 	opt.sh -S
>>       opt.sh -S ''
> [...]
>> And that is exactly why gitcli.txt tells users to use the 'sticked'
>> form, and ends the bullet point with:
>>
>>    An option that takes optional option-argument must be written in
>>    the 'sticked' form.
> 
> Yes, another possibility in that vein would be to teach rev-parse
> --parseopt an OPTIONS_LONG_STICKED output format, and then parse with
Aren't you people trying to solve something that can't besolved? What does
  git commit -S LICENSE

mean? Commit the index and sign with the key-id "LICENSE" or commit just the file "LICENSE" with the default key-id?

-- Hannes
Jeff King· Oct 16, 2013, 08:53 UTC · re: Johannes Sixt · lore

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

On Wed, Oct 16, 2013 at 09:04:32AM +0200, Johannes Sixt wrote:
Show 9 quoted lines
> > Yes, another possibility in that vein would be to teach rev-parse
> > --parseopt an OPTIONS_LONG_STICKED output format, and then parse with
> 
> Aren't you people trying to solve something that can't besolved? What does
> 
>   git commit -S LICENSE
> 
> mean? Commit the index and sign with the key-id "LICENSE" or commit just
> the file "LICENSE" with the default key-id?

It means the latter. Because the argument is optional, you must use the "sticked" form:

  git commit -SLICENSE

If the caller does not know whether the argument is optional or not, they should use the sticked form to be on the safe side.

The problem, as I understand it, arises from the fact that shell scripts can use "git rev-parse --parseopt" to check and normalize their arguments. So for example:

  # pretend we got "-bs" on our command line
  set -- -bs
  git rev-parse --parseopt -- "$@" <<\EOF
  usage...
  --
  b!    the "b" option
  s!    the "s" option
  EOF
would produce:
  set -- -b -s --

The latter is much easier to parse in the shell, because options are split, it ends with "--", etc. 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.
-Peff
Junio C Hamano· Oct 16, 2013, 21:40 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 13 quoted lines
> ... 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.

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

It is an acceptable compromise to use your suggestion as a solution that works for cases other than passing an explicit empty string as an optional parameter, I would say, if the limitation is clearly documented.

Thanks.
Jeff King· Oct 16, 2013, 21:50 UTC · re: Junio C Hamano · lore

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

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
Nicolas Vigier· Oct 16, 2013, 10:58 UTC · re: Johannes Sixt · lore

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

On Wed, 16 Oct 2013, Johannes Sixt wrote:
Show 24 quoted lines
> Am 10/16/2013 1:57, schrieb Jonathan Nieder:
> > Junio C Hamano wrote:
> > 
> >> You just made these two that the user clearly meant to express two
> >> different things indistinguishable.
> >>
> >> 	opt.sh -S
> >>       opt.sh -S ''
> > [...]
> >> And that is exactly why gitcli.txt tells users to use the 'sticked'
> >> form, and ends the bullet point with:
> >>
> >>    An option that takes optional option-argument must be written in
> >>    the 'sticked' form.
> > 
> > Yes, another possibility in that vein would be to teach rev-parse
> > --parseopt an OPTIONS_LONG_STICKED output format, and then parse with
> 
> Aren't you people trying to solve something that can't besolved? What does
> 
>   git commit -S LICENSE
> 
> mean? Commit the index and sign with the key-id "LICENSE" or commit just
> the file "LICENSE" with the default key-id?

The later, as optional arguments needs to be sticked, but I think this is not related to the problems with --parseopt.

Here is a summary the problems I think we have with --parseopt and proposed solutions. This command makes it possible to parse arguments with a loop like this :

  while test $# != 0
  do
        case "$1" in
        -q)
                GIT_QUIET=t ;;
        -S)
                # do something with $2
                # shift if you think $2 is an optional arg
                ;;
        --)
                shift; break ;;
        esac
        shift
  done
  # do something with the other arguments

And I think the problems with the '?' flag when using this kind of loop are :

 - You don't know if $2 is the optional argument for -S, or the next option
 
 - You can't use '--' as an optional argument to -S, because you don't
   know if '--' is the optional argument to -S, or the separator between
   options and non options.
To fix this, solution 1) is to always include the optional argument in
$2, and set it to '' if it is not set. However this brings the problem
that you can't distinguish between unset argument and empty argument.
The following two become the same :
  ./opt.sh --gpg-id
  ./opt.sh --gpg-id=

Solution 2) is to use a flag like ?<default> as suggested by Jonathan. Now you can distinguish between unset and empty args, but you can't between <default> and unset, but this is probably not a big problem as you can select <default> so that it is a value nobody would want to use.

Solution 3) is the OPTIONS_LONG_STICKED output format suggested by Jonathan. I can't see any problem with this one.

Nicolas Vigier· Oct 16, 2013, 14:14 UTC · re: Jonathan Nieder · lore

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

On Tue, 15 Oct 2013, Jonathan Nieder wrote:
Show 60 quoted lines
> Junio C Hamano wrote:
> 
> > You just made these two that the user clearly meant to express two
> > different things indistinguishable.
> >
> > 	opt.sh -S
> >       opt.sh -S ''
> [...]
> > And that is exactly why gitcli.txt tells users to use the 'sticked'
> > form, and ends the bullet point with:
> >
> >    An option that takes optional option-argument must be written in
> >    the 'sticked' form.
> 
> Yes, another possibility in that vein would be to teach rev-parse
> --parseopt an OPTIONS_LONG_STICKED output format, and then parse with
> 
> 	while :
> 	do
> 		case $1 in
> 		--gpg-sign)
> 			... no keyid ...
> 			;;
> 		--gpg-sign=*)
> 			keyid=${1#--gpg-sign=}
> 			...
> 			;;
> 		esac
> 		shift
> 	done
> 
> This still leaves
> 
> 	opt.sh -S
> 	
> and
> 
> 	opt.sh -S''
> 
> indistinguishable.  Given what the shell passes to execve, I think
> that's ok.
> 
> The analagous method without preferring long options could work almost
> as well:
> 
> 	while :
> 	do
> 		case $1 in
> 		-S)
> 			... no keyid ...
> 			;;
> 		-S?*)
> 			keyid=${1#-S}
> 			...
> 			;;
> 		esac
> 		shift
> 	done
> 
> but it mishandles "--gpg-sign=" with empty argument.
I'm thinking about a patch to add the following two options to rev-parse :
--sticked-opt-args::
	Only meaningful in --parseopt mode. Tells the options parser to
	output options with optional arguments in sticked form. The
	default is to output them in non-sticked mode, which can be
	difficult to parse unambiguously.
--long-options::
	Only meaningful in --parseopt mode. Tells the options parser to
	output long option names, when available. The default is to use
	short option names when available.

When you want to handle optional args unambiguously, you use the --sticked-opt-args option. And if you think an empty value can be a meaningful value, you add the --long-options option to be able to distinguish them.

Would it make sense ?
Jonathan Nieder· Oct 16, 2013, 22:33 UTC · re: Nicolas Vigier · lore

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

Nicolas Vigier wrote:
Show 19 quoted lines
> I'm thinking about a patch to add the following two options to rev-parse :
>
> --sticked-opt-args::
> 	Only meaningful in --parseopt mode. Tells the options parser to
> 	output options with optional arguments in sticked form. The
> 	default is to output them in non-sticked mode, which can be
> 	difficult to parse unambiguously.
>
> --long-options::
> 	Only meaningful in --parseopt mode. Tells the options parser to
> 	output long option names, when available. The default is to use
> 	short option names when available.
>
> When you want to handle optional args unambiguously, you use the
> --sticked-opt-args option. And if you think an empty value can be
> a meaningful value, you add the --long-options option to be able to
> distinguish them.
>
> Would it make sense ?
That would make four distinct output formats:
 --sticked --long:
	Doesn't lose any information that normal use of C parse_options
	would have kept, as long as every short option with optional
	argument has a corresponding long option.
 --sticked --no-long:
	Loses the distinction between --gpg-sign and --gpg-sign=
 --no-sticked --long:
	Semantically equivalent to the existing output, just noisier.
 --no-sticked --no-long:
 	The existing output.

Are all of them needed? Is it worth tempting people to use --sticked --no-long when we know its pitfalls?

I would think that only the current normalized form and --sticked --long would need to be supported.

The fix you originally proposed seems tolerable to me, too --- it is not very invasive, and while it doesn't distinguish the empty-argument form "--gpg-sign=", that's a bit of an edge case.

The main reason I slightly prefer the solution that makes the output use long, sticked options on request is that the "normalized" commandline would start being an actual equivalent command line the command expects, instead of a weird, subtly different syntax. (That problem already exists with or without your patch --- the patch just draws attention to it.)

Nicolas Vigier· Oct 25, 2013, 20:18 UTC · re: Jonathan Nieder · lore

[PATCH] rev-parse --parseopt: add the --sticked-long mode

Add the --sticked-long option to output the options in their long form if available, and with their arguments sticked.

Contrary to the default form (non sticked arguments and short options), this can be parsed unambiguously when using options with optional arguments :

 - in the non sticked form, when an option is taking an optional argument
   you cannot know if the next argument is its optional argument, or the
   next option.
 - the long options form allows to differenciate between an empty argument
   '--option=' and an unset argument '--option', which is not possible
   with short options.
Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
---
 Documentation/git-rev-parse.txt |  8 +++++++-
 builtin/rev-parse.c             | 11 +++++++++--
 t/t1502-rev-parse-parseopt.sh   | 42 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 55 insertions(+), 6 deletions(-)
Show changes to 3 files +55 −6

Documentation/git-rev-parse.txt, builtin/rev-parse.c, t/t1502-rev-parse-parseopt.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index d068a65..d3bad9d 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -50,6 +50,10 @@ Options for --parseopt
 	the first non-option argument.  This can be used to parse sub-commands
 	that take options themselves.
 
+--sticked-long::
+	Only meaningful in `--parseopt` mode. Output the options in their
+	long form if available, and with their arguments sticked.
+
 Options for Filtering
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -285,7 +289,9 @@ Each line of options has this format:
 	`<flags>` are of `*`, `=`, `?` or `!`.
 	* Use `=` if the option takes an argument.
 
-	* Use `?` to mean that the option is optional (though its use is discouraged).
+	* Use `?` to mean that the option takes an optional argument. You
+	  probably want to use the `--sticked-long` mode to be able to
+	  unambiguously parse the optional argument.
 
 	* Use `*` to mean that this option should not be listed in the usage
 	  generated for the `-h` argument. It's shown for `--help-all` as
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c76b89d..418b7f7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -30,6 +30,8 @@ static int abbrev_ref;
 static int abbrev_ref_strict;
 static int output_sq;
 
+static int sticked_long;
+
 /*
  * Some arguments are relevant "revision" arguments,
  * others are about output format or other details.
@@ -320,12 +322,15 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
 	struct strbuf *parsed = o->value;
 	if (unset)
 		strbuf_addf(parsed, " --no-%s", o->long_name);
-	else if (o->short_name)
+	else if (o->short_name && (o->long_name == NULL || !sticked_long))
 		strbuf_addf(parsed, " -%c", o->short_name);
 	else
 		strbuf_addf(parsed, " --%s", o->long_name);
 	if (arg) {
-		strbuf_addch(parsed, ' ');
+		if (!sticked_long)
+			strbuf_addch(parsed, ' ');
+		else if (o->long_name)
+			strbuf_addch(parsed, '=');
 		sq_quote_buf(parsed, arg);
 	}
 	return 0;
@@ -351,6 +356,8 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "stop-at-non-option", &stop_at_non_option,
 					N_("stop parsing after the "
 					   "first non-option argument")),
+		OPT_BOOL(0, "sticked-long", &sticked_long,
+					N_("output in sticked long form")),
 		OPT_END(),
 	};
 
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 13c88c9..7e12d9b 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -12,9 +12,11 @@ usage: some-command [options] <args>...
     -h, --help            show the help
     --foo                 some nifty option --foo
     --bar ...             some cool option --bar with an argument
+    -b, --baz             a short and long option
 
 An option group Header
     -C[...]               option C with an optional argument
+    -d, --data[=...]      short and long option with an optional argument
 
 Extras
     --extra1              line above used to cause a segfault but no longer does
@@ -31,9 +33,11 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+b,baz     a short and long option
 
  An option group Header
 C?        option C with an optional argument
+d,data?   short and long option with an optional argument
 
 Extras
 extra1    line above used to cause a segfault but no longer does
@@ -45,16 +49,16 @@ test_expect_success 'test --parseopt help output' '
 '
 
 cat > expect <<EOF
-set -- --foo --bar 'ham' -- 'arg'
+set -- --foo --bar 'ham' -b -- 'arg'
 EOF
 
 test_expect_success 'test --parseopt' '
-	git rev-parse --parseopt -- --foo --bar=ham arg < optionspec > output &&
+	git rev-parse --parseopt -- --foo --bar=ham --baz arg < optionspec > output &&
 	test_cmp expect output
 '
 
 test_expect_success 'test --parseopt with mixed options and arguments' '
-	git rev-parse --parseopt -- --foo arg --bar=ham < optionspec > output &&
+	git rev-parse --parseopt -- --foo arg --bar=ham --baz < optionspec > output &&
 	test_cmp expect output
 '
 
@@ -99,4 +103,36 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou
 	test_cmp expect output
 '
 
+cat > expect <<EOF
+set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --sticked-long' '
+	git rev-parse --parseopt --sticked-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+set -- --data='' -C --baz -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --sticked-long and empty optional argument' '
+	git rev-parse --parseopt --sticked-long -- --data= arg -C -b <optionspec >output &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+set -- --data --baz -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --sticked-long and long option with unset optional argument' '
+	git rev-parse --parseopt --sticked-long -- --data arg -b <optionspec >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'test --parseopt --sticked-long and short option with unset optional argument' '
+	git rev-parse --parseopt --sticked-long -- -d arg -b <optionspec >output &&
+	test_cmp expect output
+'
+
 test_done
-- 
1.8.4
Junio C Hamano· Oct 25, 2013, 22:01 UTC · re: Nicolas Vigier · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

Nicolas Vigier <boklm@mars-attacks.org> writes:
> Add the --sticked-long option to output the options in their long form
> if available, and with their arguments sticked.

Hmph, doesn't verb "stick" conjugate to "(present) stick (past) stuck (pp) stuck"?

Nicolas Vigier· Oct 25, 2013, 22:52 UTC · re: Junio C Hamano · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

On Fri, 25 Oct 2013, Junio C Hamano wrote:
Show 7 quoted lines
> Nicolas Vigier <boklm@mars-attacks.org> writes:
> 
> > Add the --sticked-long option to output the options in their long form
> > if available, and with their arguments sticked.
> 
> Hmph, doesn't verb "stick" conjugate to "(present) stick (past) stuck
> (pp) stuck"?
Ah, yes it seems. I don't know if 'sticked' is also correct or not.

However, 'sticked' is the word that is used in Documentation/gitcli.txt and Documentation/technical/api-parse-options.txt.

Junio C Hamano· Oct 25, 2013, 22:55 UTC · re: Nicolas Vigier · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

Nicolas Vigier <boklm@mars-attacks.org> writes:
Show 14 quoted lines
> On Fri, 25 Oct 2013, Junio C Hamano wrote:
>
>> Nicolas Vigier <boklm@mars-attacks.org> writes:
>> 
>> > Add the --sticked-long option to output the options in their long form
>> > if available, and with their arguments sticked.
>> 
>> Hmph, doesn't verb "stick" conjugate to "(present) stick (past) stuck
>> (pp) stuck"?
>
> Ah, yes it seems. I don't know if 'sticked' is also correct or not.
>
> However, 'sticked' is the word that is used in Documentation/gitcli.txt
> and Documentation/technical/api-parse-options.txt.

Yes, I know. That is why I brought it up before we inflict more damage to us.

Philip Oakley· Oct 26, 2013, 21:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

From: "Junio C Hamano" <gitster@pobox.com>
Show 23 quoted lines
> Nicolas Vigier <boklm@mars-attacks.org> writes:
>
>> On Fri, 25 Oct 2013, Junio C Hamano wrote:
>>
>>> Nicolas Vigier <boklm@mars-attacks.org> writes:
>>>
>>> > Add the --sticked-long option to output the options in their long 
>>> > form
>>> > if available, and with their arguments sticked.
>>>
>>> Hmph, doesn't verb "stick" conjugate to "(present) stick (past) 
>>> stuck
>>> (pp) stuck"?
>>
>> Ah, yes it seems. I don't know if 'sticked' is also correct or not.
>>
>> However, 'sticked' is the word that is used in 
>> Documentation/gitcli.txt
>> and Documentation/technical/api-parse-options.txt.
>
> Yes, I know. That is why I brought it up before we inflict more
> damage to us.
> --

Isn't the origin of the description that it looks like a stick (cane), and 'sticked' is a modern verbing of that form? That's what I'd assumed anyway.

Googleing "Sticked option" only linked back to Git.
Philip 
Junio C Hamano· Oct 28, 2013, 15:47 UTC · re: Philip Oakley · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

"Philip Oakley" <philipoakley@iee.org> writes:
Show 5 quoted lines
> Isn't the origin of the description that it looks like a stick (cane),
> and 'sticked' is a modern verbing of that form? That's what I'd
> assumed anyway.
>
> Googleing "Sticked option" only linked back to Git.

I know web is not the authoritative source of information ;-) but it seems that looking for "sticked vs stuck" seems to give me many explanations that boils down to what this says:

    http://en.wiktionary.org/wiki/Talk:sticked

and http://en.wiktionary.org/wiki/stick#English lists "sticked" as "archaic" when the word is used for "To glue; to attach; to adhere".

Nicolas Vigier· Oct 31, 2013, 11:08 UTC · re: Nicolas Vigier · lore

[PATCH 1/2] Use the word 'stuck' instead of 'sticked'

The past participle of 'stick' is 'stuck'.
Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
---
 Documentation/gitcli.txt                      | 6 +++---
 Documentation/technical/api-parse-options.txt | 6 +++---
 diff.c                                        | 2 +-
 diff.h                                        | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)
Show changes to 4 files +8 −8

Documentation/gitcli.txt, Documentation/technical/api-parse-options.txt, diff.c, diff.h

diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
index 3146413cce0d..c87f0ae88d6c 100644
--- a/Documentation/gitcli.txt
+++ b/Documentation/gitcli.txt
@@ -72,11 +72,11 @@ scripting Git:
  * splitting short options to separate words (prefer `git foo -a -b`
    to `git foo -ab`, the latter may not even work).
 
- * when a command line option takes an argument, use the 'sticked' form.  In
+ * when a command line option takes an argument, use the 'stuck' form.  In
    other words, write `git foo -oArg` instead of `git foo -o Arg` for short
    options, and `git foo --long-opt=Arg` instead of `git foo --long-opt Arg`
    for long options.  An option that takes optional option-argument must be
-   written in the 'sticked' form.
+   written in the 'stuck' form.
 
  * when you give a revision parameter to a command, make sure the parameter is
    not ambiguous with a name of a file in the work tree.  E.g. do not write
@@ -165,7 +165,7 @@ $ git foo -o Arg
 ----------------------------
 
 However, this is *NOT* allowed for switches with an optional value, where the
-'sticked' form must be used:
+'stuck' form must be used:
 ----------------------------
 $ git describe --abbrev HEAD     # correct
 $ git describe --abbrev=10 HEAD  # correct
diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 0be2b5159f1b..be50cf4de35c 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -29,9 +29,9 @@ that allow to change the behavior of a command.
 
 The parse-options API allows:
 
-* 'sticked' and 'separate form' of options with arguments.
-  `-oArg` is sticked, `-o Arg` is separate form.
-  `--option=Arg` is sticked, `--option Arg` is separate form.
+* 'stuck' and 'separate form' of options with arguments.
+  `-oArg` is stuck, `-o Arg` is separate form.
+  `--option=Arg` is stuck, `--option Arg` is separate form.
 
 * Long options may be 'abbreviated', as long as the abbreviation
   is unambiguous.
diff --git a/diff.c b/diff.c
index e34bf971207f..3950e0191067 100644
--- a/diff.c
+++ b/diff.c
@@ -3394,7 +3394,7 @@ int parse_long_opt(const char *opt, const char **argv,
 	if (prefixcmp(arg, opt))
 		return 0;
 	arg += strlen(opt);
-	if (*arg == '=') { /* sticked form: --option=value */
+	if (*arg == '=') { /* stuck form: --option=value */
 		*optarg = arg + 1;
 		return 1;
 	}
diff --git a/diff.h b/diff.h
index e34232501ee8..1c05b3a6bec6 100644
--- a/diff.h
+++ b/diff.h
@@ -244,7 +244,7 @@ extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *pat
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
 /*
- * Poor man's alternative to parse-option, to allow both sticked form
+ * Poor man's alternative to parse-option, to allow both stuck form
  * (--option=value) and separate form (--option value).
  */
 extern int parse_long_opt(const char *opt, const char **argv,
-- 
1.8.4
Junio C Hamano· Oct 31, 2013, 19:35 UTC · re: Nicolas Vigier · lore

Re: [PATCH 1/2] Use the word 'stuck' instead of 'sticked'

Nicolas Vigier <boklm@mars-attacks.org> writes:
> The past participle of 'stick' is 'stuck'.
>
> Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
> ---
Thanks.

It was good that we caught this before introducing the option; the documentation update does not hurt the users much, but if we unleash a misspelled option in a release, it is much harder to correct it later.

Will queue both patches.
Show 76 quoted lines
>  Documentation/gitcli.txt                      | 6 +++---
>  Documentation/technical/api-parse-options.txt | 6 +++---
>  diff.c                                        | 2 +-
>  diff.h                                        | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
> index 3146413cce0d..c87f0ae88d6c 100644
> --- a/Documentation/gitcli.txt
> +++ b/Documentation/gitcli.txt
> @@ -72,11 +72,11 @@ scripting Git:
>   * splitting short options to separate words (prefer `git foo -a -b`
>     to `git foo -ab`, the latter may not even work).
>  
> - * when a command line option takes an argument, use the 'sticked' form.  In
> + * when a command line option takes an argument, use the 'stuck' form.  In
>     other words, write `git foo -oArg` instead of `git foo -o Arg` for short
>     options, and `git foo --long-opt=Arg` instead of `git foo --long-opt Arg`
>     for long options.  An option that takes optional option-argument must be
> -   written in the 'sticked' form.
> +   written in the 'stuck' form.
>  
>   * when you give a revision parameter to a command, make sure the parameter is
>     not ambiguous with a name of a file in the work tree.  E.g. do not write
> @@ -165,7 +165,7 @@ $ git foo -o Arg
>  ----------------------------
>  
>  However, this is *NOT* allowed for switches with an optional value, where the
> -'sticked' form must be used:
> +'stuck' form must be used:
>  ----------------------------
>  $ git describe --abbrev HEAD     # correct
>  $ git describe --abbrev=10 HEAD  # correct
> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
> index 0be2b5159f1b..be50cf4de35c 100644
> --- a/Documentation/technical/api-parse-options.txt
> +++ b/Documentation/technical/api-parse-options.txt
> @@ -29,9 +29,9 @@ that allow to change the behavior of a command.
>  
>  The parse-options API allows:
>  
> -* 'sticked' and 'separate form' of options with arguments.
> -  `-oArg` is sticked, `-o Arg` is separate form.
> -  `--option=Arg` is sticked, `--option Arg` is separate form.
> +* 'stuck' and 'separate form' of options with arguments.
> +  `-oArg` is stuck, `-o Arg` is separate form.
> +  `--option=Arg` is stuck, `--option Arg` is separate form.
>  
>  * Long options may be 'abbreviated', as long as the abbreviation
>    is unambiguous.
> diff --git a/diff.c b/diff.c
> index e34bf971207f..3950e0191067 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3394,7 +3394,7 @@ int parse_long_opt(const char *opt, const char **argv,
>  	if (prefixcmp(arg, opt))
>  		return 0;
>  	arg += strlen(opt);
> -	if (*arg == '=') { /* sticked form: --option=value */
> +	if (*arg == '=') { /* stuck form: --option=value */
>  		*optarg = arg + 1;
>  		return 1;
>  	}
> diff --git a/diff.h b/diff.h
> index e34232501ee8..1c05b3a6bec6 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -244,7 +244,7 @@ extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *pat
>  #define DIFF_SETUP_USE_SIZE_CACHE	4
>  
>  /*
> - * Poor man's alternative to parse-option, to allow both sticked form
> + * Poor man's alternative to parse-option, to allow both stuck form
>   * (--option=value) and separate form (--option value).
>   */
>  extern int parse_long_opt(const char *opt, const char **argv,
Nicolas Vigier· Oct 31, 2013, 11:08 UTC · re: Nicolas Vigier · lore

[PATCH 2/2] rev-parse --parseopt: add the --stuck-long mode

Add the --stuck-long option to output the options in their long form if available, and with their arguments stuck.

Contrary to the default form (non stuck arguments and short options), this can be parsed unambiguously when using options with optional arguments :

 - in the non stuck form, when an option is taking an optional argument
   you cannot know if the next argument is its optional argument, or the
   next option.
 - the long options form allows to differentiate between an empty argument
   '--option=' and an unset argument '--option', which is not possible
   with short options.
Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
---
 Documentation/git-rev-parse.txt |  8 +++++++-
 builtin/rev-parse.c             | 11 +++++++++--
 t/t1502-rev-parse-parseopt.sh   | 42 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 55 insertions(+), 6 deletions(-)
Show changes to 3 files +55 −6

Documentation/git-rev-parse.txt, builtin/rev-parse.c, t/t1502-rev-parse-parseopt.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index d068a653778d..a436b24cc406 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -50,6 +50,10 @@ Options for --parseopt
 	the first non-option argument.  This can be used to parse sub-commands
 	that take options themselves.
 
+--stuck-long::
+	Only meaningful in `--parseopt` mode. Output the options in their
+	long form if available, and with their arguments stuck.
+
 Options for Filtering
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -285,7 +289,9 @@ Each line of options has this format:
 	`<flags>` are of `*`, `=`, `?` or `!`.
 	* Use `=` if the option takes an argument.
 
-	* Use `?` to mean that the option is optional (though its use is discouraged).
+	* Use `?` to mean that the option takes an optional argument. You
+	  probably want to use the `--stuck-long` mode to be able to
+	  unambiguously parse the optional argument.
 
 	* Use `*` to mean that this option should not be listed in the usage
 	  generated for the `-h` argument. It's shown for `--help-all` as
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c76b89dc5bcc..3e8c4cce060e 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -30,6 +30,8 @@ static int abbrev_ref;
 static int abbrev_ref_strict;
 static int output_sq;
 
+static int stuck_long;
+
 /*
  * Some arguments are relevant "revision" arguments,
  * others are about output format or other details.
@@ -320,12 +322,15 @@ static int parseopt_dump(const struct option *o, const char *arg, int unset)
 	struct strbuf *parsed = o->value;
 	if (unset)
 		strbuf_addf(parsed, " --no-%s", o->long_name);
-	else if (o->short_name)
+	else if (o->short_name && (o->long_name == NULL || !stuck_long))
 		strbuf_addf(parsed, " -%c", o->short_name);
 	else
 		strbuf_addf(parsed, " --%s", o->long_name);
 	if (arg) {
-		strbuf_addch(parsed, ' ');
+		if (!stuck_long)
+			strbuf_addch(parsed, ' ');
+		else if (o->long_name)
+			strbuf_addch(parsed, '=');
 		sq_quote_buf(parsed, arg);
 	}
 	return 0;
@@ -351,6 +356,8 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "stop-at-non-option", &stop_at_non_option,
 					N_("stop parsing after the "
 					   "first non-option argument")),
+		OPT_BOOL(0, "stuck-long", &stuck_long,
+					N_("output in stuck long form")),
 		OPT_END(),
 	};
 
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 13c88c9aae7f..83b1300cef91 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -12,9 +12,11 @@ usage: some-command [options] <args>...
     -h, --help            show the help
     --foo                 some nifty option --foo
     --bar ...             some cool option --bar with an argument
+    -b, --baz             a short and long option
 
 An option group Header
     -C[...]               option C with an optional argument
+    -d, --data[=...]      short and long option with an optional argument
 
 Extras
     --extra1              line above used to cause a segfault but no longer does
@@ -31,9 +33,11 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+b,baz     a short and long option
 
  An option group Header
 C?        option C with an optional argument
+d,data?   short and long option with an optional argument
 
 Extras
 extra1    line above used to cause a segfault but no longer does
@@ -45,16 +49,16 @@ test_expect_success 'test --parseopt help output' '
 '
 
 cat > expect <<EOF
-set -- --foo --bar 'ham' -- 'arg'
+set -- --foo --bar 'ham' -b -- 'arg'
 EOF
 
 test_expect_success 'test --parseopt' '
-	git rev-parse --parseopt -- --foo --bar=ham arg < optionspec > output &&
+	git rev-parse --parseopt -- --foo --bar=ham --baz arg < optionspec > output &&
 	test_cmp expect output
 '
 
 test_expect_success 'test --parseopt with mixed options and arguments' '
-	git rev-parse --parseopt -- --foo arg --bar=ham < optionspec > output &&
+	git rev-parse --parseopt -- --foo arg --bar=ham --baz < optionspec > output &&
 	test_cmp expect output
 '
 
@@ -99,4 +103,36 @@ test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option withou
 	test_cmp expect output
 '
 
+cat > expect <<EOF
+set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --stuck-long' '
+	git rev-parse --parseopt --stuck-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+set -- --data='' -C --baz -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --stuck-long and empty optional argument' '
+	git rev-parse --parseopt --stuck-long -- --data= arg -C -b <optionspec >output &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+set -- --data --baz -- 'arg'
+EOF
+
+test_expect_success 'test --parseopt --stuck-long and long option with unset optional argument' '
+	git rev-parse --parseopt --stuck-long -- --data arg -b <optionspec >output &&
+	test_cmp expect output
+'
+
+test_expect_success 'test --parseopt --stuck-long and short option with unset optional argument' '
+	git rev-parse --parseopt --stuck-long -- -d arg -b <optionspec >output &&
+	test_cmp expect output
+'
+
 test_done
-- 
1.8.4
Michael Haggerty· Oct 27, 2013, 05:45 UTC · re: Nicolas Vigier · lore

Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode

On 10/25/2013 10:18 PM, Nicolas Vigier wrote:
Show 14 quoted lines
> Add the --sticked-long option to output the options in their long form
> if available, and with their arguments sticked.
> 
> Contrary to the default form (non sticked arguments and short options),
> this can be parsed unambiguously when using options with optional
> arguments :
> 
>  - in the non sticked form, when an option is taking an optional argument
>    you cannot know if the next argument is its optional argument, or the
>    next option.
> 
>  - the long options form allows to differenciate between an empty argument
>    '--option=' and an unset argument '--option', which is not possible
>    with short options.
Trivial: s/differenciate/differentiate/
Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Nicolas Vigier· Oct 15, 2013, 23:53 UTC · re: Jonathan Nieder · lore

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

On Tue, 15 Oct 2013, Jonathan Nieder wrote:
Show 40 quoted lines
> Nicolas Vigier wrote:
> 
> >   $ 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.
> 
> Hmph.
> 
> As Junio mentioned, inserting '' would be a backward-incompatible
> change.  I don't think it's worth breaking existing scripts.  Probably
> what is needed is a new parseopt special character with the new
> semantics (e.g.,
> 
> 	Use ?? to mean the option has an optional argument.  If the
> 	option is supplied without its argument, the argument is taken
> 	to be ''.
> 
> or something like
> 
> 	Use ?<default> to mean the option has an optional argument.  If
> 	the option is supplied without its argument and <default> is
> 	nonempty, the argument is taken to be <default>.
> 
> ).
> 
> Sensible?

Yes, I think it's sensible. I will look at this and propose an other patch. Thanks.

← back to recent threads