Merge branch 'ib/scripted-parse-opt-better-hint-string'

The "rev-parse --parseopt" mode parsed the option specification
and the argument hint in a strange way to allow '=' and other
special characters in the option name while forbidding them from
the argument hint.  This made it impossible to define an option
like "--pair <key>=<value>" with "pair=key=value" specification,
which instead would have defined a "--pair=key <value>" option.

* ib/scripted-parse-opt-better-hint-string:
rev-parse --parseopt: allow [*=?!] in argument hints

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index c483100e75886e7326cecabcd66f1449e640365a..b6c6326cdc7bb42993cf16fcae0d492944720e51 100644 (file)
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -311,8 +311,8 @@ Each line of options has this format:
 `<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>`.
+       is necessary. May not contain any of the `<flags>` characters.
+       `h,help`, `dry-run` and `f` are examples of correct `<opt-spec>`.
 
 `<flags>`::
        `<flags>` are of `*`, `=`, `?` or `!`.

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index b6232390a649a74f9c188a6f9abe8ab9651191a1..02d747dcb1a318af98ec06a7ab2f4e489e7c2625 100644 (file)
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -371,6 +371,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
                                        N_("output in stuck long form")),
                OPT_END(),
        };
+       static const char * const flag_chars = "*=?!";
 
        struct strbuf sb = STRBUF_INIT, parsed = STRBUF_INIT;
        const char **usage = NULL;
@@ -400,7 +401,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
        /* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
        while (strbuf_getline(&sb, stdin, '\n') != EOF) {
                const char *s;
-               const char *end;
+               const char *help;
                struct option *o;
 
                if (!sb.len)
@@ -410,54 +411,56 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
                memset(opts + onb, 0, sizeof(opts[onb]));
 
                o = &opts[onb++];
-               s = strchr(sb.buf, ' ');
-               if (!s || *sb.buf == ' ') {
+               help = strchr(sb.buf, ' ');
+               if (!help || *sb.buf == ' ') {
                        o->type = OPTION_GROUP;
                        o->help = xstrdup(skipspaces(sb.buf));
                        continue;
                }
 
                o->type = OPTION_CALLBACK;
-               o->help = xstrdup(skipspaces(s));
+               o->help = xstrdup(skipspaces(help));
                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) {
+               /* name(s) */
+               s = strpbrk(sb.buf, flag_chars);
+               if (s == NULL)
+                       s = help;
+
+               if (s - sb.buf == 1) /* short option only */
+                       o->short_name = *sb.buf;
+               else if (sb.buf[1] != ',') /* long option only */
+                       o->long_name = xmemdupz(sb.buf, s - sb.buf);
+               else {
+                       o->short_name = *sb.buf;
+                       o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
+               }
+
+               /* flags */
+               while (s < help) {
+                       switch (*s++) {
                        case '=':
                                o->flags &= ~PARSE_OPT_NOARG;
-                               break;
+                               continue;
                        case '?':
                                o->flags &= ~PARSE_OPT_NOARG;
                                o->flags |= PARSE_OPT_OPTARG;
-                               break;
+                               continue;
                        case '!':
                                o->flags |= PARSE_OPT_NONEG;
-                               break;
+                               continue;
                        case '*':
                                o->flags |= PARSE_OPT_HIDDEN;
-                               break;
+                               continue;
                        }
+                       s--;
+                       break;
                }
 
-               if (s - sb.buf == 1) /* short option only */
-                       o->short_name = *sb.buf;
-               else if (sb.buf[1] != ',') /* long option only */
-                       o->long_name = xmemdupz(sb.buf, s - sb.buf);
-               else {
-                       o->short_name = *sb.buf;
-                       o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
-               }
+               if (s < help)
+                       o->argh = xmemdupz(s, help - s);
        }
        strbuf_release(&sb);
 

diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index ebe7c3b87c34755399495a375b9f4b7e0a45b1c1..310f93fd30ea51f94eaa2b4dd2e8d0e398783cba 100755 (executable)
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -3,7 +3,40 @@
 test_description='test git rev-parse --parseopt'
 . ./test-lib.sh
 
-sed -e 's/^|//' >expect <<\END_EXPECT
+test_expect_success 'setup optionspec' '
+       sed -e "s/^|//" >optionspec <<\EOF
+|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
+|b,baz     a short and long option
+|
+| An option group Header
+|C?        option C with an optional argument
+|d,data?   short and long option with an optional argument
+|
+| 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
+|pair=key=value  with an equals sign in the hint
+|short-hint=a    with a one symbol hint
+|
+|Extras
+|extra1    line above used to cause a segfault but no longer does
+EOF
+'
+
+test_expect_success 'test --parseopt help output' '
+       sed -e "s/^|//" >expect <<\END_EXPECT &&
 |cat <<\EOF
 |usage: some-command [options] <args>...
 |
@@ -28,49 +61,23 @@ sed -e 's/^|//' >expect <<\END_EXPECT
 |    -g, --fluf[=<path>]   short and long option optional argument
 |    --longest <very-long-argument-hint>
 |                          a very long argument hint
+|    --pair <key=value>    with an equals sign in the hint
+|    --short-hint <a>      with a one symbol hint
 |
 |Extras
 |    --extra1              line above used to cause a segfault but no longer does
 |
 |EOF
 END_EXPECT
-
-sed -e 's/^|//' >optionspec <<\EOF
-|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
-|b,baz     a short and long option
-|
-| An option group Header
-|C?        option C with an optional argument
-|d,data?   short and long option with an optional argument
-|
-| 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
-
-test_expect_success 'test --parseopt help output' '
        test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec &&
        test_i18ncmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.1' "
+       cat > expect <<EOF
 set -- --foo --bar 'ham' -b -- 'arg'
 EOF
+"
 
 test_expect_success 'test --parseopt' '
        git rev-parse --parseopt -- --foo --bar=ham --baz arg < optionspec > output &&
@@ -82,9 +89,11 @@ test_expect_success 'test --parseopt with mixed options and arguments' '
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.2' "
+       cat > expect <<EOF
 set -- --foo -- 'arg' '--bar=ham'
 EOF
+"
 
 test_expect_success 'test --parseopt with --' '
        git rev-parse --parseopt -- --foo -- arg --bar=ham < optionspec > output &&
@@ -96,54 +105,66 @@ test_expect_success 'test --parseopt --stop-at-non-option' '
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.3' "
+       cat > expect <<EOF
 set -- --foo -- '--' 'arg' '--bar=ham'
 EOF
+"
 
 test_expect_success 'test --parseopt --keep-dashdash' '
        git rev-parse --parseopt --keep-dashdash -- --foo -- arg --bar=ham < optionspec > output &&
        test_cmp expect output
 '
 
-cat >expect <<EOF
+test_expect_success 'setup expect.4' "
+       cat >expect <<EOF
 set -- --foo -- '--' 'arg' '--spam=ham'
 EOF
+"
 
 test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option with --' '
        git rev-parse --parseopt --keep-dashdash --stop-at-non-option -- --foo -- arg --spam=ham <optionspec >output &&
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.5' "
+       cat > expect <<EOF
 set -- --foo -- 'arg' '--spam=ham'
 EOF
+"
 
 test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option without --' '
        git rev-parse --parseopt --keep-dashdash --stop-at-non-option -- --foo arg --spam=ham <optionspec >output &&
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.6' "
+       cat > expect <<EOF
 set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'
 EOF
+"
 
 test_expect_success 'test --parseopt --stuck-long' '
        git rev-parse --parseopt --stuck-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.7' "
+       cat > expect <<EOF
 set -- --data='' -C --baz -- 'arg'
 EOF
+"
 
 test_expect_success 'test --parseopt --stuck-long and empty optional argument' '
        git rev-parse --parseopt --stuck-long -- --data= arg -C -b <optionspec >output &&
        test_cmp expect output
 '
 
-cat > expect <<EOF
+test_expect_success 'setup expect.8' "
+       cat > expect <<EOF
 set -- --data --baz -- 'arg'
 EOF
+"
 
 test_expect_success 'test --parseopt --stuck-long and long option with unset optional argument' '
        git rev-parse --parseopt --stuck-long -- --data arg -b <optionspec >output &&