From f3f5c7f5201ef71e62cea41a8b692cace2dcf579 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 4 Oct 2016 15:21:38 -0700 Subject: [PATCH 1/4] t4015: split out the "setup" part of ws-error-highlight test We'd want to run this same set of test twice, once with the option and another time with an equivalent configuration setting. Split out the step that prepares the test data and expected output and move the test for the command line option into a separate test. Signed-off-by: Junio C Hamano --- t/t4015-diff-whitespace.sh | 39 ++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 2434157aa7..4a4f374824 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -869,7 +869,8 @@ test_expect_success 'diff that introduces and removes ws breakages' ' test_cmp expected current ' -test_expect_success 'the same with --ws-error-highlight' ' +test_expect_success 'ws-error-highlight test setup' ' + git reset --hard && { echo "0. blank-at-eol " && @@ -882,10 +883,7 @@ test_expect_success 'the same with --ws-error-highlight' ' echo "2. and a new line " } >x && - git -c color.diff=always diff --ws-error-highlight=default,old | - test_decode_color >current && - - cat >expected <<-\EOF && + cat >expect.default-old <<-\EOF && diff --git a/x b/x index d0233a2..700886e 100644 --- a/x @@ -897,12 +895,7 @@ test_expect_success 'the same with --ws-error-highlight' ' +2. and a new line EOF - test_cmp expected current && - - git -c color.diff=always diff --ws-error-highlight=all | - test_decode_color >current && - - cat >expected <<-\EOF && + cat >expect.all <<-\EOF && diff --git a/x b/x index d0233a2..700886e 100644 --- a/x @@ -914,12 +907,7 @@ test_expect_success 'the same with --ws-error-highlight' ' +2. and a new line EOF - test_cmp expected current && - - git -c color.diff=always diff --ws-error-highlight=none | - test_decode_color >current && - - cat >expected <<-\EOF && + cat >expect.none <<-\EOF diff --git a/x b/x index d0233a2..700886e 100644 --- a/x @@ -931,7 +919,22 @@ test_expect_success 'the same with --ws-error-highlight' ' +2. and a new line EOF - test_cmp expected current +' + +test_expect_success 'test --ws-error-highlight option' ' + + git -c color.diff=always diff --ws-error-highlight=default,old | + test_decode_color >current && + test_cmp expect.default-old current && + + git -c color.diff=always diff --ws-error-highlight=all | + test_decode_color >current && + test_cmp expect.all current && + + git -c color.diff=always diff --ws-error-highlight=none | + test_decode_color >current && + test_cmp expect.none current + ' test_done From 077965f84a580b7e1de7d60ed13656bec19cc2fb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 4 Oct 2016 15:03:33 -0700 Subject: [PATCH 2/4] diff.c: refactor parse_ws_error_highlight() Rename the function to parse_ws_error_highlight_opt(), because it is meant to parse a command line option, and then refactor the meat of the function into a helper function that reports the parsed result which is typically a small unsigned int (these are OR'ed bitmask after all), or a negative offset that indicates where in the input string a parse error happened. Signed-off-by: Junio C Hamano --- diff.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/diff.c b/diff.c index 46260ed7a1..d346378600 100644 --- a/diff.c +++ b/diff.c @@ -3666,10 +3666,11 @@ static int parse_one_token(const char **arg, const char *token) return 0; } -static int parse_ws_error_highlight(struct diff_options *opt, const char *arg) +static int parse_ws_error_highlight(const char *arg) { const char *orig_arg = arg; unsigned val = 0; + while (*arg) { if (parse_one_token(&arg, "none")) val = 0; @@ -3684,13 +3685,23 @@ static int parse_ws_error_highlight(struct diff_options *opt, const char *arg) else if (parse_one_token(&arg, "context")) val |= WSEH_CONTEXT; else { - error("unknown value after ws-error-highlight=%.*s", - (int)(arg - orig_arg), orig_arg); - return 0; + return -1 - (int)(arg - orig_arg); } if (*arg) arg++; } + return val; +} + +static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arg) +{ + int val = parse_ws_error_highlight(arg); + + if (val < 0) { + error("unknown value after ws-error-highlight=%.*s", + -1 - val, arg); + return 0; + } opt->ws_error_highlight = val; return 1; } @@ -3894,7 +3905,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (skip_prefix(arg, "--submodule=", &arg)) return parse_submodule_opt(options, arg); else if (skip_prefix(arg, "--ws-error-highlight=", &arg)) - return parse_ws_error_highlight(options, arg); + return parse_ws_error_highlight_opt(options, arg); /* misc options */ else if (!strcmp(arg, "-z")) From 0b4b42e7fe1b507fb387c18d9a2748768359bf5d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 4 Oct 2016 15:09:18 -0700 Subject: [PATCH 3/4] diff.c: move ws-error-highlight parsing helpers up These need to be usable from git_diff_ui_config() code to help parsing a configuration variable, so move them up. Signed-off-by: Junio C Hamano --- diff.c | 74 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/diff.c b/diff.c index d346378600..bd625cf3f7 100644 --- a/diff.c +++ b/diff.c @@ -163,6 +163,43 @@ long parse_algorithm_value(const char *value) return -1; } +static int parse_one_token(const char **arg, const char *token) +{ + const char *rest; + if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) { + *arg = rest; + return 1; + } + return 0; +} + +static int parse_ws_error_highlight(const char *arg) +{ + const char *orig_arg = arg; + unsigned val = 0; + + while (*arg) { + if (parse_one_token(&arg, "none")) + val = 0; + else if (parse_one_token(&arg, "default")) + val = WSEH_NEW; + else if (parse_one_token(&arg, "all")) + val = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT; + else if (parse_one_token(&arg, "new")) + val |= WSEH_NEW; + else if (parse_one_token(&arg, "old")) + val |= WSEH_OLD; + else if (parse_one_token(&arg, "context")) + val |= WSEH_CONTEXT; + else { + return -1 - (int)(arg - orig_arg); + } + if (*arg) + arg++; + } + return val; +} + /* * These are to give UI layer defaults. * The core-level commands such as git-diff-files should @@ -3656,43 +3693,6 @@ static void enable_patch_output(int *fmt) { *fmt |= DIFF_FORMAT_PATCH; } -static int parse_one_token(const char **arg, const char *token) -{ - const char *rest; - if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) { - *arg = rest; - return 1; - } - return 0; -} - -static int parse_ws_error_highlight(const char *arg) -{ - const char *orig_arg = arg; - unsigned val = 0; - - while (*arg) { - if (parse_one_token(&arg, "none")) - val = 0; - else if (parse_one_token(&arg, "default")) - val = WSEH_NEW; - else if (parse_one_token(&arg, "all")) - val = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT; - else if (parse_one_token(&arg, "new")) - val |= WSEH_NEW; - else if (parse_one_token(&arg, "old")) - val |= WSEH_OLD; - else if (parse_one_token(&arg, "context")) - val |= WSEH_CONTEXT; - else { - return -1 - (int)(arg - orig_arg); - } - if (*arg) - arg++; - } - return val; -} - static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arg) { int val = parse_ws_error_highlight(arg); From a17505f262b62e429bb0e188c5ed73ac749e25b8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 4 Oct 2016 15:26:27 -0700 Subject: [PATCH 4/4] diff: introduce diff.wsErrorHighlight option With the preparatory steps, it has become trivial to teach the system a new diff.wsErrorHighlight configuration that gives the default value for --ws-error-highlight command line option. Signed-off-by: Junio C Hamano --- Documentation/diff-config.txt | 6 ++++++ Documentation/diff-options.txt | 2 ++ diff.c | 11 ++++++++++- t/t4015-diff-whitespace.sh | 35 ++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 6eaa45271c..c0396e66a5 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -182,3 +182,9 @@ diff.algorithm:: low-occurrence common elements". -- + + +diff.wsErrorHighlight:: + A comma separated list of `old`, `new`, `context`, that + specifies how whitespace errors on lines are highlighted + with `color.diff.whitespace`. Can be overridden by the + command line option `--ws-error-highlight=` diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 306b7e3604..dfd6bc99c6 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -303,6 +303,8 @@ ifndef::git-format-patch[] lines are highlighted. E.g. `--ws-error-highlight=new,old` highlights whitespace errors on both deleted and added lines. `all` can be used as a short-hand for `old,new,context`. + The `diff.wsErrorHighlight` configuration variable can be + used to specify the default behaviour. endif::git-format-patch[] diff --git a/diff.c b/diff.c index bd625cf3f7..9acf04f5b0 100644 --- a/diff.c +++ b/diff.c @@ -41,6 +41,7 @@ static int diff_stat_graph_width; static int diff_dirstat_permille_default = 30; static struct diff_options default_diff_options; static long diff_algorithm; +static unsigned ws_error_highlight_default = WSEH_NEW; static char diff_colors[][COLOR_MAXLEN] = { GIT_COLOR_RESET, @@ -262,6 +263,14 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "diff.wserrorhighlight")) { + int val = parse_ws_error_highlight(value); + if (val < 0) + return -1; + ws_error_highlight_default = val; + return 0; + } + if (git_color_config(var, value, cb) < 0) return -1; @@ -3306,7 +3315,7 @@ void diff_setup(struct diff_options *options) options->rename_limit = -1; options->dirstat_permille = diff_dirstat_permille_default; options->context = diff_context_default; - options->ws_error_highlight = WSEH_NEW; + options->ws_error_highlight = ws_error_highlight_default; DIFF_OPT_SET(options, RENAME_EMPTY); /* pathchange left =NULL by default */ diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 4a4f374824..289806d0c7 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -937,4 +937,39 @@ test_expect_success 'test --ws-error-highlight option' ' ' +test_expect_success 'test diff.wsErrorHighlight config' ' + + git -c color.diff=always -c diff.wsErrorHighlight=default,old diff | + test_decode_color >current && + test_cmp expect.default-old current && + + git -c color.diff=always -c diff.wsErrorHighlight=all diff | + test_decode_color >current && + test_cmp expect.all current && + + git -c color.diff=always -c diff.wsErrorHighlight=none diff | + test_decode_color >current && + test_cmp expect.none current + +' + +test_expect_success 'option overrides diff.wsErrorHighlight' ' + + git -c color.diff=always -c diff.wsErrorHighlight=none \ + diff --ws-error-highlight=default,old | + test_decode_color >current && + test_cmp expect.default-old current && + + git -c color.diff=always -c diff.wsErrorHighlight=default \ + diff --ws-error-highlight=all | + test_decode_color >current && + test_cmp expect.all current && + + git -c color.diff=always -c diff.wsErrorHighlight=all \ + diff --ws-error-highlight=none | + test_decode_color >current && + test_cmp expect.none current + +' + test_done