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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/20/2014 11:38 AM, Junio C Hamano wrote:
Ilya Bobyr <ilya.bobyr@xxxxxxxxx> 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@xxxxxxxxx>
---
  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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]