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

diff --git a/builtin/blame.c b/builtin/blame.c
index f1a2fd67029e6980d317da5136aece233ba192f1..db38c0b307c5719ab3bd5e6b8597ec8810c0de3d 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -729,6 +729,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
        for (;;) {
                switch (parse_options_step(&ctx, options, blame_opt_usage)) {
                case PARSE_OPT_HELP:
+               case PARSE_OPT_ERROR:
                        exit(129);
                case PARSE_OPT_DONE:
                        if (ctx.argv[0])

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 3a823b3128259c8f9ee92dbbfaacb492a90e7d6e..608d6ba77bdfb4673513444651053d0e8e789020 100644 (file)
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -284,6 +284,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
        for (;;) {
                switch (parse_options_step(&ctx, options, shortlog_usage)) {
                case PARSE_OPT_HELP:
+               case PARSE_OPT_ERROR:
                        exit(129);
                case PARSE_OPT_DONE:
                        goto parse_done;

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 9625d1e10a691c1dbd2ec2aa5d10a6c8a6ca7885..10d070a76fb1b0b94c058f60934bb05db37a4164 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1059,6 +1059,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
                        break;
                switch (parseopt_state) {
                case PARSE_OPT_HELP:
+               case PARSE_OPT_ERROR:
                        exit(129);
                case PARSE_OPT_NON_OPTION:
                case PARSE_OPT_DONE:

diff --git a/parse-options.c b/parse-options.c
index 125e84f98451b4eb12e9d8a6cb4da58b2d8db51e..0f7059a8ab32a624775026d7dc2289245c87c192 100644 (file)
--- a/parse-options.c
+++ b/parse-options.c
@@ -317,14 +317,16 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                return get_value(p, options, all_opts, flags ^ opt_flags);
        }
 
-       if (ambiguous_option)
-               return error("Ambiguous option: %s "
+       if (ambiguous_option) {
+               error("Ambiguous option: %s "
                        "(could be --%s%s or --%s%s)",
                        arg,
                        (ambiguous_flags & OPT_UNSET) ?  "no-" : "",
                        ambiguous_option->long_name,
                        (abbrev_flags & OPT_UNSET) ?  "no-" : "",
                        abbrev_option->long_name);
+               return -3;
+       }
        if (abbrev_option)
                return get_value(p, abbrev_option, all_opts, abbrev_flags);
        return -2;
@@ -476,7 +478,6 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                       const char * const usagestr[])
 {
        int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
-       int err = 0;
 
        /* we must reset ->opt, unknown short option leave it dangling */
        ctx->opt = NULL;
@@ -505,7 +506,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                        ctx->opt = arg + 1;
                        switch (parse_short_opt(ctx, options)) {
                        case -1:
-                               goto show_usage_error;
+                               return PARSE_OPT_ERROR;
                        case -2:
                                if (ctx->opt)
                                        check_typos(arg + 1, options);
@@ -518,7 +519,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                        while (ctx->opt) {
                                switch (parse_short_opt(ctx, options)) {
                                case -1:
-                                       goto show_usage_error;
+                                       return PARSE_OPT_ERROR;
                                case -2:
                                        if (internal_help && *ctx->opt == 'h')
                                                goto show_usage;
@@ -550,9 +551,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                        goto show_usage;
                switch (parse_long_opt(ctx, arg + 2, options)) {
                case -1:
-                       goto show_usage_error;
+                       return PARSE_OPT_ERROR;
                case -2:
                        goto unknown;
+               case -3:
+                       goto show_usage;
                }
                continue;
 unknown:
@@ -563,10 +566,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
        }
        return PARSE_OPT_DONE;
 
- show_usage_error:
-       err = 1;
  show_usage:
-       return usage_with_options_internal(ctx, usagestr, options, 0, err);
+       return usage_with_options_internal(ctx, usagestr, options, 0, 0);
 }
 
 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,
        parse_options_start(&ctx, argc, argv, prefix, options, flags);
        switch (parse_options_step(&ctx, options, usagestr)) {
        case PARSE_OPT_HELP:
+       case PARSE_OPT_ERROR:
                exit(129);
        case PARSE_OPT_NON_OPTION:
        case PARSE_OPT_DONE:

diff --git a/parse-options.h b/parse-options.h
index ab1cc362bf2918c28a14dd851c1b1a13dfa0c863..dd14911a297a5b10705ecb31243c55a7dc2f193c 100644 (file)
--- a/parse-options.h
+++ b/parse-options.h
@@ -200,6 +200,7 @@ enum {
        PARSE_OPT_HELP = -1,
        PARSE_OPT_DONE,
        PARSE_OPT_NON_OPTION,
+       PARSE_OPT_ERROR,
        PARSE_OPT_UNKNOWN
 };
 

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 0c2fc81d7b0fa401db58c41cd2fed4469e80b058..04d474c84fd69121c686f5ca5adc40ce081f0e9d 100755 (executable)
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -291,7 +291,7 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '
 test_expect_success 'OPT_CALLBACK() and callback errors work' '
        test_must_fail test-parse-options --no-length >output 2>output.err &&
        test_i18ncmp expect output &&
-       test_i18ncmp expect.err output.err
+       test_must_be_empty output.err
 '
 
 cat >expect <<\EOF

diff --git a/t/t0041-usage.sh b/t/t0041-usage.sh
new file mode 100755 (executable)
index 0000000..5b927b7
--- /dev/null
+++ b/t/t0041-usage.sh
@@ -0,0 +1,107 @@
+#!/bin/sh
+
+test_description='Test commands behavior when given invalid argument value'
+
+. ./test-lib.sh
+
+test_expect_success 'setup ' '
+       test_commit "v1.0"
+'
+
+test_expect_success 'tag --contains <existent_tag>' '
+       git tag --contains "v1.0" >actual 2>actual.err &&
+       grep "v1.0" actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'tag --contains <inexistent_tag>' '
+       test_must_fail git tag --contains "notag" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'tag --no-contains <existent_tag>' '
+       git tag --no-contains "v1.0" >actual 2>actual.err  &&
+       test_line_count = 0 actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'tag --no-contains <inexistent_tag>' '
+       test_must_fail git tag --no-contains "notag" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'tag usage error' '
+       test_must_fail git tag --noopt >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "usage" actual.err
+'
+
+test_expect_success 'branch --contains <existent_commit>' '
+       git branch --contains "master" >actual 2>actual.err &&
+       test_i18ngrep "master" actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'branch --contains <inexistent_commit>' '
+       test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'branch --no-contains <existent_commit>' '
+       git branch --no-contains "master" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'branch --no-contains <inexistent_commit>' '
+       test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'branch usage error' '
+       test_must_fail git branch --noopt >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "usage" actual.err
+'
+
+test_expect_success 'for-each-ref --contains <existent_object>' '
+       git for-each-ref --contains "master" >actual 2>actual.err &&
+       test_line_count = 2 actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'for-each-ref --contains <inexistent_object>' '
+       test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'for-each-ref --no-contains <existent_object>' '
+       git for-each-ref --no-contains "master" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_line_count = 0 actual.err
+'
+
+test_expect_success 'for-each-ref --no-contains <inexistent_object>' '
+       test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "error" actual.err &&
+       test_i18ngrep ! "usage" actual.err
+'
+
+test_expect_success 'for-each-ref usage error' '
+       test_must_fail git for-each-ref --noopt >actual 2>actual.err &&
+       test_line_count = 0 actual &&
+       test_i18ngrep "usage" actual.err
+'
+
+test_done

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 3b905406df79187f70c828f72e9e2dc187f1be57..c59d0384fd6f797ae30df9c157dbdf4b0307fa02 100755 (executable)
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -927,10 +927,8 @@ test_expect_success 'rebase --exec works without -i ' '
 test_expect_success 'rebase -i --exec without <CMD>' '
        git reset --hard execute &&
        set_fake_editor &&
-       test_must_fail git rebase -i --exec 2>tmp &&
-       sed -e "1d" tmp >actual &&
-       test_must_fail git rebase -h >expected &&
-       test_cmp expected actual &&
+       test_must_fail git rebase -i --exec 2>actual &&
+       test_i18ngrep "requires a value" actual &&
        git checkout master
 '