diff: align move detection error handling with other options

This changes the error handling for the options --color-moved-ws
and --color-moved-ws to be like the rest of the options.

Move the die() call out of parse_color_moved_ws into the parsing
of command line options. As the function returns a bit field, change
its signature to return an unsigned instead of an int; add a new bit
to signal errors. Once the error is signaled, we discard the other
bits, such that it doesn't matter if the error bit overlaps with any
other bit.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 8647db3d307c297448c91b328b1dabf19635fb94..d7d467b605d0f77b47f378c675eb5b61dbe2bc5f 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -291,7 +291,7 @@ static int parse_color_moved(const char *arg)
                return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed-zebra', 'plain'"));
 }
 
-static int parse_color_moved_ws(const char *arg)
+static unsigned parse_color_moved_ws(const char *arg)
 {
        int ret = 0;
        struct string_list l = STRING_LIST_INIT_DUP;
@@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)
                        ret |= XDF_IGNORE_WHITESPACE;
                else if (!strcmp(sb.buf, "allow-indentation-change"))
                        ret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;
-               else
-                       error(_("ignoring unknown color-moved-ws mode '%s'"), sb.buf);
+               else {
+                       ret |= COLOR_MOVED_WS_ERROR;
+                       error(_("unknown color-moved-ws mode '%s', possible values are 'ignore-space-change', 'ignore-space-at-eol', 'ignore-all-space', 'allow-indentation-change'"), sb.buf);
+               }
 
                strbuf_release(&sb);
        }
 
        if ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&
-           (ret & XDF_WHITESPACE_FLAGS))
-               die(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));
+           (ret & XDF_WHITESPACE_FLAGS)) {
+               error(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));
+               ret |= COLOR_MOVED_WS_ERROR;
+       }
 
        string_list_clear(&l, 0);
 
@@ -341,8 +345,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
                return 0;
        }
        if (!strcmp(var, "diff.colormovedws")) {
-               int cm = parse_color_moved_ws(value);
-               if (cm < 0)
+               unsigned cm = parse_color_moved_ws(value);
+               if (cm & COLOR_MOVED_WS_ERROR)
                        return -1;
                diff_color_moved_ws_default = cm;
                return 0;
@@ -5032,10 +5036,13 @@ int diff_opt_parse(struct diff_options *options,
        else if (skip_prefix(arg, "--color-moved=", &arg)) {
                int cm = parse_color_moved(arg);
                if (cm < 0)
-                       die("bad --color-moved argument: %s", arg);
+                       return error("bad --color-moved argument: %s", arg);
                options->color_moved = cm;
        } else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
-               options->color_moved_ws_handling = parse_color_moved_ws(arg);
+               unsigned cm = parse_color_moved_ws(arg);
+               if (cm & COLOR_MOVED_WS_ERROR)
+                       return -1;
+               options->color_moved_ws_handling = cm;
        } else if (skip_to_optional_arg_default(arg, "--color-words", &options->word_regex, NULL)) {
                options->use_color = 1;
                options->word_diff = DIFF_WORDS_COLOR;

diff --git a/diff.h b/diff.h
index ce5e8a8183e848b2e36d50bea271f687a450e3af..9e8061ca29ab9010e591727cc1a09e0121a6ade6 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -225,7 +225,8 @@ struct diff_options {
 
        /* XDF_WHITESPACE_FLAGS regarding block detection are set at 2, 3, 4 */
        #define COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE (1<<5)
-       int color_moved_ws_handling;
+       #define COLOR_MOVED_WS_ERROR (1<<0)
+       unsigned color_moved_ws_handling;
 
        struct repository *repo;
 };

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index a9fb226c5ad566095163c5141a4ca591f2aeacb1..9a3e4fdfecef4fa00d12b6ff32fc5b0beaff089e 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1890,6 +1890,24 @@ test_expect_success 'compare whitespace delta across moved blocks' '
        test_cmp expected actual
 '
 
+test_expect_success 'bogus settings in move detection erroring out' '
+       test_must_fail git diff --color-moved=bogus 2>err &&
+       test_i18ngrep "must be one of" err &&
+       test_i18ngrep bogus err &&
+
+       test_must_fail git -c diff.colormoved=bogus diff 2>err &&
+       test_i18ngrep "must be one of" err &&
+       test_i18ngrep "from command-line config" err &&
+
+       test_must_fail git diff --color-moved-ws=bogus 2>err &&
+       test_i18ngrep "possible values" err &&
+       test_i18ngrep bogus err &&
+
+       test_must_fail git -c diff.colormovedws=bogus diff 2>err &&
+       test_i18ngrep "possible values" err &&
+       test_i18ngrep "from command-line config" err
+'
+
 test_expect_success 'compare whitespace delta incompatible with other space options' '
        test_must_fail git diff \
                --color-moved-ws=allow-indentation-change,ignore-all-space \