Skip to content

Commit 2bf2d81

Browse files
gitstergitster
committed
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
2 parents 3ecca88 + 2d893df commit 2bf2d81

File tree

3 files changed

+92
-68
lines changed

3 files changed

+92
-68
lines changed

‎Documentation/git-rev-parse.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ Each line of options has this format:
311311
`<opt-spec>`::
312312
its format is the short option character, then the long option name
313313
separated by a comma. Both parts are not required, though at least one
314-
is necessary. `h,help`, `dry-run` and `f` are all three correct
315-
`<opt-spec>`.
314+
is necessary. May not contain any of the `<flags>` characters.
315+
`h,help`, `dry-run` and `f` are examples of correct `<opt-spec>`.
316316

317317
`<flags>`::
318318
`<flags>` are of `*`, `=`, `?` or `!`.

‎builtin/rev-parse.c

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
371371
N_("output in stuck long form")),
372372
OPT_END(),
373373
};
374+
static const char * const flag_chars = "*=?!";
374375

375376
struct strbuf sb = STRBUF_INIT, parsed = STRBUF_INIT;
376377
const char **usage = NULL;
@@ -400,7 +401,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
400401
/* parse: (<short>|<short>,<long>|<long>)[*=?!]*<arghint>? SP+ <help> */
401402
while (strbuf_getline(&sb, stdin, '\n') != EOF) {
402403
const char *s;
403-
const char *end;
404+
const char *help;
404405
struct option *o;
405406

406407
if (!sb.len)
@@ -410,54 +411,56 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
410411
memset(opts + onb, 0, sizeof(opts[onb]));
411412

412413
o = &opts[onb++];
413-
s = strchr(sb.buf, ' ');
414-
if (!s || *sb.buf == ' ') {
414+
help = strchr(sb.buf, ' ');
415+
if (!help || *sb.buf == ' ') {
415416
o->type = OPTION_GROUP;
416417
o->help = xstrdup(skipspaces(sb.buf));
417418
continue;
418419
}
419420

420421
o->type = OPTION_CALLBACK;
421-
o->help = xstrdup(skipspaces(s));
422+
o->help = xstrdup(skipspaces(help));
422423
o->value = &parsed;
423424
o->flags = PARSE_OPT_NOARG;
424425
o->callback = &parseopt_dump;
425426

426-
/* Possible argument name hint */
427-
end = s;
428-
while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
429-
--s;
430-
if (s != sb.buf && s != end)
431-
o->argh = xmemdupz(s, end - s);
432-
if (s == sb.buf)
433-
s = end;
434-
435-
while (s > sb.buf && strchr("*=?!", s[-1])) {
436-
switch (*--s) {
427+
/* name(s) */
428+
s = strpbrk(sb.buf, flag_chars);
429+
if (s == NULL)
430+
s = help;
431+
432+
if (s - sb.buf == 1) /* short option only */
433+
o->short_name = *sb.buf;
434+
else if (sb.buf[1] != ',') /* long option only */
435+
o->long_name = xmemdupz(sb.buf, s - sb.buf);
436+
else {
437+
o->short_name = *sb.buf;
438+
o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
439+
}
440+
441+
/* flags */
442+
while (s < help) {
443+
switch (*s++) {
437444
case '=':
438445
o->flags &= ~PARSE_OPT_NOARG;
439-
break;
446+
continue;
440447
case '?':
441448
o->flags &= ~PARSE_OPT_NOARG;
442449
o->flags |= PARSE_OPT_OPTARG;
443-
break;
450+
continue;
444451
case '!':
445452
o->flags |= PARSE_OPT_NONEG;
446-
break;
453+
continue;
447454
case '*':
448455
o->flags |= PARSE_OPT_HIDDEN;
449-
break;
456+
continue;
450457
}
458+
s--;
459+
break;
451460
}
452461

453-
if (s - sb.buf == 1) /* short option only */
454-
o->short_name = *sb.buf;
455-
else if (sb.buf[1] != ',') /* long option only */
456-
o->long_name = xmemdupz(sb.buf, s - sb.buf);
457-
else {
458-
o->short_name = *sb.buf;
459-
o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
460-
}
462+
if (s < help)
463+
o->argh = xmemdupz(s, help - s);
461464
}
462465
strbuf_release(&sb);
463466

‎t/t1502-rev-parse-parseopt.sh

Lines changed: 60 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,40 @@
33
test_description='test git rev-parse --parseopt'
44
. ./test-lib.sh
55

6-
sed -e 's/^|//' >expect <<\END_EXPECT
6+
test_expect_success 'setup optionspec' '
7+
sed -e "s/^|//" >optionspec <<\EOF
8+
|some-command [options] <args>...
9+
|
10+
|some-command does foo and bar!
11+
|--
12+
|h,help show the help
13+
|
14+
|foo some nifty option --foo
15+
|bar= some cool option --bar with an argument
16+
|b,baz a short and long option
17+
|
18+
| An option group Header
19+
|C? option C with an optional argument
20+
|d,data? short and long option with an optional argument
21+
|
22+
| Argument hints
23+
|B=arg short option required argument
24+
|bar2=arg long option required argument
25+
|e,fuz=with-space short and long option required argument
26+
|s?some short option optional argument
27+
|long?data long option optional argument
28+
|g,fluf?path short and long option optional argument
29+
|longest=very-long-argument-hint a very long argument hint
30+
|pair=key=value with an equals sign in the hint
31+
|short-hint=a with a one symbol hint
32+
|
33+
|Extras
34+
|extra1 line above used to cause a segfault but no longer does
35+
EOF
36+
'
37+
38+
test_expect_success 'test --parseopt help output' '
39+
sed -e "s/^|//" >expect <<\END_EXPECT &&
740
|cat <<\EOF
841
|usage: some-command [options] <args>...
942
|
@@ -28,49 +61,23 @@ sed -e 's/^|//' >expect <<\END_EXPECT
2861
| -g, --fluf[=<path>] short and long option optional argument
2962
| --longest <very-long-argument-hint>
3063
| a very long argument hint
64+
| --pair <key=value> with an equals sign in the hint
65+
| --short-hint <a> with a one symbol hint
3166
|
3267
|Extras
3368
| --extra1 line above used to cause a segfault but no longer does
3469
|
3570
|EOF
3671
END_EXPECT
37-
38-
sed -e 's/^|//' >optionspec <<\EOF
39-
|some-command [options] <args>...
40-
|
41-
|some-command does foo and bar!
42-
|--
43-
|h,help show the help
44-
|
45-
|foo some nifty option --foo
46-
|bar= some cool option --bar with an argument
47-
|b,baz a short and long option
48-
|
49-
| An option group Header
50-
|C? option C with an optional argument
51-
|d,data? short and long option with an optional argument
52-
|
53-
| Argument hints
54-
|B=arg short option required argument
55-
|bar2=arg long option required argument
56-
|e,fuz=with-space short and long option required argument
57-
|s?some short option optional argument
58-
|long?data long option optional argument
59-
|g,fluf?path short and long option optional argument
60-
|longest=very-long-argument-hint a very long argument hint
61-
|
62-
|Extras
63-
|extra1 line above used to cause a segfault but no longer does
64-
EOF
65-
66-
test_expect_success 'test --parseopt help output' '
6772
test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec &&
6873
test_i18ncmp expect output
6974
'
7075

71-
cat > expect <<EOF
76+
test_expect_success 'setup expect.1' "
77+
cat > expect <<EOF
7278
set -- --foo --bar 'ham' -b -- 'arg'
7379
EOF
80+
"
7481

7582
test_expect_success 'test --parseopt' '
7683
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' '
8289
test_cmp expect output
8390
'
8491

85-
cat > expect <<EOF
92+
test_expect_success 'setup expect.2' "
93+
cat > expect <<EOF
8694
set -- --foo -- 'arg' '--bar=ham'
8795
EOF
96+
"
8897

8998
test_expect_success 'test --parseopt with --' '
9099
git rev-parse --parseopt -- --foo -- arg --bar=ham < optionspec > output &&
@@ -96,54 +105,66 @@ test_expect_success 'test --parseopt --stop-at-non-option' '
96105
test_cmp expect output
97106
'
98107

99-
cat > expect <<EOF
108+
test_expect_success 'setup expect.3' "
109+
cat > expect <<EOF
100110
set -- --foo -- '--' 'arg' '--bar=ham'
101111
EOF
112+
"
102113

103114
test_expect_success 'test --parseopt --keep-dashdash' '
104115
git rev-parse --parseopt --keep-dashdash -- --foo -- arg --bar=ham < optionspec > output &&
105116
test_cmp expect output
106117
'
107118

108-
cat >expect <<EOF
119+
test_expect_success 'setup expect.4' "
120+
cat >expect <<EOF
109121
set -- --foo -- '--' 'arg' '--spam=ham'
110122
EOF
123+
"
111124

112125
test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option with --' '
113126
git rev-parse --parseopt --keep-dashdash --stop-at-non-option -- --foo -- arg --spam=ham <optionspec >output &&
114127
test_cmp expect output
115128
'
116129

117-
cat > expect <<EOF
130+
test_expect_success 'setup expect.5' "
131+
cat > expect <<EOF
118132
set -- --foo -- 'arg' '--spam=ham'
119133
EOF
134+
"
120135

121136
test_expect_success 'test --parseopt --keep-dashdash --stop-at-non-option without --' '
122137
git rev-parse --parseopt --keep-dashdash --stop-at-non-option -- --foo arg --spam=ham <optionspec >output &&
123138
test_cmp expect output
124139
'
125140

126-
cat > expect <<EOF
141+
test_expect_success 'setup expect.6' "
142+
cat > expect <<EOF
127143
set -- --foo --bar='z' --baz -C'Z' --data='A' -- 'arg'
128144
EOF
145+
"
129146

130147
test_expect_success 'test --parseopt --stuck-long' '
131148
git rev-parse --parseopt --stuck-long -- --foo --bar=z -b arg -CZ -dA <optionspec >output &&
132149
test_cmp expect output
133150
'
134151

135-
cat > expect <<EOF
152+
test_expect_success 'setup expect.7' "
153+
cat > expect <<EOF
136154
set -- --data='' -C --baz -- 'arg'
137155
EOF
156+
"
138157

139158
test_expect_success 'test --parseopt --stuck-long and empty optional argument' '
140159
git rev-parse --parseopt --stuck-long -- --data= arg -C -b <optionspec >output &&
141160
test_cmp expect output
142161
'
143162

144-
cat > expect <<EOF
163+
test_expect_success 'setup expect.8' "
164+
cat > expect <<EOF
145165
set -- --data --baz -- 'arg'
146166
EOF
167+
"
147168

148169
test_expect_success 'test --parseopt --stuck-long and long option with unset optional argument' '
149170
git rev-parse --parseopt --stuck-long -- --data arg -b <optionspec >output &&

0 commit comments

Comments
 (0)