parse-options: do not show usage upon invalid option value
Usually, the usage should be shown only if the user does not know what options are available. If the user specifies an invalid value, the user is already aware of the available options. In this case, there is no point in displaying the usage anymore. This patch applies to "git tag --contains", "git branch --contains", "git branch --points-at", "git for-each-ref --contains" and many more. Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Paul-Sebastian Ungureanu committed
Mar 22, 2018 at 20:43 UTC
3bb0923f06c55ea44569f547cefa9e1a59069ff2
8 files changed
+125
-14
builtin/blame.c
+1
@@ -720,6 +720,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
720
for (;;) {
721
switch (parse_options_step(&ctx, options, blame_opt_usage)) {
722
case PARSE_OPT_HELP:
723
+ case PARSE_OPT_ERROR:
724
exit(129);
725
case PARSE_OPT_DONE:
726
if (ctx.argv[0])
builtin/shortlog.c
+1
@@ -283,6 +283,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
283
for (;;) {
284
switch (parse_options_step(&ctx, options, shortlog_usage)) {
285
case PARSE_OPT_HELP:
286
+ case PARSE_OPT_ERROR:
287
exit(129);
288
case PARSE_OPT_DONE:
289
goto parse_done;
builtin/update-index.c
+1
@@ -1059,6 +1059,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
1059
break;
1060
switch (parseopt_state) {
1061
case PARSE_OPT_HELP:
1062
+ case PARSE_OPT_ERROR:
1063
exit(129);
1064
case PARSE_OPT_NON_OPTION:
1065
case PARSE_OPT_DONE:
parse-options.c
+11
-9
@@ -317,14 +317,16 @@ is_abbreviated:
317
return get_value(p, options, all_opts, flags ^ opt_flags);
318
}
319
320
- if (ambiguous_option)
321
- return error("Ambiguous option: %s "
320
+ if (ambiguous_option) {
321
+ error("Ambiguous option: %s "
322
"(could be --%s%s or --%s%s)",
323
arg,
324
(ambiguous_flags & OPT_UNSET) ? "no-" : "",
325
ambiguous_option->long_name,
326
(abbrev_flags & OPT_UNSET) ? "no-" : "",
327
abbrev_option->long_name);
328
+ return -3;
329
+ }
330
if (abbrev_option)
331
return get_value(p, abbrev_option, all_opts, abbrev_flags);
332
return -2;
@@ -434,7 +436,6 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
436
const char * const usagestr[])
437
{
438
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
437
- int err = 0;
439
440
/* we must reset ->opt, unknown short option leave it dangling */
441
ctx->opt = NULL;
@@ -459,7 +460,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
460
ctx->opt = arg + 1;
461
switch (parse_short_opt(ctx, options)) {
462
case -1:
462
- goto show_usage_error;
463
+ return PARSE_OPT_ERROR;
464
case -2:
465
if (ctx->opt)
466
check_typos(arg + 1, options);
@@ -472,7 +473,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
473
while (ctx->opt) {
474
switch (parse_short_opt(ctx, options)) {
475
case -1:
475
- goto show_usage_error;
476
+ return PARSE_OPT_ERROR;
477
case -2:
478
if (internal_help && *ctx->opt == 'h')
479
goto show_usage;
@@ -504,9 +505,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
505
goto show_usage;
506
switch (parse_long_opt(ctx, arg + 2, options)) {
507
case -1:
507
- goto show_usage_error;
508
+ return PARSE_OPT_ERROR;
509
case -2:
510
goto unknown;
511
+ case -3:
512
+ goto show_usage;
513
}
514
continue;
515
unknown:
@@ -517,10 +520,8 @@ unknown:
520
}
521
return PARSE_OPT_DONE;
522
520
- show_usage_error:
521
- err = 1;
523
show_usage:
523
- return usage_with_options_internal(ctx, usagestr, options, 0, err);
524
+ return usage_with_options_internal(ctx, usagestr, options, 0, 0);
525
}
526
527
int parse_options_end(struct parse_opt_ctx_t *ctx)
@@ -539,6 +540,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
540
parse_options_start(&ctx, argc, argv, prefix, options, flags);
541
switch (parse_options_step(&ctx, options, usagestr)) {
542
case PARSE_OPT_HELP:
543
+ case PARSE_OPT_ERROR:
544
exit(129);
545
case PARSE_OPT_NON_OPTION:
546
case PARSE_OPT_DONE:
parse-options.h
+1
@@ -188,6 +188,7 @@ enum {
188
PARSE_OPT_HELP = -1,
189
PARSE_OPT_DONE,
190
PARSE_OPT_NON_OPTION,
191
+ PARSE_OPT_ERROR,
192
PARSE_OPT_UNKNOWN
193
};
194
t/t0040-parse-options.sh
+1
-1
@@ -291,7 +291,7 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '
291
test_expect_success 'OPT_CALLBACK() and callback errors work' '
292
test_must_fail test-parse-options --no-length >output 2>output.err &&
293
test_i18ncmp expect output &&
294
- test_i18ncmp expect.err output.err
294
+ test_must_be_empty output.err
295
'
296
297
cat >expect <<\EOF
t/t0041-usage.sh
new
+107
@@ -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
+2
-4
@@ -915,10 +915,8 @@ test_expect_success 'rebase --exec works without -i ' '
915
test_expect_success 'rebase -i --exec without <CMD>' '
916
git reset --hard execute &&
917
set_fake_editor &&
918
- test_must_fail git rebase -i --exec 2>tmp &&
919
- sed -e "1d" tmp >actual &&
920
- test_must_fail git rebase -h >expected &&
921
- test_cmp expected actual &&
918
+ test_must_fail git rebase -i --exec 2>actual &&
919
+ test_i18ngrep "requires a value" actual &&
920
git checkout master
921
'
922