[PATCH] rev-parse --parseopt: option argument name hints

[PATCH] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

Built-in commands can specify names for option arguments, that are shown
when usage text is generated for the command.  sh based commands should
be able to do the same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 Documentation/git-rev-parse.txt |   11 +++++++++--
 builtin/rev-parse.c             |   17 ++++++++++++++++-
 t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 ------------
-<opt_spec><flags>* SP+ help LF
+<opt_spec><flags>*<argh>? SP+ help LF
 ------------
 
 `<opt_spec>`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
 	* Use `!` to not make the corresponding negated long option available.
 
+`<argh>`::
+	`<argh>`, if specified, is used as a name of the argument, if the
+	option takes an argument. `<argh>` is terminated by the first
+	whitespace. Angle braces are added automatically.  Underscore symbols
+	are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,7 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+baz=arg   another cool option --baz with an argument named <arg>
 
   An option group Header
 C?        option C with an optional argument"
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index aaeb611..83a769e 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		usage[unb++] = strbuf_detach(&sb, NULL);
 	}
 
-	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
+	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
 	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
 		const char *s;
+		const char *argh;
 		struct option *o;
 
 		if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		o->value = &parsed;
 		o->flags = PARSE_OPT_NOARG;
 		o->callback = &parseopt_dump;
+
+		/* Possible argument name hint */
+		argh = s;
+		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
+			--s;
+		if (s != sb.buf && s != argh) {
+			char *a;
+			o->argh = a = xmemdupz(s, argh - s);
+			while (a = strchr(a, '_'))
+				*a = ' ';
+		}
+		if (s == sb.buf)
+			s = argh;
+
 		while (s > sb.buf && strchr("*=?!", s[-1])) {
 			switch (*--s) {
 			case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
     -C[...]               option C with an optional argument
     -d, --data[=...]      short and long option with an optional argument
 
+Argument hints
+    -b <arg>              short option required argument
+    --bar2 <arg>          long option required argument
+    -e, --fuz <with spaces>
+                          short and long option required argument
+    -s[<some>]            short option optional argument
+    --long[=<data>]       long option optional argument
+    -g, --fluf[=<path>]   short and long option optional argument
+    --longest <a very long argument hint>
+                          a very long argument hint
+
 Extras
     --extra1              line above used to cause a segfault but no longer does
 
@@ -39,6 +50,15 @@ b,baz     a short and long option
 C?        option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg     short option required argument
+bar2=arg  long option required argument
+e,fuz=with_spaces  short and long option required argument
+s?some    short option optional argument
+long?data long option optional argument
+g,fluf?path     short and long option optional argument
+longest=a_very_long_argument_hint  a very long argument hint
+
 Extras
 extra1    line above used to cause a segfault but no longer does
 EOF
-- 
1.7.9

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> Built-in commands can specify names for option arguments, that are shown
> when usage text is generated for the command.  sh based commands should
> be able to do the same.
>
> Option argument name hint is any text that comes after [*=?!] after the
> argument name up to the first whitespace.  Underscores are replaced with
> whitespace.  It is unlikely that an underscore would be useful in the
> hint text.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  Documentation/git-rev-parse.txt |   11 +++++++++--
>  builtin/rev-parse.c             |   17 ++++++++++++++++-
>  t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
>  3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 0d2cdcd..4cb6e02 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -284,13 +284,13 @@ Input Format
>  
>  'git rev-parse --parseopt' input format is fully text based. It has two parts,
>  separated by a line that contains only `--`. The lines before the separator
> -(should be more than one) are used for the usage.
> +(could be more than one) are used for the usage.

Good spotting.  I think the original author meant to say there
should be at least one line to serve as the usage string, so
updating it to "should be one or more" may be more accurate, but
"could be more than one" would also work.

>  The lines after the separator describe the options.
>  
>  Each line of options has this format:
>  
>  ------------
> -<opt_spec><flags>* SP+ help LF
> +<opt_spec><flags>*<argh>? SP+ help LF
>  ------------
>  
>  `<opt_spec>`::
> @@ -313,6 +313,12 @@ Each line of options has this format:
>  
>  	* Use `!` to not make the corresponding negated long option available.
>  
> +`<argh>`::
> +	`<argh>`, if specified, is used as a name of the argument, if the
> +	option takes an argument. `<argh>` is terminated by the first
> +	whitespace. Angle braces are added automatically.  Underscore symbols
> +	are replaced with spaces.

I had a hard time understanding this "Angle brackets are added
automatically" one (obviously nobody wants extra angle brackets
added around option arguments given by the user), until I looked at
the addition of the test to realize that this description is only
about how it appears in the help output.  The description needs to
be clarified to avoid confusion.

> @@ -333,6 +339,7 @@ h,help    show the help
>  
>  foo       some nifty option --foo
>  bar=      some cool option --bar with an argument
> +baz=arg   another cool option --baz with an argument named <arg>

It probably is better not to have " named <arg>" at the end here, as
that gives an apparent-but-false contradiction with the "Angle
brackets are added *automatically*" and confuse readers.  At least,
it confused _this_ reader.

After the "eval" in the existing example to parse the "$@" argument
list in this part of the documentation, it may be a good idea to say
something like:

	The above command, when "$@" is "--help", produces the
	following help output:

	... sample output here ...

to show the actual output.  That way, we can illustrate how input
"baz?arg description of baz" is turned into "--baz[=<arg>]" output
clearly (yes, I am suggesting to use '?' in the new example, not '='
whose usage is already shown in the existing example).

> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index aaeb611..83a769e 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
>  		usage[unb++] = strbuf_detach(&sb, NULL);
>  	}
>  
> -	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
> +	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
>  	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
>  		const char *s;
> +		const char *argh;

Let's spell that variable name out, e.g. arg_hint or something.

> diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
> index 83b1300..bf0db05 100755
> --- a/t/t1502-rev-parse-parseopt.sh
> +++ b/t/t1502-rev-parse-parseopt.sh
> @@ -18,6 +18,17 @@ An option group Header
>      -C[...]               option C with an optional argument
>      -d, --data[=...]      short and long option with an optional argument
>  
> +Argument hints
> +    -b <arg>              short option required argument
> +    --bar2 <arg>          long option required argument
> +    -e, --fuz <with spaces>
> +                          short and long option required argument
> +    -s[<some>]            short option optional argument
> +    --long[=<data>]       long option optional argument
> +    -g, --fluf[=<path>]   short and long option optional argument
> +    --longest <a very long argument hint>
> +                          a very long argument hint
> +
>  Extras
>      --extra1              line above used to cause a segfault but no longer does
>  
> @@ -39,6 +50,15 @@ b,baz     a short and long option
>  C?        option C with an optional argument
>  d,data?   short and long option with an optional argument
>  
> + Argument hints
> +b=arg     short option required argument
> +bar2=arg  long option required argument
> +e,fuz=with_spaces  short and long option required argument
> +s?some    short option optional argument
> +long?data long option optional argument
> +g,fluf?path     short and long option optional argument
> +longest=a_very_long_argument_hint  a very long argument hint
> +

Nice.

Thanks.

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/4/2014 11:22 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> Built-in commands can specify names for option arguments, that are shown
>> when usage text is generated for the command.  sh based commands should
>> be able to do the same.
>>
>> Option argument name hint is any text that comes after [*=?!] after the
>> argument name up to the first whitespace.  Underscores are replaced with
>> whitespace.  It is unlikely that an underscore would be useful in the
>> hint text.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>>   Documentation/git-rev-parse.txt |   11 +++++++++--
>>   builtin/rev-parse.c             |   17 ++++++++++++++++-
>>   t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
>>   3 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
>> index 0d2cdcd..4cb6e02 100644
>> --- a/Documentation/git-rev-parse.txt
>> +++ b/Documentation/git-rev-parse.txt
>> @@ -284,13 +284,13 @@ Input Format
>>   
>>   'git rev-parse --parseopt' input format is fully text based. It has two parts,
>>   separated by a line that contains only `--`. The lines before the separator
>> -(should be more than one) are used for the usage.
>> +(could be more than one) are used for the usage.
> Good spotting.  I think the original author meant to say there
> should be at least one line to serve as the usage string, so
> updating it to "should be one or more" may be more accurate, but
> "could be more than one" would also work.

Changed to "should be one or more".

>>   The lines after the separator describe the options.
>>   
>>   Each line of options has this format:
>>   
>>   ------------
>> -<opt_spec><flags>* SP+ help LF
>> +<opt_spec><flags>*<argh>? SP+ help LF
>>   ------------
>>   
>>   `<opt_spec>`::
>> @@ -313,6 +313,12 @@ Each line of options has this format:
>>   
>>   	* Use `!` to not make the corresponding negated long option available.
>>   
>> +`<argh>`::
>> +	`<argh>`, if specified, is used as a name of the argument, if the
>> +	option takes an argument. `<argh>` is terminated by the first
>> +	whitespace. Angle braces are added automatically.  Underscore symbols
>> +	are replaced with spaces.
> I had a hard time understanding this "Angle brackets are added
> automatically" one (obviously nobody wants extra angle brackets
> added around option arguments given by the user), until I looked at
> the addition of the test to realize that this description is only
> about how it appears in the help output.  The description needs to
> be clarified to avoid confusion.

I've reworded some of the sentences.  I think it is better now.  Let me 
know what you think.

>> @@ -333,6 +339,7 @@ h,help    show the help
>>   
>>   foo       some nifty option --foo
>>   bar=      some cool option --bar with an argument
>> +baz=arg   another cool option --baz with an argument named <arg>
> It probably is better not to have " named <arg>" at the end here, as
> that gives an apparent-but-false contradiction with the "Angle
> brackets are added *automatically*" and confuse readers.  At least,
> it confused _this_ reader.

I am not sure I understand what is confusing here.  But I removed the " 
named <arg>" part.
If there would be an example, I think, it is easy to understand how it 
works.

> After the "eval" in the existing example to parse the "$@" argument
> list in this part of the documentation, it may be a good idea to say
> something like:
>
> 	The above command, when "$@" is "--help", produces the
> 	following help output:
>
> 	... sample output here ...
>
> to show the actual output.  That way, we can illustrate how input
> "baz?arg description of baz" is turned into "--baz[=<arg>]" output
> clearly (yes, I am suggesting to use '?' in the new example, not '='
> whose usage is already shown in the existing example).

Documentation on the whole argument parsing is quite short, so, I 
though, adding an example just to show how usage is generated would look 
like I am trying to make this feature look important than it is :)

I've added another section that shows usage text generated for the 
example specification.

>> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
>> index aaeb611..83a769e 100644
>> --- a/builtin/rev-parse.c
>> +++ b/builtin/rev-parse.c
>> @@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
>>   		usage[unb++] = strbuf_detach(&sb, NULL);
>>   	}
>>   
>> -	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
>> +	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
>>   	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
>>   		const char *s;
>> +		const char *argh;
> Let's spell that variable name out, e.g. arg_hint or something.

I was looking at the surrounding code for some style guidance, but most 
local variables have short names like "s", "o", "onb", "osz", "sb".
There are some that are longer.  So I was quite unsure here.
At the same time the target structure that holds the option description 
calls this string "argh".
Also, this is not really an "arg_hint" but the end of it.  Argument name 
is actually between s and argh, if there is some.
Considering all that, "argh" seemed like an OK name.

I've renamed it to "end".  It is used to remember possible end of the 
argument name in just one paragraph of code.
Comments above the paragraph clarifies what is been extracted.
Should there be another "parameter" in the option specification, the 
same variable could be used while parsing that one as well.

Let me know if you what that to be "arg_hint", or "arg_hint_end", or 
anything else.

>> diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
>> index 83b1300..bf0db05 100755
>> --- a/t/t1502-rev-parse-parseopt.sh
>> +++ b/t/t1502-rev-parse-parseopt.sh
>> @@ -18,6 +18,17 @@ An option group Header
>>       -C[...]               option C with an optional argument
>>       -d, --data[=...]      short and long option with an optional argument
>>   
>> +Argument hints
>> +    -b <arg>              short option required argument
>> +    --bar2 <arg>          long option required argument
>> +    -e, --fuz <with spaces>
>> +                          short and long option required argument
>> +    -s[<some>]            short option optional argument
>> +    --long[=<data>]       long option optional argument
>> +    -g, --fluf[=<path>]   short and long option optional argument
>> +    --longest <a very long argument hint>
>> +                          a very long argument hint
>> +
>>   Extras
>>       --extra1              line above used to cause a segfault but no longer does
>>   
>> @@ -39,6 +50,15 @@ b,baz     a short and long option
>>   C?        option C with an optional argument
>>   d,data?   short and long option with an optional argument
>>   
>> + Argument hints
>> +b=arg     short option required argument
>> +bar2=arg  long option required argument
>> +e,fuz=with_spaces  short and long option required argument
>> +s?some    short option optional argument
>> +long?data long option optional argument
>> +g,fluf?path     short and long option optional argument
>> +longest=a_very_long_argument_hint  a very long argument hint
>> +
> Nice.

Thanks :)

P.S. Patch comes in the next message.

[PATCH v2] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 Documentation/git-rev-parse.txt |   11 +++++++++--
 builtin/rev-parse.c             |   17 ++++++++++++++++-
 t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 ------------
-<opt_spec><flags>* SP+ help LF
+<opt_spec><flags>*<argh>? SP+ help LF
 ------------
 
 `<opt_spec>`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
 	* Use `!` to not make the corresponding negated long option available.
 
+`<argh>`::
+	`<argh>`, if specified, is used as a name of the argument, if the
+	option takes an argument. `<argh>` is terminated by the first
+	whitespace. Angle braces are added automatically.  Underscore symbols
+	are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,7 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+baz=arg   another cool option --baz with an argument named <arg>
 
   An option group Header
 C?        option C with an optional argument"
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..7a58404 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		usage[unb++] = strbuf_detach(&sb, NULL);
 	}
 
-	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
+	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
 	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
 		const char *s;
+		const char *argh;
 		struct option *o;
 
 		if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		o->value = &parsed;
 		o->flags = PARSE_OPT_NOARG;
 		o->callback = &parseopt_dump;
+
+		/* Possible argument name hint */
+		argh = s;
+		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
+			--s;
+		if (s != sb.buf && s != argh) {
+			char *a;
+			o->argh = a = xmemdupz(s, argh - s);
+			while (a = strchr(a, '_'))
+				*a = ' ';
+		}
+		if (s == sb.buf)
+			s = argh;
+
 		while (s > sb.buf && strchr("*=?!", s[-1])) {
 			switch (*--s) {
 			case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
     -C[...]               option C with an optional argument
     -d, --data[=...]      short and long option with an optional argument
 
+Argument hints
+    -b <arg>              short option required argument
+    --bar2 <arg>          long option required argument
+    -e, --fuz <with spaces>
+                          short and long option required argument
+    -s[<some>]            short option optional argument
+    --long[=<data>]       long option optional argument
+    -g, --fluf[=<path>]   short and long option optional argument
+    --longest <a very long argument hint>
+                          a very long argument hint
+
 Extras
     --extra1              line above used to cause a segfault but no longer does
 
@@ -39,6 +50,15 @@ b,baz     a short and long option
 C?        option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg     short option required argument
+bar2=arg  long option required argument
+e,fuz=with_spaces  short and long option required argument
+s?some    short option optional argument
+long?data long option optional argument
+g,fluf?path     short and long option optional argument
+longest=a_very_long_argument_hint  a very long argument hint
+
 Extras
 extra1    line above used to cause a segfault but no longer does
 EOF
-- 
1.7.9

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> On 3/4/2014 11:22 AM, Junio C Hamano wrote:
>> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>>> @@ -333,6 +339,7 @@ h,help    show the help
>>>     foo       some nifty option --foo
>>>   bar=      some cool option --bar with an argument
>>> +baz=arg   another cool option --baz with an argument named <arg>
>> It probably is better not to have " named <arg>" at the end here, as
>> that gives an apparent-but-false contradiction with the "Angle
>> brackets are added *automatically*" and confuse readers.  At least,
>> it confused _this_ reader.
>
> I am not sure I understand what is confusing here.  But I removed the
> " named <arg>" part.

After reading "Angle brackets are automatically given", seeing that
the argument description has manually spelled "<arg>" gave me "Huh?".

Without " named <arg>" there is no such confusion.

> If there would be an example, I think, it is easy to understand how it
> works.

Of course.  That is why I suggested to do without " named <arg>"
part---I didn't mean to suggest not to add the example.  I also
think that you can demonstrate something other than '=' (whose usage
is already shown with "bar=" above) here as well, but I think we can
go either way.

>> After the "eval" in the existing example to parse the "$@" argument
>> list in this part of the documentation, it may be a good idea to say
>> something like:
>>
>> 	The above command, when "$@" is "--help", produces the
>> 	following help output:
>>
>> 	... sample output here ...
>>
>> to show the actual output.  That way, we can illustrate how input
>> "baz?arg description of baz" is turned into "--baz[=<arg>]" output
>> clearly (yes, I am suggesting to use '?' in the new example, not '='
>> whose usage is already shown in the existing example).
>
> Documentation on the whole argument parsing is quite short, so, I
> though, adding an example just to show how usage is generated would
> look like I am trying to make this feature look important than it is
> :)

You already are by saying the "Angle brackets are automatic", aren't
you?

> At the same time the target structure that holds the option
> description calls this string "argh".

OK, that is fine, then (I'd prefer a field name not to sound like
arrrgh, but that is an entirely different topic).

> I've renamed it to "end".  It is used to remember possible end of the
> argument name in just one paragraph of code.

Sounds good.

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> Documentation on the whole argument parsing is quite short, so, I
>> though, adding an example just to show how usage is generated would
>> look like I am trying to make this feature look important than it is
>> :)
>
> You already are by saying the "Angle brackets are automatic", aren't
> you?

That is, among the things --parseopt mode does, the above stresses
what happens _only_ when it emits help text for items that use this
feature.

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/11/2014 12:10 PM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Documentation on the whole argument parsing is quite short, so, I
>>> though, adding an example just to show how usage is generated would
>>> look like I am trying to make this feature look important than it is
>>> :)
>> You already are by saying the "Angle brackets are automatic", aren't
>> you?
> That is, among the things --parseopt mode does, the above stresses
> what happens _only_ when it emits help text for items that use this
> feature.

`argh' is used only while help text is generated.  So, there seems to be 
no way around it :)
I was talking not about the automatic addition of angle brackets, but 
about the documentation on `argh' in general.
The section where I've added a paragraph, is not specific to the help 
output, but describes --parseopt.
I though that an example just to describe `argh' while useful would look 
a bit disproportional, compared to the amount of text on --parseopt.

But now that I've added a "Usage text" section to looks quite in place.

I just realized that the second patch I sent did not contain the 
changes.  Sorry about - I will resend it.

I was also wondering about the possible next step(s).
If you like the patch will you just take it from the maillist and it 
would appear in the next "What's cooking in git.git"?
Or the process is different?

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> I though that an example just to describe `argh' while useful would
> look a bit disproportional, compared to the amount of text on
> --parseopt.
>
> But now that I've added a "Usage text" section to looks quite in place.

Good thinking.

> I was also wondering about the possible next step(s).  If you like
> the patch will you just take it from the maillist and it would
> appear in the next "What's cooking in git.git"?  Or the process is
> different?

It goes more like this:

 - A topic that is in a good enough shape to be discussed and moved
   forward is given its own topic branch and then merged to 'pu', so
   that we do not forget.  The topic enters "What's cooking" at this
   stage.

 - Discussion on the topic continues on the list, and the topic can
   be replaced or built upon while it is still on 'pu' to polish it
   further.

   . We may see a grave issue with the change and may discard it
     from 'pu'.  

   . We may see a period of inaction after issues are pointed out
     and/or improvements are suggested, which would cause the topic
     marked as stalled; this may cause it to be eventually discarded
     as "abandoned" if nobody cares deeply enough.

 - After a while, when it seems that we, collectively as the Git
   development circle, agree that we would eventually want that
   change in a released version in some future (not necessarily in
   the upcoming release), the topic is merged to 'next', which is
   the branch Git developers are expected to run in their daily
   lives.

    . We may see some updates that builds on the patches merged to
      'next' so far to fix late issues discovered.

    . We may see a grave issue with the change and may have to
      revert & discard it from 'next'.

 - After a while, when the topic proves to be solid, it is merged to
   'master', in preparation for the upcoming release.

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/12/2014 9:59 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> I though that an example just to describe `argh' while useful would
>> look a bit disproportional, compared to the amount of text on
>> --parseopt.
>>
>> But now that I've added a "Usage text" section to looks quite in place.
> Good thinking.
>
>> I was also wondering about the possible next step(s).  If you like
>> the patch will you just take it from the maillist and it would
>> appear in the next "What's cooking in git.git"?  Or the process is
>> different?
> It goes more like this:

Thank you for all the details.

>   - A topic that is in a good enough shape to be discussed and moved
>     forward is given its own topic branch and then merged to 'pu', so
>     that we do not forget.  The topic enters "What's cooking" at this
>     stage.

I can not find this particular patch in the latest "What's cooking" email.
Is there something I can do?
It does not seems like there is a lot of interest, so I am not sure 
there will be a lot of discussion.
It is a minor fix and considering the number of the emails on the list, 
I do not unexpected this kind of stuff to be very popular.
But it seems like a valid improvement to me.
Maybe I am missing something?

Same questions about this one:

     [PATCH] gitk: replace SHA1 entry field on keyboard paste
     http://www.mail-archive.com/git@vger.kernel.org/msg45040.html

I think they are more or less similar, except that the second one is 
just trivial.

>   - Discussion on the topic continues on the list, and the topic can
>     be replaced or built upon while it is still on 'pu' to polish it
>     further.
>
>     . We may see a grave issue with the change and may discard it
>       from 'pu'.
>
>     . We may see a period of inaction after issues are pointed out
>       and/or improvements are suggested, which would cause the topic
>       marked as stalled; this may cause it to be eventually discarded
>       as "abandoned" if nobody cares deeply enough.
>
>   - After a while, when it seems that we, collectively as the Git
>     development circle, agree that we would eventually want that
>     change in a released version in some future (not necessarily in
>     the upcoming release), the topic is merged to 'next', which is
>     the branch Git developers are expected to run in their daily
>     lives.
>
>      . We may see some updates that builds on the patches merged to
>        'next' so far to fix late issues discovered.
>
>      . We may see a grave issue with the change and may have to
>        revert & discard it from 'next'.
>
>   - After a while, when the topic proves to be solid, it is merged to
>     'master', in preparation for the upcoming release.
>

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> I can not find this particular patch in the latest "What's cooking" email.
> Is there something I can do?

IIRC, I think I was waiting for the version with a new "Usage text"
section to the documentation you alluded to in this exchange
($gmane/243924):

    Ilya Bobyr <ilya.bobyr@gmail.com> writes:

    > On 3/11/2014 12:10 PM, Junio C Hamano wrote:
    >>
    >>>> Documentation on the whole argument parsing is quite short, so,...
    > ...
    > I though that an example just to describe `argh' while useful would
    > look a bit disproportional, compared to the amount of text on
    > --parseopt.
    >
    > But now that I've added a "Usage text" section to looks quite in place.
    >
    > I just realized that the second patch I sent did not contain the
    > changes.  Sorry about - I will resend it.

> It does not seems like there is a lot of interest, so I am not sure
> there will be a lot of discussion.
> It is a minor fix and considering the number of the emails on the
> list, I do not unexpected this kind of stuff to be very popular.
> But it seems like a valid improvement to me.
> Maybe I am missing something?

You did the right thing by sending a reminder message with a pointer
to help others locate the original (like the one I am responding
to), as nobody can keep up with a busy list traffic.

> Same questions about this one:
>
>     [PATCH] gitk: replace SHA1 entry field on keyboard paste
>     http://www.mail-archive.com/git@vger.kernel.org/msg45040.html
>
> I think they are more or less similar, except that the second one is
> just trivial.

I do not remember if I forwarded the patch to the area maintainer
Paul Mackerras <paulus@samba.org>, but if I didn't please do so
yourself.  The changes to gitk and git-gui come to me via their own
project repositories.

Re: [PATCH] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/19/2014 11:46 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> I can not find this particular patch in the latest "What's cooking" email.
>> Is there something I can do?
> IIRC, I think I was waiting for the version with a new "Usage text"
> section to the documentation you alluded to in this exchange
> ($gmane/243924):
>
>      Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>      > On 3/11/2014 12:10 PM, Junio C Hamano wrote:
>      >>
>      >>>> Documentation on the whole argument parsing is quite short, so,...
>      > ...
>      > I though that an example just to describe `argh' while useful would
>      > look a bit disproportional, compared to the amount of text on
>      > --parseopt.
>      >
>      > But now that I've added a "Usage text" section to looks quite in place.
>      >
>      > I just realized that the second patch I sent did not contain the
>      > changes.  Sorry about - I will resend it.

Oh %)
I did sent it in the next minute.  And did receive a copy myself.
But it seems it never showed up in the list.
I am still a bit new to the tools, maybe I did something wrong.
Will try again :)

>> It does not seems like there is a lot of interest, so I am not sure
>> there will be a lot of discussion.
>> It is a minor fix and considering the number of the emails on the
>> list, I do not unexpected this kind of stuff to be very popular.
>> But it seems like a valid improvement to me.
>> Maybe I am missing something?
> You did the right thing by sending a reminder message with a pointer
> to help others locate the original (like the one I am responding
> to), as nobody can keep up with a busy list traffic.

Thanks :)

>> Same questions about this one:
>>
>>      [PATCH] gitk: replace SHA1 entry field on keyboard paste
>>      http://www.mail-archive.com/git@vger.kernel.org/msg45040.html
>>
>> I think they are more or less similar, except that the second one is
>> just trivial.
> I do not remember if I forwarded the patch to the area maintainer
> Paul Mackerras <paulus@samba.org>, but if I didn't please do so
> yourself.  The changes to gitk and git-gui come to me via their own
> project repositories.

You did and I even replied with additional details, that I should have 
included as a cover letter.
I can see those messages in the web archive.
It seems that Paul Mackerras gitk repository is here: 
git://ozlabs.org/~paulus/gitk.git
At least that is what is online.  I do not see the change in there.
I will remind him about it.

[PATCH v3] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 Changed according to the last comments.  Added "Usage text" paragraph in the
 documentation and updated variable names.

 Documentation/git-rev-parse.txt |   34 ++++++++++++++++++++++++++++++++--
 builtin/rev-parse.c             |   17 ++++++++++++++++-
 t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..b8aabc9 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(should be one or more) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 ------------
-<opt_spec><flags>* SP+ help LF
+<opt_spec><flags>*<arg_hint>? SP+ help LF
 ------------
 
 `<opt_spec>`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
 	* Use `!` to not make the corresponding negated long option available.
 
+`<arg_hint>`::
+	`<arg_hing>`, if specified, is used as a name of the argument in the
+	help output, for options that take arguments. `<arg_hint>` is
+	terminated by the first whitespace. When output the name is shown in
+	angle braces.  Underscore symbols are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,8 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+baz=arg   another cool option --baz with a named argument
+qux?path  qux may take a path argument but has meaning by itself
 
   An option group Header
 C?        option C with an optional argument"
@@ -340,6 +348,28 @@ C?        option C with an optional argument"
 eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
 ------------
 
+
+Usage text
+~~~~~~~~~~
+
+When "$@" is "-h" or "--help" the above example would produce the following
+usage text:
+
+------------
+usage: some-command [options] <args>...
+
+    some-command does foo and bar!
+
+    -h, --help            show the help
+    --foo                 some nifty option --foo
+    --bar ...             some cool option --bar with an argument
+    --bar <arg>           another cool option --baz with a named argument
+    --qux[=<path>]        qux may take a path argument but has meaning by itself
+
+An option group Header
+    -C[...]               option C with an optional argument
+------------
+
 SQ-QUOTE
 --------
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..a4c9fdf 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		usage[unb++] = strbuf_detach(&sb, NULL);
 	}
 
-	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
+	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
 	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
 		const char *s;
+		const char *end;
 		struct option *o;
 
 		if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		o->value = &parsed;
 		o->flags = PARSE_OPT_NOARG;
 		o->callback = &parseopt_dump;
+
+		/* Possible argument name hint */
+		end = s;
+		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
+			--s;
+		if (s != sb.buf && s != end) {
+			char *a;
+			o->argh = a = xmemdupz(s, end - s);
+			while (a = strchr(a, '_'))
+				*a = ' ';
+		}
+		if (s == sb.buf)
+			s = end;
+
 		while (s > sb.buf && strchr("*=?!", s[-1])) {
 			switch (*--s) {
 			case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
     -C[...]               option C with an optional argument
     -d, --data[=...]      short and long option with an optional argument
 
+Argument hints
+    -b <arg>              short option required argument
+    --bar2 <arg>          long option required argument
+    -e, --fuz <with spaces>
+                          short and long option required argument
+    -s[<some>]            short option optional argument
+    --long[=<data>]       long option optional argument
+    -g, --fluf[=<path>]   short and long option optional argument
+    --longest <a very long argument hint>
+                          a very long argument hint
+
 Extras
     --extra1              line above used to cause a segfault but no longer does
 
@@ -39,6 +50,15 @@ b,baz     a short and long option
 C?        option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg     short option required argument
+bar2=arg  long option required argument
+e,fuz=with_spaces  short and long option required argument
+s?some    short option optional argument
+long?data long option optional argument
+g,fluf?path     short and long option optional argument
+longest=a_very_long_argument_hint  a very long argument hint
+
 Extras
 extra1    line above used to cause a segfault but no longer does
 EOF
-- 
1.7.9

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

> Built-in commands can specify names for option arguments when usage text
> is generated for a command.  sh based commands should be able to do the
> same.
>
> Option argument name hint is any text that comes after [*=?!] after the
> argument name up to the first whitespace.  Underscores are replaced with
> whitespace.  It is unlikely that an underscore would be useful in the
> hint text.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  Changed according to the last comments.  Added "Usage text" paragraph in the
>  documentation and updated variable names.
>
>  Documentation/git-rev-parse.txt |   34 ++++++++++++++++++++++++++++++++--
>  builtin/rev-parse.c             |   17 ++++++++++++++++-
>  t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
>  3 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 0d2cdcd..b8aabc9 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -284,13 +284,13 @@ Input Format
>  
>  'git rev-parse --parseopt' input format is fully text based. It has two parts,
>  separated by a line that contains only `--`. The lines before the separator
> -(should be more than one) are used for the usage.
> +(should be one or more) are used for the usage.
>  The lines after the separator describe the options.
>  
>  Each line of options has this format:
>  
>  ------------
> -<opt_spec><flags>* SP+ help LF
> +<opt_spec><flags>*<arg_hint>? SP+ help LF
>  ------------
>  
>  `<opt_spec>`::
> @@ -313,6 +313,12 @@ Each line of options has this format:
>  
>  	* Use `!` to not make the corresponding negated long option available.
>  
> +`<arg_hint>`::
> +	`<arg_hing>`, if specified, is used as a name of the argument in the
> +	help output, for options that take arguments. `<arg_hint>` is
> +	terminated by the first whitespace. When output the name is shown in
> +	angle braces.  Underscore symbols are replaced with spaces.

The last part is troubling (and sounds not very sane).  Do we do
such a munging anywhere else, or is it just here?  If the latter I'd
prefer not to see such a hack.

> @@ -333,6 +339,8 @@ h,help    show the help
>  
>  foo       some nifty option --foo
>  bar=      some cool option --bar with an argument
> +baz=arg   another cool option --baz with a named argument
> +qux?path  qux may take a path argument but has meaning by itself
>  
>    An option group Header
>  C?        option C with an optional argument"
> @@ -340,6 +348,28 @@ C?        option C with an optional argument"
>  eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
>  ------------
>  
> +
> +Usage text
> +~~~~~~~~~~
> +
> +When "$@" is "-h" or "--help" the above example would produce the following
> +usage text:

Sounds like a good idea to add this; all the above arguments inside
double quotes should be typeset `as-typed`, though.

> @@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
>  		o->value = &parsed;
>  		o->flags = PARSE_OPT_NOARG;
>  		o->callback = &parseopt_dump;
> +
> +		/* Possible argument name hint */
> +		end = s;
> +		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
> +			--s;
> +		if (s != sb.buf && s != end) {
> +			char *a;
> +			o->argh = a = xmemdupz(s, end - s);
> +			while (a = strchr(a, '_'))
> +				*a = ' ';

... and without the "underscore" munging, we do not have to allocate
a new piece of memory, either.

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/20/2014 11:38 AM, Junio C Hamano wrote:
> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>
>> Built-in commands can specify names for option arguments when usage text
>> is generated for a command.  sh based commands should be able to do the
>> same.
>>
>> Option argument name hint is any text that comes after [*=?!] after the
>> argument name up to the first whitespace.  Underscores are replaced with
>> whitespace.  It is unlikely that an underscore would be useful in the
>> hint text.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>>   Changed according to the last comments.  Added "Usage text" paragraph in the
>>   documentation and updated variable names.
>>
>>   Documentation/git-rev-parse.txt |   34 ++++++++++++++++++++++++++++++++--
>>   builtin/rev-parse.c             |   17 ++++++++++++++++-
>>   t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
>>   3 files changed, 68 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
>> index 0d2cdcd..b8aabc9 100644
>> --- a/Documentation/git-rev-parse.txt
>> +++ b/Documentation/git-rev-parse.txt
>> @@ -284,13 +284,13 @@ Input Format
>>   
>>   'git rev-parse --parseopt' input format is fully text based. It has two parts,
>>   separated by a line that contains only `--`. The lines before the separator
>> -(should be more than one) are used for the usage.
>> +(should be one or more) are used for the usage.
>>   The lines after the separator describe the options.
>>   
>>   Each line of options has this format:
>>   
>>   ------------
>> -<opt_spec><flags>* SP+ help LF
>> +<opt_spec><flags>*<arg_hint>? SP+ help LF
>>   ------------
>>   
>>   `<opt_spec>`::
>> @@ -313,6 +313,12 @@ Each line of options has this format:
>>   
>>   	* Use `!` to not make the corresponding negated long option available.
>>   
>> +`<arg_hint>`::
>> +	`<arg_hing>`, if specified, is used as a name of the argument in the
>> +	help output, for options that take arguments. `<arg_hint>` is
>> +	terminated by the first whitespace. When output the name is shown in
>> +	angle braces.  Underscore symbols are replaced with spaces.
> The last part is troubling (and sounds not very sane).  Do we do
> such a munging anywhere else, or is it just here?  If the latter I'd
> prefer not to see such a hack.

The following commands have spaces in argument names in the "-h" output 
for one or two arguments:
   * clone
   * commit
   * merge

A number of commands use dashes to separate words in arguments names.

"git notes" is the only command that uses an underscore in one argument 
name.

At the moment space is used to separate option specification from the 
help line.  As argument name hint is part of the option specification it 
ends at the first space.

It seems a bit unfair if sh based commands would not be able to use 
spaces while the build-in ones can.
As underscores are not used in the UI (at least that was my impression 
so far), I thought that to be a good option.

Do you think a different kind of escaping should be used? Backslashes?
Or no spaces?

>> @@ -333,6 +339,8 @@ h,help    show the help
>>   
>>   foo       some nifty option --foo
>>   bar=      some cool option --bar with an argument
>> +baz=arg   another cool option --baz with a named argument
>> +qux?path  qux may take a path argument but has meaning by itself
>>   
>>     An option group Header
>>   C?        option C with an optional argument"
>> @@ -340,6 +348,28 @@ C?        option C with an optional argument"
>>   eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
>>   ------------
>>   
>> +
>> +Usage text
>> +~~~~~~~~~~
>> +
>> +When "$@" is "-h" or "--help" the above example would produce the following
>> +usage text:
> Sounds like a good idea to add this; all the above arguments inside
> double quotes should be typeset `as-typed`, though.

Thanks, I will fix that.

>> @@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
>>   		o->value = &parsed;
>>   		o->flags = PARSE_OPT_NOARG;
>>   		o->callback = &parseopt_dump;
>> +
>> +		/* Possible argument name hint */
>> +		end = s;
>> +		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
>> +			--s;
>> +		if (s != sb.buf && s != end) {
>> +			char *a;
>> +			o->argh = a = xmemdupz(s, end - s);
>> +			while (a = strchr(a, '_'))
>> +				*a = ' ';
> ... and without the "underscore" munging, we do not have to allocate
> a new piece of memory, either.

We would have to do it any way to have the string zero terminated.
The list of arguments that holds the lines been parsed is "const char *".

But I do not think this is an argument to be considered when designing 
the user interface :)

Never the less if there is a way not to allocate extra memory that I am 
missing - let me know, I would remove the allocation.

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/20/2014 4:19 PM, Ilya Bobyr wrote:
> On 3/20/2014 11:38 AM, Junio C Hamano wrote:
>> Ilya Bobyr <ilya.bobyr@gmail.com> writes:
>>
>>> [...]
>>>     ------------
>>> -<opt_spec><flags>* SP+ help LF
>>> +<opt_spec><flags>*<arg_hint>? SP+ help LF
>>>   ------------
>>>     `<opt_spec>`::
>>> @@ -313,6 +313,12 @@ Each line of options has this format:
>>>         * Use `!` to not make the corresponding negated long option
>>> available.
>>>   +`<arg_hint>`::
>>> +    `<arg_hing>`, if specified, is used as a name of the argument
>>> in the
>>> +    help output, for options that take arguments. `<arg_hint>` is
>>> +    terminated by the first whitespace. When output the name is
>>> shown in
>>> +    angle braces.  Underscore symbols are replaced with spaces.
>> The last part is troubling (and sounds not very sane).  Do we do
>> such a munging anywhere else, or is it just here?  If the latter I'd
>> prefer not to see such a hack.
>
> The following commands have spaces in argument names in the "-h"
> output for one or two arguments:
>   * clone
s/clone/checkout/
>   * commit
>   * merge
>
> A number of commands use dashes to separate words in arguments names.
>
> "git notes" is the only command that uses an underscore in one
> argument name.
>
> At the moment space is used to separate option specification from the
> help line.  As argument name hint is part of the option specification
> it ends at the first space.
>
> It seems a bit unfair if sh based commands would not be able to use
> spaces while the build-in ones can.
> As underscores are not used in the UI (at least that was my impression
> so far), I thought that to be a good option.
>
> Do you think a different kind of escaping should be used? Backslashes?
> Or no spaces?

"git merge" also uses equals sign in one of the argument names.  That
would not be possible for sh based commands either.

As a lot of commands are using dashes instead of spaces, so not
supporting spaces is probably fine.

Another option I can think of is to use (or just allow) angle brackets
around argument names.  That would look similar to the actual output.
"git shortlog" has some punctuation in an argument name, which braces
would make a bit easier to read.
This is how an option description would look like then:

OPTION_SPEC="\
...
S,gpg-sign?<key id>     GPG sign commit from "commit"
w?<w[,i1[,i2]]>         "shortlog" option with a complicated argument name
...
"

If there is interest in this, I could code it up and post for discussion.

> [...]

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Junio C Hamano]

Ilya Bobyr <ilya.bobyr@gmail.com> writes:

>>> +	`<arg_hing>`, if specified, is used as a name of the argument in the
>>> +	help output, for options that take arguments. `<arg_hint>` is
>>> +	terminated by the first whitespace. When output the name is shown in
>>> +	angle braces.  Underscore symbols are replaced with spaces.
>> The last part is troubling (and sounds not very sane).  Do we do
>> such a munging anywhere else, or is it just here?  If the latter I'd
>> prefer not to see such a hack.
>
> The following commands have spaces in argument names in the "-h"
> output for one or two arguments:
>   * clone
>   * commit
>   * merge
>
> A number of commands use dashes to separate words in arguments names.

That was not what I asked.  I was asking if there is a precedent to
use "you cannot have underscores in hint; they will be turned into
spaces" quoting convention.  I do not think of any (we either do a
backslash-quote, c-quote inside dq-pair, or %20, depending on the
context).

Personally, because these "hints" are not even hints (they are more
like placeholders for value that makes it easier to refer to in the
description of an option [*1*]), I wouldn't shed tears if scripted
Porcelains cannot use a space in the argh.  In fact, it probably
makes the result harder to read and format more funnily if you had a
space in the argh string, be it in a subcommand implemented in C or
in a scripted Porcelain.

"An optional argh is terminated by a whitespace" is perfectly fine,
and by doing so we do not have to worry about having to introduce a
new quoting convention like you did, which is a big plus.


[Footnote]

*1* Perhaps like this:

	--gpg-sign[=<key-id>]
        	Sign (with the key specified with <key-id>)

[PATCH v4] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.

Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
---
 Fixed "arg_hing" typo, decoration for `-h` and `--help` and removed
 underscore replacement for the hint text.

 Documentation/git-rev-parse.txt |   34 ++++++++++++++++++++++++++++++++--
 builtin/rev-parse.c             |   13 ++++++++++++-
 t/t1502-rev-parse-parseopt.sh   |   20 ++++++++++++++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..be85023 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(should be one or more) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 ------------
-<opt_spec><flags>* SP+ help LF
+<opt_spec><flags>*<arg_hint>? SP+ help LF
 ------------
 
 `<opt_spec>`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
 	* Use `!` to not make the corresponding negated long option available.
 
+`<arg_hint>`::
+	`<arg_hint>`, if specified, is used as a name of the argument in the
+	help output, for options that take arguments. `<arg_hint>` is
+	terminated by the first whitespace. When you need to use space in the
+	argument hint use dash instead.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,8 @@ h,help    show the help
 
 foo       some nifty option --foo
 bar=      some cool option --bar with an argument
+baz=arg   another cool option --baz with a named argument
+qux?path  qux may take a path argument but has meaning by itself
 
   An option group Header
 C?        option C with an optional argument"
@@ -340,6 +348,28 @@ C?        option C with an optional argument"
 eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
 ------------
 
+
+Usage text
+~~~~~~~~~~
+
+When "$@" is `-h` or `--help` the above example would produce the following
+usage text:
+
+------------
+usage: some-command [options] <args>...
+
+    some-command does foo and bar!
+
+    -h, --help            show the help
+    --foo                 some nifty option --foo
+    --bar ...             some cool option --bar with an argument
+    --bar <arg>           another cool option --baz with a named argument
+    --qux[=<path>]        qux may take a path argument but has meaning by itself
+
+An option group Header
+    -C[...]               option C with an optional argument
+------------
+
 SQ-QUOTE
 --------
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..1a6122d 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		usage[unb++] = strbuf_detach(&sb, NULL);
 	}
 
-	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
+	/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
 	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
 		const char *s;
+		const char *end;
 		struct option *o;
 
 		if (!sb.len)
@@ -419,6 +420,16 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		o->value = &parsed;
 		o->flags = PARSE_OPT_NOARG;
 		o->callback = &parseopt_dump;
+
+		/* Possible argument name hint */
+		end = s;
+		while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
+			--s;
+		if (s != sb.buf && s != end)
+			o->argh = xmemdupz(s, end - s);
+		if (s == sb.buf)
+			s = end;
+
 		while (s > sb.buf && strchr("*=?!", s[-1])) {
 			switch (*--s) {
 			case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..e3c6f02 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
     -C[...]               option C with an optional argument
     -d, --data[=...]      short and long option with an optional argument
 
+Argument hints
+    -b <arg>              short option required argument
+    --bar2 <arg>          long option required argument
+    -e, --fuz <with-space>
+                          short and long option required argument
+    -s[<some>]            short option optional argument
+    --long[=<data>]       long option optional argument
+    -g, --fluf[=<path>]   short and long option optional argument
+    --longest <very-long-argument-hint>
+                          a very long argument hint
+
 Extras
     --extra1              line above used to cause a segfault but no longer does
 
@@ -39,6 +50,15 @@ b,baz     a short and long option
 C?        option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg     short option required argument
+bar2=arg  long option required argument
+e,fuz=with-space  short and long option required argument
+s?some    short option optional argument
+long?data long option optional argument
+g,fluf?path     short and long option optional argument
+longest=very-long-argument-hint  a very long argument hint
+
 Extras
 extra1    line above used to cause a segfault but no longer does
 EOF
-- 
1.7.9

[PATCH 0/3] Parse-options: spell multi-word placeholders with dashes

[Junio C Hamano]

This is a follow-up to Ilya's 4th round of letting scripted
porcelains to give argv-help to their users with their command line
option parser based on "rev-parse --parseopt".

While reviewing the patch, we found that a few options to the
built-in commands were described with an argv-help (the placeholder
for an option parameter, e.g. "key-id" in "--gpg-sign <key-id>")
that has multiple words to decribe a single entity, spelling these
multiple words separated in spaces.  It is more customary to spell a
multi-word parameter with dashes, and the first patch in series is
about making it so.

During the course of the development of the first patch, I needed a
mechanical way to catch existing offenders; the last patch teaches
the parse-options API implementation to find argv-help strings that
contain SP or underscore.

There is one glitch, though.  "update-index --cacheinfo" option
takes THREE parameters: mode, sha1, and path.  Because a command
line option that takes multiple options is very unusual, the second
patch introduces a new syntax to pass these three items as a single
parameter to "--cacheinfo" option, which brings our command line
argument convention more uniform and consistent.  We however cannot
deprecate or remove the traditional syntax, so it is still kept as
an alternative "backward compatibility" syntax.

Junio C Hamano (3):
  parse-options: multi-word argh should use dash to separate words
  update-index: teach --cacheinfo a new syntax "mode,sha1,path"
  parse-options: make sure argh string does not have SP or _

 Documentation/git-cherry-pick.txt  |  6 +++---
 Documentation/git-commit.txt       |  2 +-
 Documentation/git-merge.txt        |  2 +-
 Documentation/git-notes.txt        |  2 +-
 Documentation/git-rev-parse.txt    | 16 ++++++++--------
 Documentation/git-revert.txt       |  6 +++---
 Documentation/git-update-index.txt |  8 ++++++--
 builtin/checkout.c                 |  2 +-
 builtin/commit.c                   |  2 +-
 builtin/merge.c                    |  2 +-
 builtin/notes.c                    |  2 +-
 builtin/revert.c                   |  2 +-
 builtin/tag.c                      |  2 +-
 builtin/update-index.c             | 34 +++++++++++++++++++++++++++++++---
 parse-options.c                    |  3 +++
 parse-options.h                    |  2 +-
 t/t2107-update-index-basic.sh      | 13 +++++++++++++
 17 files changed, 77 insertions(+), 29 deletions(-)

-- 
1.9.1-471-gcccbd8b

[PATCH 1/3] parse-options: multi-word argh should use dash to separate words

[Junio C Hamano]

"When you need to use space, use dash" is a strange way to say that
you must not use a space.  Because it is more common for the command
line descriptions to use dashed-multi-words, you do not even want to
use spaces in these places.  Rephrase the documentation to avoid
this strangeness.

Fix a few existing multi-word argument help strings, i.e.

 - GPG key-ids given to -S/--gpg-sign are "key-id";
 - Refs used for storing notes are "notes-ref"; and
 - Expiry timestamps given to --expire are "expiry-date".

and update the corresponding documentation pages.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-cherry-pick.txt |  6 +++---
 Documentation/git-commit.txt      |  2 +-
 Documentation/git-merge.txt       |  2 +-
 Documentation/git-notes.txt       |  2 +-
 Documentation/git-rev-parse.txt   | 16 ++++++++--------
 Documentation/git-revert.txt      |  6 +++---
 builtin/checkout.c                |  2 +-
 builtin/commit.c                  |  2 +-
 builtin/merge.c                   |  2 +-
 builtin/notes.c                   |  2 +-
 builtin/revert.c                  |  2 +-
 builtin/tag.c                     |  2 +-
 parse-options.h                   |  2 +-
 13 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index f1e6b2f..1c03c79 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
-		  [-S[<keyid>]] <commit>...
+		  [-S[<key-id>]] <commit>...
 'git cherry-pick' --continue
 'git cherry-pick' --quit
 'git cherry-pick' --abort
@@ -101,8 +101,8 @@ effect to your index in a row.
 --signoff::
 	Add Signed-off-by line at the end of the commit message.
 
--S[<keyid>]::
---gpg-sign[=<keyid>]::
+-S[<key-id>]::
+--gpg-sign[=<key-id>]::
 	GPG-sign commits.
 
 --ff::
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 7c42e9c..d58758f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<key-id>]] [--] [<file>...]
 
 DESCRIPTION
 -----------
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4395459..a3c1fa3 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[-s <strategy>] [-X <strategy-option>] [-S[<key-id>]]
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 84bb0fe..310f0a5 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 'git notes' append [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
 'git notes' edit [<object>]
 'git notes' show [<object>]
-'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
+'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
 'git notes' merge --commit [-v | -q]
 'git notes' merge --abort [-v | -q]
 'git notes' remove [--ignore-missing] [--stdin] [<object>...]
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index e05e6b3..c452f33 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -290,14 +290,14 @@ The lines after the separator describe the options.
 Each line of options has this format:
 
 ------------
-<opt_spec><flags>*<arg_hint>? SP+ help LF
+<opt-spec><flags>*<arg-hint>? SP+ help LF
 ------------
 
-`<opt_spec>`::
+`<opt-spec>`::
 	its format is the short option character, then the long option name
 	separated by a comma. Both parts are not required, though at least one
 	is necessary. `h,help`, `dry-run` and `f` are all three correct
-	`<opt_spec>`.
+	`<opt-spec>`.
 
 `<flags>`::
 	`<flags>` are of `*`, `=`, `?` or `!`.
@@ -313,11 +313,11 @@ Each line of options has this format:
 
 	* Use `!` to not make the corresponding negated long option available.
 
-`<arg_hint>`::
-	`<arg_hint>`, if specified, is used as a name of the argument in the
-	help output, for options that take arguments. `<arg_hint>` is
-	terminated by the first whitespace. When you need to use space in the
-	argument hint use dash instead.
+`<arg-hint>`::
+	`<arg-hint>`, if specified, is used as a name of the argument in the
+	help output, for options that take arguments. `<arg-hint>` is
+	terminated by the first whitespace.  It is customary to use a
+	dash to separate words in a multi-word argument hint.
 
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 9eb83f0..cceb5f2 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -8,7 +8,7 @@ git-revert - Revert some existing commits
 SYNOPSIS
 --------
 [verse]
-'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<keyid>]] <commit>...
+'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[<key-id>]] <commit>...
 'git revert' --continue
 'git revert' --quit
 'git revert' --abort
@@ -80,8 +80,8 @@ more details.
 This is useful when reverting more than one commits'
 effect to your index in a row.
 
--S[<keyid>]::
---gpg-sign[=<keyid>]::
+-S[<key-id>]::
+--gpg-sign[=<key-id>]::
 	GPG-sign commits.
 
 -s::
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ada51fa..a0e72d2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1095,7 +1095,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "detach", &opts.force_detach, N_("detach the HEAD at named commit")),
 		OPT_SET_INT('t', "track",  &opts.track, N_("set upstream info for new branch"),
 			BRANCH_TRACK_EXPLICIT),
-		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new branch"), N_("new unparented branch")),
+		OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
 		OPT_SET_INT('2', "ours", &opts.writeout_stage, N_("checkout our version for unmerged files"),
 			    2),
 		OPT_SET_INT('3', "theirs", &opts.writeout_stage, N_("checkout their version for unmerged files"),
diff --git a/builtin/commit.c b/builtin/commit.c
index 3783bca..96bf762 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1472,7 +1472,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
 		OPT_STRING(0, "cleanup", &cleanup_arg, N_("default"), N_("how to strip spaces and #comments from message")),
 		OPT_BOOL(0, "status", &include_status, N_("include status in commit message template")),
-		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
+		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 		/* end commit message options */
 
diff --git a/builtin/merge.c b/builtin/merge.c
index f0cf120..2a144e1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -220,7 +220,7 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
 	OPT_SET_INT(0, "progress", &show_progress, N_("force progress reporting"), 1),
-	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key id"),
+	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_END()
diff --git a/builtin/notes.c b/builtin/notes.c
index bb89930..39c8573 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -939,7 +939,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	int result;
 	const char *override_notes_ref = NULL;
 	struct option options[] = {
-		OPT_STRING(0, "ref", &override_notes_ref, N_("notes_ref"),
+		OPT_STRING(0, "ref", &override_notes_ref, N_("notes-ref"),
 			   N_("use notes from <notes_ref>")),
 		OPT_END()
 	};
diff --git a/builtin/revert.c b/builtin/revert.c
index 065d88d..f9ed5bd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -89,7 +89,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
 		OPT_CALLBACK('X', "strategy-option", &opts, N_("option"),
 			N_("option for merge strategy"), option_parse_x),
-		{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key id"),
+		{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
 		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 		OPT_END(),
 		OPT_END(),
diff --git a/builtin/tag.c b/builtin/tag.c
index 40356e3..6c7c6bd 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -513,7 +513,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed tag")),
 		OPT_STRING(0, "cleanup", &cleanup_arg, N_("mode"),
 			N_("how to strip spaces and #comments from message")),
-		OPT_STRING('u', "local-user", &keyid, N_("key id"),
+		OPT_STRING('u', "local-user", &keyid, N_("key-id"),
 					N_("use another key to sign the tag")),
 		OPT__FORCE(&force, N_("replace the tag if exists")),
 		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
diff --git a/parse-options.h b/parse-options.h
index d670cb9..8fa02dc 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -143,7 +143,7 @@ struct option {
 	{ OPTION_CALLBACK, (s), (l), (v), N_("time"),(h), 0,	\
 	  parse_opt_approxidate_cb }
 #define OPT_EXPIRY_DATE(s, l, v, h) \
-	{ OPTION_CALLBACK, (s), (l), (v), N_("expiry date"),(h), 0,	\
+	{ OPTION_CALLBACK, (s), (l), (v), N_("expiry-date"),(h), 0,	\
 	  parse_opt_expiry_date_cb }
 #define OPT_CALLBACK(s, l, v, a, h, f) \
 	{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
-- 
1.9.1-471-gcccbd8b

[PATCH 2/3] update-index: teach --cacheinfo a new syntax "mode,sha1,path"

[Junio C Hamano]

The "--cacheinfo" option is unusual in that it takes three option
parameters.  An option with an optional parameter is bad enough.  An
option with multiple parameters is simply insane.

Introduce a new syntax that takes these three things concatenated
together with a comma, which makes the command line syntax more
uniform across subcommands, while retaining the traditional syntax
for backward compatiblity.

If we were designing the "update-index" subcommand from scratch
today, it may probably have made sense to make this option (and
possibly others) a command mode option that does not take any option
parameter (hence no need for arg-help).  But we do not live in such
an ideal world, and as far as I can tell, the command still supports
(and must support) mixed command modes in a single invocation, e.g.

    $ git update-index path1 --add path2 \
        --cacheinfo 100644 $(git hash-object --stdin -w <path3) path3 \
	path4

must make sure path1 is already in the index and update all of these
four paths.  So this is probably as far as we can go to fix this issue
without risking to break people's existing scripts.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-update-index.txt |  8 ++++++--
 builtin/update-index.c             | 34 +++++++++++++++++++++++++++++++---
 t/t2107-update-index-basic.sh      | 13 +++++++++++++
 3 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index e0a8702..d6de4a0 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git update-index'
 	     [--add] [--remove | --force-remove] [--replace]
 	     [--refresh] [-q] [--unmerged] [--ignore-missing]
-	     [(--cacheinfo <mode> <object> <file>)...]
+	     [(--cacheinfo <mode>,<object>,<file>)...]
 	     [--chmod=(+|-)x]
 	     [--[no-]assume-unchanged]
 	     [--[no-]skip-worktree]
@@ -68,8 +68,12 @@ OPTIONS
 --ignore-missing::
 	Ignores missing files during a --refresh
 
+--cacheinfo <mode>,<object>,<path>::
 --cacheinfo <mode> <object> <path>::
-	Directly insert the specified info into the index.
+	Directly insert the specified info into the index.  For
+	backward compatibility, you can also give these three
+	arguments as three separate parameters, but new users are
+	encouraged to use a single-parameter form.
 
 --index-info::
         Read index information from stdin.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index d12ad95..ba54e19 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -629,14 +629,42 @@ static int resolve_undo_clear_callback(const struct option *opt,
 	return 0;
 }
 
+static int parse_new_style_cacheinfo(const char *arg,
+				     unsigned int *mode,
+				     unsigned char sha1[],
+				     const char **path)
+{
+	unsigned long ul;
+	char *endp;
+
+	errno = 0;
+	ul = strtoul(arg, &endp, 8);
+	if (errno || endp == arg || *endp != ',' || (unsigned int) ul != ul)
+		return -1; /* not a new-style cacheinfo */
+	*mode = ul;
+	endp++;
+	if (get_sha1_hex(endp, sha1) || endp[40] != ',')
+		return -1;
+	*path = endp + 41;
+	return 0;
+}
+
 static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
 				const struct option *opt, int unset)
 {
 	unsigned char sha1[20];
 	unsigned int mode;
+	const char *path;
 
+	if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, sha1, &path)) {
+		if (add_cacheinfo(mode, sha1, path, 0))
+			die("git update-index: --cacheinfo cannot add %s", path);
+		ctx->argv++;
+		ctx->argc--;
+		return 0;
+	}
 	if (ctx->argc <= 3)
-		return error("option 'cacheinfo' expects three arguments");
+		return error("option 'cacheinfo' expects <mode>,<sha1>,<path>");
 	if (strtoul_ui(*++ctx->argv, 8, &mode) ||
 	    get_sha1_hex(*++ctx->argv, sha1) ||
 	    add_cacheinfo(mode, sha1, *++ctx->argv, 0))
@@ -740,9 +768,9 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_NOARG | PARSE_OPT_NONEG,
 			really_refresh_callback},
 		{OPTION_LOWLEVEL_CALLBACK, 0, "cacheinfo", NULL,
-			N_("<mode> <object> <path>"),
+			N_("<mode>,<object>,<path>"),
 			N_("add the specified entry to the index"),
-			PARSE_OPT_NOARG |	/* disallow --cacheinfo=<mode> form */
+			PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
 			PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
 			(parse_opt_cb *) cacheinfo_callback},
 		{OPTION_CALLBACK, 0, "chmod", &set_executable_bit, N_("(+/-)x"),
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index a6405d3..fe2fb17 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -48,4 +48,17 @@ test_expect_success '--cacheinfo does not accept gitlink null sha1' '
 	test_cmp expect actual
 '
 
+test_expect_success '--cacheinfo mode,sha1,path (new syntax)' '
+	echo content >file &&
+	git hash-object -w --stdin <file >expect &&
+
+	git update-index --add --cacheinfo 100644 "$(cat expect)" file &&
+	git rev-parse :file >actual &&
+	test_cmp expect actual &&
+
+	git update-index --add --cacheinfo "100644,$(cat expect),elif" &&
+	git rev-parse :elif >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.9.1-471-gcccbd8b

[PATCH 3/3] parse-options: make sure argh string does not have SP or _

[Junio C Hamano]

We encourage to spell an argument hint that consists of multiple
words as a single-token separated with dashes.  In order to help
catching violations added by new callers of parse-options, make sure
argh does not contain SP or _ when the code validates the option
definitions.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 parse-options.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/parse-options.c b/parse-options.c
index a5fa0b8..c81d3a0 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -375,6 +375,9 @@ static void parse_options_check(const struct option *opts)
 		default:
 			; /* ok. (usually accepts an argument) */
 		}
+		if (opts->argh &&
+		    strcspn(opts->argh, " _") != strlen(opts->argh))
+			err |= optbug(opts, "multi-word argh should use dash to separate words");
 	}
 	if (err)
 		exit(128);
-- 
1.9.1-471-gcccbd8b

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Eric Sunshine]

On Thu, Mar 20, 2014 at 4:44 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
> Built-in commands can specify names for option arguments when usage text
> is generated for a command.  sh based commands should be able to do the
> same.
>
> Option argument name hint is any text that comes after [*=?!] after the
> argument name up to the first whitespace.  Underscores are replaced with
> whitespace.  It is unlikely that an underscore would be useful in the
> hint text.
>
> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
> ---
>  Changed according to the last comments.  Added "Usage text" paragraph in the
>  documentation and updated variable names.

As this is a high-traffic list, it can be difficult for reviewers to
remember all the comments regarding the previous version. It can help
a lot if you include a reference to the previous attempt, like this
[1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/243216/focus=243945

One more comment below...

> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 0d2cdcd..b8aabc9 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -313,6 +313,12 @@ Each line of options has this format:
>
>         * Use `!` to not make the corresponding negated long option available.
>
> +`<arg_hint>`::
> +       `<arg_hing>`, if specified, is used as a name of the argument in the

arg_hing?

> +       help output, for options that take arguments. `<arg_hint>` is
> +       terminated by the first whitespace. When output the name is shown in
> +       angle braces.  Underscore symbols are replaced with spaces.
> +
>  The remainder of the line, after stripping the spaces, is used
>  as the help associated to the option.

Re: [PATCH v3] rev-parse --parseopt: option argument name hints

[Ilya Bobyr]

On 3/20/2014 1:18 PM, Eric Sunshine wrote:
> On Thu, Mar 20, 2014 at 4:44 AM, Ilya Bobyr <ilya.bobyr@gmail.com> wrote:
>> Built-in commands can specify names for option arguments when usage text
>> is generated for a command.  sh based commands should be able to do the
>> same.
>>
>> Option argument name hint is any text that comes after [*=?!] after the
>> argument name up to the first whitespace.  Underscores are replaced with
>> whitespace.  It is unlikely that an underscore would be useful in the
>> hint text.
>>
>> Signed-off-by: Ilya Bobyr <ilya.bobyr@gmail.com>
>> ---
>>   Changed according to the last comments.  Added "Usage text" paragraph in the
>>   documentation and updated variable names.
> As this is a high-traffic list, it can be difficult for reviewers to
> remember all the comments regarding the previous version. It can help
> a lot if you include a reference to the previous attempt, like this
> [1].

Got it, thanks :)

>> [...]
>>
>> +`<arg_hint>`::
>> +       `<arg_hing>`, if specified, is used as a name of the argument in the
> arg_hing?

Will fix it in the next patch.