{"thread":{"id":"44213","subject":"color.diff.whitespace unused on removed lines","startedAt":"2016-10-04T08:14:46Z","lastAt":"2016-10-18T11:02:03Z","messageCount":11,"participants":["Sandro Santilli","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"303194","messageId":"20161004081429.GC17002@localhost","threadId":"44213","inReplyTo":null,"subject":"color.diff.whitespace unused on removed lines","fromName":"Sandro Santilli","fromEmail":"strk@kbt.io","sentAt":"2016-10-04T08:14:29Z","receivedAt":"2016-10-04T08:14:46Z","isPatch":false,"sender":{"key":"strk@kbt.io","avatar":"https://gravatar.com/avatar/5ae72c1d61f690d66f62227997cbc311e948230af98faff72e2bda393a682537?d=mp&s=160"},"body":"The color.diff.whitespace configuration is not used on\nremoved lines, but only on added lines.\n\nAs I'm removing trailing whitespaces all my diffs fail to\ngive me any information about the real action being taken,\ndue to this lack of support.\n\nAs a workaround, I've found the -R switch for \"git show\" [1]\nbut I thought about asking if this is a bug or intended behavior,\nand in case it is intended I'd like to know why.\n\nThanks in advance.\n\n[1]\nhttp://stackoverflow.com/questions/5257553/coloring-white-space-in-git-diffs-output/11509388#11509388\n\n--strk; \n\n  ()   Free GIS & Flash consultant/developer\n  /\\   https://strk.kbt.io/services.html\n"},{"id":"303243","messageId":"20161004152954.74bojbyagxr2xefz@sigill.intra.peff.net","threadId":"44213","inReplyTo":"20161004081429.GC17002@localhost","subject":"Re: color.diff.whitespace unused on removed lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-04T15:29:55Z","receivedAt":"2016-10-04T15:30:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2016 at 10:14:29AM +0200, Sandro Santilli wrote:\n\n> The color.diff.whitespace configuration is not used on\n> removed lines, but only on added lines.\n\nRight. The original purpose was to warn you when you were introducing\nwhitespace breakages. Getting rid of other people's whitespace breakages\nis OK.\n\nWe later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,\n2015-05-26) to let you see them on other lines, though. I think that\nwould do what you want.\n\n-Peff\n"},{"id":"303244","messageId":"20161004153523.GA2798@localhost","threadId":"44213","inReplyTo":"20161004152954.74bojbyagxr2xefz@sigill.intra.peff.net","subject":"Re: color.diff.whitespace unused on removed lines","fromName":"Sandro Santilli","fromEmail":"strk@kbt.io","sentAt":"2016-10-04T15:35:23Z","receivedAt":"2016-10-04T15:36:03Z","isPatch":false,"sender":{"key":"strk@kbt.io","avatar":"https://gravatar.com/avatar/5ae72c1d61f690d66f62227997cbc311e948230af98faff72e2bda393a682537?d=mp&s=160"},"body":"On Tue, Oct 04, 2016 at 11:29:55AM -0400, Jeff King wrote:\n> On Tue, Oct 04, 2016 at 10:14:29AM +0200, Sandro Santilli wrote:\n> \n> > The color.diff.whitespace configuration is not used on\n> > removed lines, but only on added lines.\n> \n> Right. The original purpose was to warn you when you were introducing\n> whitespace breakages. Getting rid of other people's whitespace breakages\n> is OK.\n> \n> We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,\n> 2015-05-26) to let you see them on other lines, though. I think that\n> would do what you want.\n\nThanks, it does do what I want.\nAny chance to specify it in the config file that I want it\nalways to behave in a certain way ?\n\n--strk;\n"},{"id":"303251","messageId":"20161004161323.53qec37i2tujaxcy@sigill.intra.peff.net","threadId":"44213","inReplyTo":"20161004153523.GA2798@localhost","subject":"Re: color.diff.whitespace unused on removed lines","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-04T16:13:24Z","receivedAt":"2016-10-04T16:13:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2016 at 05:35:23PM +0200, Sandro Santilli wrote:\n\n> > We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,\n> > 2015-05-26) to let you see them on other lines, though. I think that\n> > would do what you want.\n> \n> Thanks, it does do what I want.\n> Any chance to specify it in the config file that I want it\n> always to behave in a certain way ?\n\nNo, I don't think there's currently a matching config option. You can\nuse an alias, or propose a patch to add a config option.\n\n-Peff\n"},{"id":"303272","messageId":"xmqqk2douhe0.fsf@gitster.mtv.corp.google.com","threadId":"44213","inReplyTo":"20161004161323.53qec37i2tujaxcy@sigill.intra.peff.net","subject":"Re: color.diff.whitespace unused on removed lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T17:57:11Z","receivedAt":"2016-10-04T17:57:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Oct 04, 2016 at 05:35:23PM +0200, Sandro Santilli wrote:\n>\n>> > We later did b8767f7 (diff.c: --ws-error-highlight=<kind> option,\n>> > 2015-05-26) to let you see them on other lines, though. I think that\n>> > would do what you want.\n>> \n>> Thanks, it does do what I want.\n>> Any chance to specify it in the config file that I want it\n>> always to behave in a certain way ?\n>\n> No, I don't think there's currently a matching config option. You can\n> use an alias, or propose a patch to add a config option.\n\nThe final shape of such a patch would include something like the\nattached.  It would need to be split into a few patches and then get\nadditional tests and documentation written, so I won't be committing\nit myself in this shape.\n\n\n diff.c | 84 +++++++++++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 50 insertions(+), 34 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a178ed39bc..a2193c3aea 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -43,6 +43,7 @@ static int diff_stat_graph_width;\n static int diff_dirstat_permille_default = 30;\n static struct diff_options default_diff_options;\n static long diff_algorithm;\n+static unsigned ws_error_highlight_default = WSEH_NEW;\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_RESET,\n@@ -172,6 +173,42 @@ long parse_algorithm_value(const char *value)\n \treturn -1;\n }\n \n+static int parse_one_token(const char **arg, const char *token)\n+{\n+\tconst char *rest;\n+\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n+\t\t*arg = rest;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int parse_ws_error_highlight(const char *arg)\n+{\n+\tconst char *orig_arg = arg;\n+\tunsigned val = 0;\n+\twhile (*arg) {\n+\t\tif (parse_one_token(&arg, \"none\"))\n+\t\t\tval = 0;\n+\t\telse if (parse_one_token(&arg, \"default\"))\n+\t\t\tval = WSEH_NEW;\n+\t\telse if (parse_one_token(&arg, \"all\"))\n+\t\t\tval = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT;\n+\t\telse if (parse_one_token(&arg, \"new\"))\n+\t\t\tval |= WSEH_NEW;\n+\t\telse if (parse_one_token(&arg, \"old\"))\n+\t\t\tval |= WSEH_OLD;\n+\t\telse if (parse_one_token(&arg, \"context\"))\n+\t\t\tval |= WSEH_CONTEXT;\n+\t\telse {\n+\t\t\treturn (orig_arg - arg);\n+\t\t}\n+\t\tif (*arg)\n+\t\t\targ++;\n+\t}\n+\treturn val;\n+}\n+\n /*\n  * These are to give UI layer defaults.\n  * The core-level commands such as git-diff-files should\n@@ -254,6 +291,11 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"diff.wserrorhighlight\")) {\n+\t\tws_error_highlight_default = parse_ws_error_highlight(value);\n+\t\treturn 0;\n+\t}\n+\n \tif (git_diff_heuristic_config(var, value, cb) < 0)\n \t\treturn -1;\n \tif (git_color_config(var, value, cb) < 0)\n@@ -3307,7 +3349,7 @@ void diff_setup(struct diff_options *options)\n \toptions->rename_limit = -1;\n \toptions->dirstat_permille = diff_dirstat_permille_default;\n \toptions->context = diff_context_default;\n-\toptions->ws_error_highlight = WSEH_NEW;\n+\toptions->ws_error_highlight = ws_error_highlight_default;\n \tDIFF_OPT_SET(options, RENAME_EMPTY);\n \n \t/* pathchange left =NULL by default */\n@@ -3698,40 +3740,14 @@ static void enable_patch_output(int *fmt) {\n \t*fmt |= DIFF_FORMAT_PATCH;\n }\n \n-static int parse_one_token(const char **arg, const char *token)\n+static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arg)\n {\n-\tconst char *rest;\n-\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n-\t\t*arg = rest;\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n+\tint val = parse_ws_error_highlight(arg);\n \n-static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)\n-{\n-\tconst char *orig_arg = arg;\n-\tunsigned val = 0;\n-\twhile (*arg) {\n-\t\tif (parse_one_token(&arg, \"none\"))\n-\t\t\tval = 0;\n-\t\telse if (parse_one_token(&arg, \"default\"))\n-\t\t\tval = WSEH_NEW;\n-\t\telse if (parse_one_token(&arg, \"all\"))\n-\t\t\tval = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT;\n-\t\telse if (parse_one_token(&arg, \"new\"))\n-\t\t\tval |= WSEH_NEW;\n-\t\telse if (parse_one_token(&arg, \"old\"))\n-\t\t\tval |= WSEH_OLD;\n-\t\telse if (parse_one_token(&arg, \"context\"))\n-\t\t\tval |= WSEH_CONTEXT;\n-\t\telse {\n-\t\t\terror(\"unknown value after ws-error-highlight=%.*s\",\n-\t\t\t      (int)(arg - orig_arg), orig_arg);\n-\t\t\treturn 0;\n-\t\t}\n-\t\tif (*arg)\n-\t\t\targ++;\n+\tif (val < 0) {\n+\t\terror(\"unknown value after ws-error-highlight=%.*s\",\n+\t\t      -val, arg);\n+\t\treturn 0;\n \t}\n \topt->ws_error_highlight = val;\n \treturn 1;\n@@ -3950,7 +3966,7 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (skip_prefix(arg, \"--submodule=\", &arg))\n \t\treturn parse_submodule_opt(options, arg);\n \telse if (skip_prefix(arg, \"--ws-error-highlight=\", &arg))\n-\t\treturn parse_ws_error_highlight(options, arg);\n+\t\treturn parse_ws_error_highlight_opt(options, arg);\n \n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\n"},{"id":"303327","messageId":"20161004225449.6759-1-gitster@pobox.com","threadId":"44213","inReplyTo":"xmqqk2douhe0.fsf@gitster.mtv.corp.google.com","subject":"[PATCH 0/4] diff.wsErrorHighlight configuration variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:54:45Z","receivedAt":"2016-10-04T22:54:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git diff\" and its family of commands have \"--ws-error-highlight\"\noption to allow whitespace breakages on old and context lines\npainted in color.diff.whitespace color, instead of the usual \"we\npaint breakages only on new lines\", but there wasn't a configuration\nvariable that corresponds to it.\n\nThis would be a lot closer to a series that could be acceptable,\ncompared to the previous \"it should look like this\" patch.\n\nJunio C Hamano (4):\n  t4015: split out the \"setup\" part of ws-error-highlight test\n  diff.c: refactor parse_ws_error_highlight()\n  diff.c: move ws-error-highlight parsing helpers up\n  diff: introduce diff.wsErrorHighlight option\n\n Documentation/diff-config.txt  |  6 +++\n Documentation/diff-options.txt |  2 +\n diff.c                         | 88 ++++++++++++++++++++++++++----------------\n t/t4015-diff-whitespace.sh     | 74 ++++++++++++++++++++++++++---------\n 4 files changed, 118 insertions(+), 52 deletions(-)\n\n-- \n2.10.1-510-g1ef781f2c1\n\n"},{"id":"303328","messageId":"20161004225449.6759-2-gitster@pobox.com","threadId":"44213","inReplyTo":"20161004225449.6759-1-gitster@pobox.com","subject":"[PATCH 1/4] t4015: split out the \"setup\" part of ws-error-highlight test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:54:46Z","receivedAt":"2016-10-04T22:55:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We'd want to run this same set of test twice, once with the option\nand another time with an equivalent configuration setting.  Split\nout the step that prepares the test data and expected output and\nmove the test for the command line option into a separate test.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t4015-diff-whitespace.sh | 39 +++++++++++++++++++++------------------\n 1 file changed, 21 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 2434157aa7..4a4f374824 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -869,7 +869,8 @@ test_expect_success 'diff that introduces and removes ws breakages' '\n \ttest_cmp expected current\n '\n \n-test_expect_success 'the same with --ws-error-highlight' '\n+test_expect_success 'ws-error-highlight test setup' '\n+\n \tgit reset --hard &&\n \t{\n \t\techo \"0. blank-at-eol \" &&\n@@ -882,10 +883,7 @@ test_expect_success 'the same with --ws-error-highlight' '\n \t\techo \"2. and a new line \"\n \t} >x &&\n \n-\tgit -c color.diff=always diff --ws-error-highlight=default,old |\n-\ttest_decode_color >current &&\n-\n-\tcat >expected <<-\\EOF &&\n+\tcat >expect.default-old <<-\\EOF &&\n \t<BOLD>diff --git a/x b/x<RESET>\n \t<BOLD>index d0233a2..700886e 100644<RESET>\n \t<BOLD>--- a/x<RESET>\n@@ -897,12 +895,7 @@ test_expect_success 'the same with --ws-error-highlight' '\n \t<GREEN>+<RESET><GREEN>2. and a new line<RESET><BLUE> <RESET>\n \tEOF\n \n-\ttest_cmp expected current &&\n-\n-\tgit -c color.diff=always diff --ws-error-highlight=all |\n-\ttest_decode_color >current &&\n-\n-\tcat >expected <<-\\EOF &&\n+\tcat >expect.all <<-\\EOF &&\n \t<BOLD>diff --git a/x b/x<RESET>\n \t<BOLD>index d0233a2..700886e 100644<RESET>\n \t<BOLD>--- a/x<RESET>\n@@ -914,12 +907,7 @@ test_expect_success 'the same with --ws-error-highlight' '\n \t<GREEN>+<RESET><GREEN>2. and a new line<RESET><BLUE> <RESET>\n \tEOF\n \n-\ttest_cmp expected current &&\n-\n-\tgit -c color.diff=always diff --ws-error-highlight=none |\n-\ttest_decode_color >current &&\n-\n-\tcat >expected <<-\\EOF &&\n+\tcat >expect.none <<-\\EOF\n \t<BOLD>diff --git a/x b/x<RESET>\n \t<BOLD>index d0233a2..700886e 100644<RESET>\n \t<BOLD>--- a/x<RESET>\n@@ -931,7 +919,22 @@ test_expect_success 'the same with --ws-error-highlight' '\n \t<GREEN>+2. and a new line <RESET>\n \tEOF\n \n-\ttest_cmp expected current\n+'\n+\n+test_expect_success 'test --ws-error-highlight option' '\n+\n+\tgit -c color.diff=always diff --ws-error-highlight=default,old |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.default-old current &&\n+\n+\tgit -c color.diff=always diff --ws-error-highlight=all |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.all current &&\n+\n+\tgit -c color.diff=always diff --ws-error-highlight=none |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.none current\n+\n '\n \n test_done\n-- \n2.10.1-510-g1ef781f2c1\n\n"},{"id":"303329","messageId":"20161004225449.6759-3-gitster@pobox.com","threadId":"44213","inReplyTo":"20161004225449.6759-1-gitster@pobox.com","subject":"[PATCH 2/4] diff.c: refactor parse_ws_error_highlight()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:54:47Z","receivedAt":"2016-10-04T22:55:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rename the function to parse_ws_error_highlight_opt(), because it is\nmeant to parse a command line option, and then refactor the meat of\nthe function into a helper function that reports the parsed result\nwhich is typically a small unsigned int (these are OR'ed bitmask\nafter all), or a negative offset that indicates where in the input\nstring a parse error happened.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 21 ++++++++++++++++-----\n 1 file changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 46260ed7a1..d346378600 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3666,10 +3666,11 @@ static int parse_one_token(const char **arg, const char *token)\n \treturn 0;\n }\n \n-static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)\n+static int parse_ws_error_highlight(const char *arg)\n {\n \tconst char *orig_arg = arg;\n \tunsigned val = 0;\n+\n \twhile (*arg) {\n \t\tif (parse_one_token(&arg, \"none\"))\n \t\t\tval = 0;\n@@ -3684,13 +3685,23 @@ static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)\n \t\telse if (parse_one_token(&arg, \"context\"))\n \t\t\tval |= WSEH_CONTEXT;\n \t\telse {\n-\t\t\terror(\"unknown value after ws-error-highlight=%.*s\",\n-\t\t\t      (int)(arg - orig_arg), orig_arg);\n-\t\t\treturn 0;\n+\t\t\treturn -1 - (int)(arg - orig_arg);\n \t\t}\n \t\tif (*arg)\n \t\t\targ++;\n \t}\n+\treturn val;\n+}\n+\n+static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arg)\n+{\n+\tint val = parse_ws_error_highlight(arg);\n+\n+\tif (val < 0) {\n+\t\terror(\"unknown value after ws-error-highlight=%.*s\",\n+\t\t      -1 - val, arg);\n+\t\treturn 0;\n+\t}\n \topt->ws_error_highlight = val;\n \treturn 1;\n }\n@@ -3894,7 +3905,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (skip_prefix(arg, \"--submodule=\", &arg))\n \t\treturn parse_submodule_opt(options, arg);\n \telse if (skip_prefix(arg, \"--ws-error-highlight=\", &arg))\n-\t\treturn parse_ws_error_highlight(options, arg);\n+\t\treturn parse_ws_error_highlight_opt(options, arg);\n \n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\n-- \n2.10.1-510-g1ef781f2c1\n\n"},{"id":"303330","messageId":"20161004225449.6759-5-gitster@pobox.com","threadId":"44213","inReplyTo":"20161004225449.6759-1-gitster@pobox.com","subject":"[PATCH 4/4] diff: introduce diff.wsErrorHighlight option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:54:49Z","receivedAt":"2016-10-04T22:55:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the preparatory steps, it has become trivial to teach the\nsystem a new diff.wsErrorHighlight configuration that gives the\ndefault value for --ws-error-highlight command line option.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/diff-config.txt  |  6 ++++++\n Documentation/diff-options.txt |  2 ++\n diff.c                         | 11 ++++++++++-\n t/t4015-diff-whitespace.sh     | 35 +++++++++++++++++++++++++++++++++++\n 4 files changed, 53 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 6eaa45271c..c0396e66a5 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -182,3 +182,9 @@ diff.algorithm::\n \tlow-occurrence common elements\".\n --\n +\n+\n+diff.wsErrorHighlight::\n+\tA comma separated list of `old`, `new`, `context`, that\n+\tspecifies how whitespace errors on lines are highlighted\n+\twith `color.diff.whitespace`.  Can be overridden by the\n+\tcommand line option `--ws-error-highlight=<kind>`\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 306b7e3604..dfd6bc99c6 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -303,6 +303,8 @@ ifndef::git-format-patch[]\n \tlines are highlighted.  E.g. `--ws-error-highlight=new,old`\n \thighlights whitespace errors on both deleted and added lines.\n \t`all` can be used as a short-hand for `old,new,context`.\n+\tThe `diff.wsErrorHighlight` configuration variable can be\n+\tused to specify the default behaviour.\n \n endif::git-format-patch[]\n \ndiff --git a/diff.c b/diff.c\nindex bd625cf3f7..9acf04f5b0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -41,6 +41,7 @@ static int diff_stat_graph_width;\n static int diff_dirstat_permille_default = 30;\n static struct diff_options default_diff_options;\n static long diff_algorithm;\n+static unsigned ws_error_highlight_default = WSEH_NEW;\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_RESET,\n@@ -262,6 +263,14 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"diff.wserrorhighlight\")) {\n+\t\tint val = parse_ws_error_highlight(value);\n+\t\tif (val < 0)\n+\t\t\treturn -1;\n+\t\tws_error_highlight_default = val;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_color_config(var, value, cb) < 0)\n \t\treturn -1;\n \n@@ -3306,7 +3315,7 @@ void diff_setup(struct diff_options *options)\n \toptions->rename_limit = -1;\n \toptions->dirstat_permille = diff_dirstat_permille_default;\n \toptions->context = diff_context_default;\n-\toptions->ws_error_highlight = WSEH_NEW;\n+\toptions->ws_error_highlight = ws_error_highlight_default;\n \tDIFF_OPT_SET(options, RENAME_EMPTY);\n \n \t/* pathchange left =NULL by default */\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 4a4f374824..289806d0c7 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -937,4 +937,39 @@ test_expect_success 'test --ws-error-highlight option' '\n \n '\n \n+test_expect_success 'test diff.wsErrorHighlight config' '\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=default,old diff |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.default-old current &&\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=all diff |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.all current &&\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=none diff |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.none current\n+\n+'\n+\n+test_expect_success 'option overrides diff.wsErrorHighlight' '\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=none \\\n+\t\tdiff --ws-error-highlight=default,old |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.default-old current &&\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=default \\\n+\t\tdiff --ws-error-highlight=all |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.all current &&\n+\n+\tgit -c color.diff=always -c diff.wsErrorHighlight=all \\\n+\t\tdiff --ws-error-highlight=none |\n+\ttest_decode_color >current &&\n+\ttest_cmp expect.none current\n+\n+'\n+\n test_done\n-- \n2.10.1-510-g1ef781f2c1\n\n"},{"id":"303331","messageId":"20161004225449.6759-4-gitster@pobox.com","threadId":"44213","inReplyTo":"20161004225449.6759-1-gitster@pobox.com","subject":"[PATCH 3/4] diff.c: move ws-error-highlight parsing helpers up","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-04T22:54:48Z","receivedAt":"2016-10-04T22:55:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These need to be usable from git_diff_ui_config() code to help\nparsing a configuration variable, so move them up.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 74 +++++++++++++++++++++++++++++++++---------------------------------\n 1 file changed, 37 insertions(+), 37 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d346378600..bd625cf3f7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -163,6 +163,43 @@ long parse_algorithm_value(const char *value)\n \treturn -1;\n }\n \n+static int parse_one_token(const char **arg, const char *token)\n+{\n+\tconst char *rest;\n+\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n+\t\t*arg = rest;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int parse_ws_error_highlight(const char *arg)\n+{\n+\tconst char *orig_arg = arg;\n+\tunsigned val = 0;\n+\n+\twhile (*arg) {\n+\t\tif (parse_one_token(&arg, \"none\"))\n+\t\t\tval = 0;\n+\t\telse if (parse_one_token(&arg, \"default\"))\n+\t\t\tval = WSEH_NEW;\n+\t\telse if (parse_one_token(&arg, \"all\"))\n+\t\t\tval = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT;\n+\t\telse if (parse_one_token(&arg, \"new\"))\n+\t\t\tval |= WSEH_NEW;\n+\t\telse if (parse_one_token(&arg, \"old\"))\n+\t\t\tval |= WSEH_OLD;\n+\t\telse if (parse_one_token(&arg, \"context\"))\n+\t\t\tval |= WSEH_CONTEXT;\n+\t\telse {\n+\t\t\treturn -1 - (int)(arg - orig_arg);\n+\t\t}\n+\t\tif (*arg)\n+\t\t\targ++;\n+\t}\n+\treturn val;\n+}\n+\n /*\n  * These are to give UI layer defaults.\n  * The core-level commands such as git-diff-files should\n@@ -3656,43 +3693,6 @@ static void enable_patch_output(int *fmt) {\n \t*fmt |= DIFF_FORMAT_PATCH;\n }\n \n-static int parse_one_token(const char **arg, const char *token)\n-{\n-\tconst char *rest;\n-\tif (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {\n-\t\t*arg = rest;\n-\t\treturn 1;\n-\t}\n-\treturn 0;\n-}\n-\n-static int parse_ws_error_highlight(const char *arg)\n-{\n-\tconst char *orig_arg = arg;\n-\tunsigned val = 0;\n-\n-\twhile (*arg) {\n-\t\tif (parse_one_token(&arg, \"none\"))\n-\t\t\tval = 0;\n-\t\telse if (parse_one_token(&arg, \"default\"))\n-\t\t\tval = WSEH_NEW;\n-\t\telse if (parse_one_token(&arg, \"all\"))\n-\t\t\tval = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT;\n-\t\telse if (parse_one_token(&arg, \"new\"))\n-\t\t\tval |= WSEH_NEW;\n-\t\telse if (parse_one_token(&arg, \"old\"))\n-\t\t\tval |= WSEH_OLD;\n-\t\telse if (parse_one_token(&arg, \"context\"))\n-\t\t\tval |= WSEH_CONTEXT;\n-\t\telse {\n-\t\t\treturn -1 - (int)(arg - orig_arg);\n-\t\t}\n-\t\tif (*arg)\n-\t\t\targ++;\n-\t}\n-\treturn val;\n-}\n-\n static int parse_ws_error_highlight_opt(struct diff_options *opt, const char *arg)\n {\n \tint val = parse_ws_error_highlight(arg);\n-- \n2.10.1-510-g1ef781f2c1\n\n"},{"id":"304328","messageId":"20161018110132.ehomp2rolb5q2npl@sigill.intra.peff.net","threadId":"44213","inReplyTo":"20161004225449.6759-1-gitster@pobox.com","subject":"Re: [PATCH 0/4] diff.wsErrorHighlight configuration variable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-18T11:01:32Z","receivedAt":"2016-10-18T11:02:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 04, 2016 at 03:54:45PM -0700, Junio C Hamano wrote:\n\n> \"git diff\" and its family of commands have \"--ws-error-highlight\"\n> option to allow whitespace breakages on old and context lines\n> painted in color.diff.whitespace color, instead of the usual \"we\n> paint breakages only on new lines\", but there wasn't a configuration\n> variable that corresponds to it.\n> \n> This would be a lot closer to a series that could be acceptable,\n> compared to the previous \"it should look like this\" patch.\n> \n> Junio C Hamano (4):\n>   t4015: split out the \"setup\" part of ws-error-highlight test\n>   diff.c: refactor parse_ws_error_highlight()\n>   diff.c: move ws-error-highlight parsing helpers up\n>   diff: introduce diff.wsErrorHighlight option\n\nThis topic got stuck in my dreaded \"to review\" pile and I forgot about\nit. I see you've already marked it for merging to master, but FWIW, I\nread it over and did not see any problems.\n\n-Peff\n"}]}