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>

Stefan Beller committed Nov 13, 2018 at 13:33 UTC d173e799ea8fae7d6e4649b763d32d5f0ba82011
3 files changed +36 -10
diff.c
+16 -9
@@ -291,7 +291,7 @@ static int parse_color_moved(const char *arg)
291 return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed-zebra', 'plain'"));
292 }
293
294 -static int parse_color_moved_ws(const char *arg)
294 +static unsigned parse_color_moved_ws(const char *arg)
295 {
296 int ret = 0;
297 struct string_list l = STRING_LIST_INIT_DUP;
@@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)
312 ret |= XDF_IGNORE_WHITESPACE;
313 else if (!strcmp(sb.buf, "allow-indentation-change"))
314 ret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;
315 - else
316 - error(_("ignoring unknown color-moved-ws mode '%s'"), sb.buf);
315 + else {
316 + ret |= COLOR_MOVED_WS_ERROR;
317 + 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);
318 + }
319
320 strbuf_release(&sb);
321 }
322
323 if ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&
322 - (ret & XDF_WHITESPACE_FLAGS))
323 - die(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));
324 + (ret & XDF_WHITESPACE_FLAGS)) {
325 + error(_("color-moved-ws: allow-indentation-change cannot be combined with other white space modes"));
326 + ret |= COLOR_MOVED_WS_ERROR;
327 + }
328
329 string_list_clear(&l, 0);
330
@@ -341,8 +345,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
345 return 0;
346 }
347 if (!strcmp(var, "diff.colormovedws")) {
344 - int cm = parse_color_moved_ws(value);
345 - if (cm < 0)
348 + unsigned cm = parse_color_moved_ws(value);
349 + if (cm & COLOR_MOVED_WS_ERROR)
350 return -1;
351 diff_color_moved_ws_default = cm;
352 return 0;
@@ -5032,10 +5036,13 @@ int diff_opt_parse(struct diff_options *options,
5036 else if (skip_prefix(arg, "--color-moved=", &arg)) {
5037 int cm = parse_color_moved(arg);
5038 if (cm < 0)
5035 - die("bad --color-moved argument: %s", arg);
5039 + return error("bad --color-moved argument: %s", arg);
5040 options->color_moved = cm;
5041 } else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
5038 - options->color_moved_ws_handling = parse_color_moved_ws(arg);
5042 + unsigned cm = parse_color_moved_ws(arg);
5043 + if (cm & COLOR_MOVED_WS_ERROR)
5044 + return -1;
5045 + options->color_moved_ws_handling = cm;
5046 } else if (skip_to_optional_arg_default(arg, "--color-words", &options->word_regex, NULL)) {
5047 options->use_color = 1;
5048 options->word_diff = DIFF_WORDS_COLOR;
diff.h
+2 -1
@@ -225,7 +225,8 @@ struct diff_options {
225
226 /* XDF_WHITESPACE_FLAGS regarding block detection are set at 2, 3, 4 */
227 #define COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE (1<<5)
228 - int color_moved_ws_handling;
228 + #define COLOR_MOVED_WS_ERROR (1<<0)
229 + unsigned color_moved_ws_handling;
230
231 struct repository *repo;
232 };
t/t4015-diff-whitespace.sh
+18
@@ -1890,6 +1890,24 @@ test_expect_success 'compare whitespace delta across moved blocks' '
1890 test_cmp expected actual
1891 '
1892
1893 +test_expect_success 'bogus settings in move detection erroring out' '
1894 + test_must_fail git diff --color-moved=bogus 2>err &&
1895 + test_i18ngrep "must be one of" err &&
1896 + test_i18ngrep bogus err &&
1897 +
1898 + test_must_fail git -c diff.colormoved=bogus diff 2>err &&
1899 + test_i18ngrep "must be one of" err &&
1900 + test_i18ngrep "from command-line config" err &&
1901 +
1902 + test_must_fail git diff --color-moved-ws=bogus 2>err &&
1903 + test_i18ngrep "possible values" err &&
1904 + test_i18ngrep bogus err &&
1905 +
1906 + test_must_fail git -c diff.colormovedws=bogus diff 2>err &&
1907 + test_i18ngrep "possible values" err &&
1908 + test_i18ngrep "from command-line config" err
1909 +'
1910 +
1911 test_expect_success 'compare whitespace delta incompatible with other space options' '
1912 test_must_fail git diff \
1913 --color-moved-ws=allow-indentation-change,ignore-all-space \