Re: [PATCH] rev-parse --parseopt: option argument name hints
On 3/19/2014 11:46 AM, Junio C Hamano wrote: Ilya Bobyr 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 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 , 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
Ilya Bobyr 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 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 , but if I didn't please do so yourself. The changes to gitk and git-gui come to me via their own project repositories. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
On 3/12/2014 9:59 AM, Junio C Hamano wrote: Ilya Bobyr 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
Ilya Bobyr 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
On 3/11/2014 12:10 PM, Junio C Hamano wrote: Junio C Hamano 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? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
Junio C Hamano 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
Ilya Bobyr writes: > On 3/4/2014 11:22 AM, Junio C Hamano wrote: >> Ilya Bobyr writes: >>> @@ -333,6 +339,7 @@ h,helpshow 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 >> It probably is better not to have " named " 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 " part. After reading "Angle brackets are automatically given", seeing that the argument description has manually spelled "" gave me "Huh?". Without " named " 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 " 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[=]" 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: option argument name hints
On 3/4/2014 11:22 AM, Junio C Hamano wrote:
Ilya Bobyr 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
---
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:
-* SP+ help LF
+*? SP+ help LF
``::
@@ -313,6 +313,12 @@ Each line of options has this format:
* Use `!` to not make the corresponding negated long option available.
+``::
+ ``, if specified, is used as a name of the argument, if the
+ option takes an argument. `` 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,helpshow 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
It probably is better not to have " named " 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 " 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[=]" 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: (|,|)[=?]? SP+ */
+ /* parse: (|,|)[*=?!]*? SP+ */
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 t
Re: [PATCH] rev-parse --parseopt: option argument name hints
Ilya Bobyr 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
> ---
> 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:
>
>
> -* SP+ help LF
> +*? SP+ help LF
>
>
> ``::
> @@ -313,6 +313,12 @@ Each line of options has this format:
>
> * Use `!` to not make the corresponding negated long option available.
>
> +``::
> + ``, if specified, is used as a name of the argument, if the
> + option takes an argument. `` 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,helpshow 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
It probably is better not to have " named " 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[=]" 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: (|,|)[=?]? SP+ */
> + /* parse: (|,|)[*=?!]*? SP+ */
> 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 short option required argument
> +--bar2 long option required argument
> +-e, --fuz
> + short and long option required argument
> +-s[]short option optional argument
> +--long[=] long option optional argument
> +-g, --fluf[=] short and long option optional argument
> +--longest
> + 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 opt
[PATCH] rev-parse --parseopt: option argument name hints
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
---
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:
-* SP+ help LF
+*? SP+ help LF
``::
@@ -313,6 +313,12 @@ Each line of options has this format:
* Use `!` to not make the corresponding negated long option available.
+``::
+ ``, if specified, is used as a name of the argument, if the
+ option takes an argument. `` 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,helpshow 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
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: (|,|)[=?]? SP+ */
+ /* parse: (|,|)[*=?!]*? SP+ */
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 short option required argument
+--bar2 long option required argument
+-e, --fuz
+ short and long option required argument
+-s[]short option optional argument
+--long[=] long option optional argument
+-g, --fluf[=] short and long option optional argument
+--longest
+ 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?someshort 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
extra1line above used to cause a segfault but no longer does
EOF
--
1.7.9
--
To unsubscr
