Skip to content

Commit 62c0fd4

Browse files
gitstergitster
committed
Merge branch 'ps/contains-id-error-message'
"git tag --contains no-such-commit" gave a full list of options after giving an error message. * ps/contains-id-error-message: parse-options: do not show usage upon invalid option value
2 parents 69d71ec + 3bb0923 commit 62c0fd4

File tree

8 files changed

+125
-14
lines changed

8 files changed

+125
-14
lines changed

‎builtin/blame.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
729729
for (;;) {
730730
switch (parse_options_step(&ctx, options, blame_opt_usage)) {
731731
case PARSE_OPT_HELP:
732+
case PARSE_OPT_ERROR:
732733
exit(129);
733734
case PARSE_OPT_DONE:
734735
if (ctx.argv[0])

‎builtin/shortlog.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
284284
for (;;) {
285285
switch (parse_options_step(&ctx, options, shortlog_usage)) {
286286
case PARSE_OPT_HELP:
287+
case PARSE_OPT_ERROR:
287288
exit(129);
288289
case PARSE_OPT_DONE:
289290
goto parse_done;

‎builtin/update-index.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
10591059
break;
10601060
switch (parseopt_state) {
10611061
case PARSE_OPT_HELP:
1062+
case PARSE_OPT_ERROR:
10621063
exit(129);
10631064
case PARSE_OPT_NON_OPTION:
10641065
case PARSE_OPT_DONE:

‎parse-options.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -317,14 +317,16 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
317317
return get_value(p, options, all_opts, flags ^ opt_flags);
318318
}
319319

320-
if (ambiguous_option)
321-
return error("Ambiguous option: %s "
320+
if (ambiguous_option) {
321+
error("Ambiguous option: %s "
322322
"(could be --%s%s or --%s%s)",
323323
arg,
324324
(ambiguous_flags & OPT_UNSET) ? "no-" : "",
325325
ambiguous_option->long_name,
326326
(abbrev_flags & OPT_UNSET) ? "no-" : "",
327327
abbrev_option->long_name);
328+
return -3;
329+
}
328330
if (abbrev_option)
329331
return get_value(p, abbrev_option, all_opts, abbrev_flags);
330332
return -2;
@@ -476,7 +478,6 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
476478
const char * const usagestr[])
477479
{
478480
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
479-
int err = 0;
480481

481482
/* we must reset ->opt, unknown short option leave it dangling */
482483
ctx->opt = NULL;
@@ -505,7 +506,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
505506
ctx->opt = arg + 1;
506507
switch (parse_short_opt(ctx, options)) {
507508
case -1:
508-
goto show_usage_error;
509+
return PARSE_OPT_ERROR;
509510
case -2:
510511
if (ctx->opt)
511512
check_typos(arg + 1, options);
@@ -518,7 +519,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
518519
while (ctx->opt) {
519520
switch (parse_short_opt(ctx, options)) {
520521
case -1:
521-
goto show_usage_error;
522+
return PARSE_OPT_ERROR;
522523
case -2:
523524
if (internal_help && *ctx->opt == 'h')
524525
goto show_usage;
@@ -550,9 +551,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
550551
goto show_usage;
551552
switch (parse_long_opt(ctx, arg + 2, options)) {
552553
case -1:
553-
goto show_usage_error;
554+
return PARSE_OPT_ERROR;
554555
case -2:
555556
goto unknown;
557+
case -3:
558+
goto show_usage;
556559
}
557560
continue;
558561
unknown:
@@ -563,10 +566,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
563566
}
564567
return PARSE_OPT_DONE;
565568

566-
show_usage_error:
567-
err = 1;
568569
show_usage:
569-
return usage_with_options_internal(ctx, usagestr, options, 0, err);
570+
return usage_with_options_internal(ctx, usagestr, options, 0, 0);
570571
}
571572

572573
int parse_options_end(struct parse_opt_ctx_t *ctx)
@@ -585,6 +586,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
585586
parse_options_start(&ctx, argc, argv, prefix, options, flags);
586587
switch (parse_options_step(&ctx, options, usagestr)) {
587588
case PARSE_OPT_HELP:
589+
case PARSE_OPT_ERROR:
588590
exit(129);
589591
case PARSE_OPT_NON_OPTION:
590592
case PARSE_OPT_DONE:

‎parse-options.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ enum {
200200
PARSE_OPT_HELP = -1,
201201
PARSE_OPT_DONE,
202202
PARSE_OPT_NON_OPTION,
203+
PARSE_OPT_ERROR,
203204
PARSE_OPT_UNKNOWN
204205
};
205206

‎t/t0040-parse-options.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '
291291
test_expect_success 'OPT_CALLBACK() and callback errors work' '
292292
test_must_fail test-parse-options --no-length >output 2>output.err &&
293293
test_i18ncmp expect output &&
294-
test_i18ncmp expect.err output.err
294+
test_must_be_empty output.err
295295
'
296296

297297
cat >expect <<\EOF

‎t/t0041-usage.sh

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
#!/bin/sh
2+
3+
test_description='Test commands behavior when given invalid argument value'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success 'setup ' '
8+
test_commit "v1.0"
9+
'
10+
11+
test_expect_success 'tag --contains <existent_tag>' '
12+
git tag --contains "v1.0" >actual 2>actual.err &&
13+
grep "v1.0" actual &&
14+
test_line_count = 0 actual.err
15+
'
16+
17+
test_expect_success 'tag --contains <inexistent_tag>' '
18+
test_must_fail git tag --contains "notag" >actual 2>actual.err &&
19+
test_line_count = 0 actual &&
20+
test_i18ngrep "error" actual.err &&
21+
test_i18ngrep ! "usage" actual.err
22+
'
23+
24+
test_expect_success 'tag --no-contains <existent_tag>' '
25+
git tag --no-contains "v1.0" >actual 2>actual.err &&
26+
test_line_count = 0 actual &&
27+
test_line_count = 0 actual.err
28+
'
29+
30+
test_expect_success 'tag --no-contains <inexistent_tag>' '
31+
test_must_fail git tag --no-contains "notag" >actual 2>actual.err &&
32+
test_line_count = 0 actual &&
33+
test_i18ngrep "error" actual.err &&
34+
test_i18ngrep ! "usage" actual.err
35+
'
36+
37+
test_expect_success 'tag usage error' '
38+
test_must_fail git tag --noopt >actual 2>actual.err &&
39+
test_line_count = 0 actual &&
40+
test_i18ngrep "usage" actual.err
41+
'
42+
43+
test_expect_success 'branch --contains <existent_commit>' '
44+
git branch --contains "master" >actual 2>actual.err &&
45+
test_i18ngrep "master" actual &&
46+
test_line_count = 0 actual.err
47+
'
48+
49+
test_expect_success 'branch --contains <inexistent_commit>' '
50+
test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
51+
test_line_count = 0 actual &&
52+
test_i18ngrep "error" actual.err &&
53+
test_i18ngrep ! "usage" actual.err
54+
'
55+
56+
test_expect_success 'branch --no-contains <existent_commit>' '
57+
git branch --no-contains "master" >actual 2>actual.err &&
58+
test_line_count = 0 actual &&
59+
test_line_count = 0 actual.err
60+
'
61+
62+
test_expect_success 'branch --no-contains <inexistent_commit>' '
63+
test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
64+
test_line_count = 0 actual &&
65+
test_i18ngrep "error" actual.err &&
66+
test_i18ngrep ! "usage" actual.err
67+
'
68+
69+
test_expect_success 'branch usage error' '
70+
test_must_fail git branch --noopt >actual 2>actual.err &&
71+
test_line_count = 0 actual &&
72+
test_i18ngrep "usage" actual.err
73+
'
74+
75+
test_expect_success 'for-each-ref --contains <existent_object>' '
76+
git for-each-ref --contains "master" >actual 2>actual.err &&
77+
test_line_count = 2 actual &&
78+
test_line_count = 0 actual.err
79+
'
80+
81+
test_expect_success 'for-each-ref --contains <inexistent_object>' '
82+
test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
83+
test_line_count = 0 actual &&
84+
test_i18ngrep "error" actual.err &&
85+
test_i18ngrep ! "usage" actual.err
86+
'
87+
88+
test_expect_success 'for-each-ref --no-contains <existent_object>' '
89+
git for-each-ref --no-contains "master" >actual 2>actual.err &&
90+
test_line_count = 0 actual &&
91+
test_line_count = 0 actual.err
92+
'
93+
94+
test_expect_success 'for-each-ref --no-contains <inexistent_object>' '
95+
test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
96+
test_line_count = 0 actual &&
97+
test_i18ngrep "error" actual.err &&
98+
test_i18ngrep ! "usage" actual.err
99+
'
100+
101+
test_expect_success 'for-each-ref usage error' '
102+
test_must_fail git for-each-ref --noopt >actual 2>actual.err &&
103+
test_line_count = 0 actual &&
104+
test_i18ngrep "usage" actual.err
105+
'
106+
107+
test_done

‎t/t3404-rebase-interactive.sh

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -927,10 +927,8 @@ test_expect_success 'rebase --exec works without -i ' '
927927
test_expect_success 'rebase -i --exec without <CMD>' '
928928
git reset --hard execute &&
929929
set_fake_editor &&
930-
test_must_fail git rebase -i --exec 2>tmp &&
931-
sed -e "1d" tmp >actual &&
932-
test_must_fail git rebase -h >expected &&
933-
test_cmp expected actual &&
930+
test_must_fail git rebase -i --exec 2>actual &&
931+
test_i18ngrep "requires a value" actual &&
934932
git checkout master
935933
'
936934

0 commit comments

Comments
 (0)