{"thread":{"id":"49745","subject":"[PATCH] diff: differentiate error handling in parse_color_moved_ws","startedAt":"2018-11-02T21:23:21Z","lastAt":"2018-11-14T07:27:36Z","messageCount":6,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"362316","messageId":"20181102212316.208433-1-sbeller@google.com","threadId":"49745","inReplyTo":null,"subject":"[PATCH] diff: differentiate error handling in parse_color_moved_ws","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-02T21:23:16Z","receivedAt":"2018-11-02T21:23:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As we check command line options more strictly and allow configuration\nvariables to be parsed more leniently, we need take different actions\nbased on whether an unknown value is given on the command line or in the\nconfig.\n\nMove the die() call out of parse_color_moved_ws into the parsing\nof command line options. As the function returns a bit field, change\nits signature to return an unsigned instead of an int; add a new bit\nto signal errors. Once the error is signaled, we discard the other\nbits, such that it doesn't matter if the error bit overlaps with any\nother bit.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nThis is a fresh attempt to cleanup the sloppy part that was mentioned\nin https://public-inbox.org/git/xmqqa7nkf6o4.fsf@gitster-ct.c.googlers.com/\n\nAnother thing to follow up is to have color-moved-ws imply color-moved.\n\nThanks,\nStefan\n\n\n diff.c | 21 ++++++++++++++-------\n diff.h |  3 ++-\n 2 files changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8647db3d30..f21f8b0332 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -291,7 +291,7 @@ static int parse_color_moved(const char *arg)\n \t\treturn error(_(\"color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed-zebra', 'plain'\"));\n }\n \n-static int parse_color_moved_ws(const char *arg)\n+static unsigned parse_color_moved_ws(const char *arg)\n {\n \tint ret = 0;\n \tstruct string_list l = STRING_LIST_INIT_DUP;\n@@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)\n \t\t\tret |= XDF_IGNORE_WHITESPACE;\n \t\telse if (!strcmp(sb.buf, \"allow-indentation-change\"))\n \t\t\tret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;\n-\t\telse\n+\t\telse {\n+\t\t\tret |= COLOR_MOVED_WS_ERROR;\n \t\t\terror(_(\"ignoring unknown color-moved-ws mode '%s'\"), sb.buf);\n+\t\t}\n \n \t\tstrbuf_release(&sb);\n \t}\n \n \tif ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&\n-\t    (ret & XDF_WHITESPACE_FLAGS))\n-\t\tdie(_(\"color-moved-ws: allow-indentation-change cannot be combined with other white space modes\"));\n+\t    (ret & XDF_WHITESPACE_FLAGS)) {\n+\t\terror(_(\"color-moved-ws: allow-indentation-change cannot be combined with other white space modes\"));\n+\t\tret |= COLOR_MOVED_WS_ERROR;\n+\t}\n \n \tstring_list_clear(&l, 0);\n \n@@ -341,8 +345,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.colormovedws\")) {\n-\t\tint cm = parse_color_moved_ws(value);\n-\t\tif (cm < 0)\n+\t\tunsigned cm = parse_color_moved_ws(value);\n+\t\tif (cm & COLOR_MOVED_WS_ERROR)\n \t\t\treturn -1;\n \t\tdiff_color_moved_ws_default = cm;\n \t\treturn 0;\n@@ -5035,7 +5039,10 @@ int diff_opt_parse(struct diff_options *options,\n \t\t\tdie(\"bad --color-moved argument: %s\", arg);\n \t\toptions->color_moved = cm;\n \t} else if (skip_prefix(arg, \"--color-moved-ws=\", &arg)) {\n-\t\toptions->color_moved_ws_handling = parse_color_moved_ws(arg);\n+\t\tunsigned cm = parse_color_moved_ws(arg);\n+\t\tif (cm & COLOR_MOVED_WS_ERROR)\n+\t\t\tdie(\"bad --color-moved-ws argument: %s\", arg);\n+\t\toptions->color_moved_ws_handling = cm;\n \t} else if (skip_to_optional_arg_default(arg, \"--color-words\", &options->word_regex, NULL)) {\n \t\toptions->use_color = 1;\n \t\toptions->word_diff = DIFF_WORDS_COLOR;\ndiff --git a/diff.h b/diff.h\nindex ce5e8a8183..9e8061ca29 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -225,7 +225,8 @@ struct diff_options {\n \n \t/* XDF_WHITESPACE_FLAGS regarding block detection are set at 2, 3, 4 */\n \t#define COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE (1<<5)\n-\tint color_moved_ws_handling;\n+\t#define COLOR_MOVED_WS_ERROR (1<<0)\n+\tunsigned color_moved_ws_handling;\n \n \tstruct repository *repo;\n };\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362328","messageId":"xmqqzhurlzv2.fsf@gitster-ct.c.googlers.com","threadId":"49745","inReplyTo":"20181102212316.208433-1-sbeller@google.com","subject":"Re: [PATCH] diff: differentiate error handling in parse_color_moved_ws","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-03T01:21:53Z","receivedAt":"2018-11-03T01:21:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>  \n> -static int parse_color_moved_ws(const char *arg)\n> +static unsigned parse_color_moved_ws(const char *arg)\n>  {\n>  \tint ret = 0;\n>  \tstruct string_list l = STRING_LIST_INIT_DUP;\n> @@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)\n>  \t\t\tret |= XDF_IGNORE_WHITESPACE;\n>  \t\telse if (!strcmp(sb.buf, \"allow-indentation-change\"))\n>  \t\t\tret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;\n> -\t\telse\n> +\t\telse {\n> +\t\t\tret |= COLOR_MOVED_WS_ERROR;\n>  \t\t\terror(_(\"ignoring unknown color-moved-ws mode '%s'\"), sb.buf);\n> +\t\t}\n> ...  \n>  \t} else if (skip_prefix(arg, \"--color-moved-ws=\", &arg)) {\n> -\t\toptions->color_moved_ws_handling = parse_color_moved_ws(arg);\n> +\t\tunsigned cm = parse_color_moved_ws(arg);\n> +\t\tif (cm & COLOR_MOVED_WS_ERROR)\n> +\t\t\tdie(\"bad --color-moved-ws argument: %s\", arg);\n> +\t\toptions->color_moved_ws_handling = cm;\n\nExcellent.\n\nWill queue.  Perhaps a test or two can follow to ensure a bad value\nfrom config does not kill while a command line does?\n\nThanks.\n"},{"id":"362455","messageId":"xmqqk1lshx26.fsf@gitster-ct.c.googlers.com","threadId":"49745","inReplyTo":"xmqqzhurlzv2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] diff: differentiate error handling in parse_color_moved_ws","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-05T06:12:49Z","receivedAt":"2018-11-05T06:12:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Stefan Beller <sbeller@google.com> writes:\n>\n>>  \n>> -static int parse_color_moved_ws(const char *arg)\n>> +static unsigned parse_color_moved_ws(const char *arg)\n>>  {\n>>  \tint ret = 0;\n>>  \tstruct string_list l = STRING_LIST_INIT_DUP;\n>> @@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)\n>>  \t\t\tret |= XDF_IGNORE_WHITESPACE;\n>>  \t\telse if (!strcmp(sb.buf, \"allow-indentation-change\"))\n>>  \t\t\tret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;\n>> -\t\telse\n>> +\t\telse {\n>> +\t\t\tret |= COLOR_MOVED_WS_ERROR;\n>>  \t\t\terror(_(\"ignoring unknown color-moved-ws mode '%s'\"), sb.buf);\n>> +\t\t}\n>> ...  \n>>  \t} else if (skip_prefix(arg, \"--color-moved-ws=\", &arg)) {\n>> -\t\toptions->color_moved_ws_handling = parse_color_moved_ws(arg);\n>> +\t\tunsigned cm = parse_color_moved_ws(arg);\n>> +\t\tif (cm & COLOR_MOVED_WS_ERROR)\n>> +\t\t\tdie(\"bad --color-moved-ws argument: %s\", arg);\n>> +\t\toptions->color_moved_ws_handling = cm;\n>\n> Excellent.\n>\n> Will queue.  Perhaps a test or two can follow to ensure a bad value\n> from config does not kill while a command line does?\n\nWait.  This does not fix\n\n\tgit -c diff.colormovedws=nonsense diff\n\nthat dies with an error message---it should ignore the config and at\nmoat issue a warning.\n\nThe command line handling of\n\n\tgit diff --color-moved-ws=nonsense\n\ndoes correctly die, but it first says \"error: ignoring\" before\nsaying \"fatal: bad argument\", which is suboptimal.\n\nSo, not so excellent (yet) X-<.\n\n"},{"id":"362497","messageId":"CAGZ79kbJvX1Y8-iiAzXKKam7hy=pg-5+p_rzDse42-oCswMXSQ@mail.gmail.com","threadId":"49745","inReplyTo":"xmqqk1lshx26.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] diff: differentiate error handling in parse_color_moved_ws","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-05T18:19:58Z","receivedAt":"2018-11-05T18:20:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Nov 4, 2018 at 10:12 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n> > Stefan Beller <sbeller@google.com> writes:\n> >\n> >>\n> >> -static int parse_color_moved_ws(const char *arg)\n> >> +static unsigned parse_color_moved_ws(const char *arg)\n> >>  {\n> >>      int ret = 0;\n> >>      struct string_list l = STRING_LIST_INIT_DUP;\n> >> @@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)\n> >>                      ret |= XDF_IGNORE_WHITESPACE;\n> >>              else if (!strcmp(sb.buf, \"allow-indentation-change\"))\n> >>                      ret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;\n> >> -            else\n> >> +            else {\n> >> +                    ret |= COLOR_MOVED_WS_ERROR;\n> >>                      error(_(\"ignoring unknown color-moved-ws mode '%s'\"), sb.buf);\n> >> +            }\n> >> ...\n> >>      } else if (skip_prefix(arg, \"--color-moved-ws=\", &arg)) {\n> >> -            options->color_moved_ws_handling = parse_color_moved_ws(arg);\n> >> +            unsigned cm = parse_color_moved_ws(arg);\n> >> +            if (cm & COLOR_MOVED_WS_ERROR)\n> >> +                    die(\"bad --color-moved-ws argument: %s\", arg);\n> >> +            options->color_moved_ws_handling = cm;\n> >\n> > Excellent.\n> >\n> > Will queue.  Perhaps a test or two can follow to ensure a bad value\n> > from config does not kill while a command line does?\n>\n> Wait.  This does not fix\n>\n>         git -c diff.colormovedws=nonsense diff\n>\n> that dies with an error message---it should ignore the config and at\n> moat issue a warning.\n\n$ git -c core.abbrev=41 diff\nerror: abbrev length out of range: 41\nfatal: unable to parse 'core.abbrev' from command-line config\n$ ./git -c  diff.colormovedws=nonsense diff HEAD\nerror: ignoring unknown color-moved-ws mode 'nonsense'\nfatal: unable to parse 'diff.colormovedws' from command-line config\n\nAh, I see the issue there. We actually have to return 'success' to the\nconfig machinery after the warning claiming ignoring the setting or\nwe'd have to reword the warning to state we're not ignoring the bogus\nsetting.\n\n> The command line handling of\n>\n>         git diff --color-moved-ws=nonsense\n>\n> does correctly die, but it first says \"error: ignoring\" before\n> saying \"fatal: bad argument\", which is suboptimal.\n\nSo to find the analogous here, maybe:\n\n$ git diff --color=bogus\nerror: option `color' expects \"always\", \"auto\", or \"never\"\n\n> So, not so excellent (yet) X-<.\n\nSo to reach excellence, we'd want to reword the warning\nmessage and a test.\n\nThanks,\nStefan\n"},{"id":"363220","messageId":"20181113213357.205769-1-sbeller@google.com","threadId":"49745","inReplyTo":"CAGZ79kbJvX1Y8-iiAzXKKam7hy=pg-5+p_rzDse42-oCswMXSQ@mail.gmail.com","subject":"[PATCH] diff: align move detection error handling with other options","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-13T21:33:57Z","receivedAt":"2018-11-13T21:34:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This changes the error handling for the options --color-moved-ws\nand --color-moved-ws to be like the rest of the options.\n\nMove the die() call out of parse_color_moved_ws into the parsing\nof command line options. As the function returns a bit field, change\nits signature to return an unsigned instead of an int; add a new bit\nto signal errors. Once the error is signaled, we discard the other\nbits, such that it doesn't matter if the error bit overlaps with any\nother bit.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nc.f.\n./git -c diff.colormovedws=bogus diff HEAD\nerror: unknown color-moved-ws mode 'bogus'\nfatal: unable to parse 'diff.colormovedws' from command-line config\n./git -c core.abbrev=41 diff\nerror: abbrev length out of range: 41\nfatal: unable to parse 'core.abbrev' from command-line config\n\n./git diff --color=bogus\nerror: option `color' expects \"always\", \"auto\", or \"never\"\n./git -c diff.colormovedws=bogus diff HEAD\nerror: unknown color-moved-ws mode 'bogus'\nfatal: unable to parse 'diff.colormovedws' from command-line config\n\n\n diff.c                     | 25 ++++++++++++++++---------\n diff.h                     |  3 ++-\n t/t4015-diff-whitespace.sh | 18 ++++++++++++++++++\n 3 files changed, 36 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8647db3d30..d7d467b605 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -291,7 +291,7 @@ static int parse_color_moved(const char *arg)\n \t\treturn error(_(\"color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed-zebra', 'plain'\"));\n }\n \n-static int parse_color_moved_ws(const char *arg)\n+static unsigned parse_color_moved_ws(const char *arg)\n {\n \tint ret = 0;\n \tstruct string_list l = STRING_LIST_INIT_DUP;\n@@ -312,15 +312,19 @@ static int parse_color_moved_ws(const char *arg)\n \t\t\tret |= XDF_IGNORE_WHITESPACE;\n \t\telse if (!strcmp(sb.buf, \"allow-indentation-change\"))\n \t\t\tret |= COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE;\n-\t\telse\n-\t\t\terror(_(\"ignoring unknown color-moved-ws mode '%s'\"), sb.buf);\n+\t\telse {\n+\t\t\tret |= COLOR_MOVED_WS_ERROR;\n+\t\t\terror(_(\"unknown color-moved-ws mode '%s', possible values are 'ignore-space-change', 'ignore-space-at-eol', 'ignore-all-space', 'allow-indentation-change'\"), sb.buf);\n+\t\t}\n \n \t\tstrbuf_release(&sb);\n \t}\n \n \tif ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&\n-\t    (ret & XDF_WHITESPACE_FLAGS))\n-\t\tdie(_(\"color-moved-ws: allow-indentation-change cannot be combined with other white space modes\"));\n+\t    (ret & XDF_WHITESPACE_FLAGS)) {\n+\t\terror(_(\"color-moved-ws: allow-indentation-change cannot be combined with other white space modes\"));\n+\t\tret |= COLOR_MOVED_WS_ERROR;\n+\t}\n \n \tstring_list_clear(&l, 0);\n \n@@ -341,8 +345,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \tif (!strcmp(var, \"diff.colormovedws\")) {\n-\t\tint cm = parse_color_moved_ws(value);\n-\t\tif (cm < 0)\n+\t\tunsigned cm = parse_color_moved_ws(value);\n+\t\tif (cm & COLOR_MOVED_WS_ERROR)\n \t\t\treturn -1;\n \t\tdiff_color_moved_ws_default = cm;\n \t\treturn 0;\n@@ -5032,10 +5036,13 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (skip_prefix(arg, \"--color-moved=\", &arg)) {\n \t\tint cm = parse_color_moved(arg);\n \t\tif (cm < 0)\n-\t\t\tdie(\"bad --color-moved argument: %s\", arg);\n+\t\t\treturn error(\"bad --color-moved argument: %s\", arg);\n \t\toptions->color_moved = cm;\n \t} else if (skip_prefix(arg, \"--color-moved-ws=\", &arg)) {\n-\t\toptions->color_moved_ws_handling = parse_color_moved_ws(arg);\n+\t\tunsigned cm = parse_color_moved_ws(arg);\n+\t\tif (cm & COLOR_MOVED_WS_ERROR)\n+\t\t\treturn -1;\n+\t\toptions->color_moved_ws_handling = cm;\n \t} else if (skip_to_optional_arg_default(arg, \"--color-words\", &options->word_regex, NULL)) {\n \t\toptions->use_color = 1;\n \t\toptions->word_diff = DIFF_WORDS_COLOR;\ndiff --git a/diff.h b/diff.h\nindex ce5e8a8183..9e8061ca29 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -225,7 +225,8 @@ struct diff_options {\n \n \t/* XDF_WHITESPACE_FLAGS regarding block detection are set at 2, 3, 4 */\n \t#define COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE (1<<5)\n-\tint color_moved_ws_handling;\n+\t#define COLOR_MOVED_WS_ERROR (1<<0)\n+\tunsigned color_moved_ws_handling;\n \n \tstruct repository *repo;\n };\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex a9fb226c5a..9a3e4fdfec 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -1890,6 +1890,24 @@ test_expect_success 'compare whitespace delta across moved blocks' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'bogus settings in move detection erroring out' '\n+\ttest_must_fail git diff --color-moved=bogus 2>err &&\n+\ttest_i18ngrep \"must be one of\" err &&\n+\ttest_i18ngrep bogus err &&\n+\n+\ttest_must_fail git -c diff.colormoved=bogus diff 2>err &&\n+\ttest_i18ngrep \"must be one of\" err &&\n+\ttest_i18ngrep \"from command-line config\" err &&\n+\n+\ttest_must_fail git diff --color-moved-ws=bogus 2>err &&\n+\ttest_i18ngrep \"possible values\" err &&\n+\ttest_i18ngrep bogus err &&\n+\n+\ttest_must_fail git -c diff.colormovedws=bogus diff 2>err &&\n+\ttest_i18ngrep \"possible values\" err &&\n+\ttest_i18ngrep \"from command-line config\" err\n+'\n+\n test_expect_success 'compare whitespace delta incompatible with other space options' '\n \ttest_must_fail git diff \\\n \t\t--color-moved-ws=allow-indentation-change,ignore-all-space \\\n-- \n2.19.1.1215.g8438c0b245-goog\n\n"},{"id":"363328","messageId":"xmqqa7mcnmoz.fsf@gitster-ct.c.googlers.com","threadId":"49745","inReplyTo":"20181113213357.205769-1-sbeller@google.com","subject":"Re: [PATCH] diff: align move detection error handling with other options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T07:27:24Z","receivedAt":"2018-11-14T07:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>Subject: Re: [PATCH] diff: align move detection error handling with other options\n\nWhen sending an updated version of existing topic, please make sure\nyou indicate as such with v$n etc.  I will assume that this is to\nreplace the patch queued on sb/diff-color-moved-config-option-fixup\ntopic.  Please do not assume that all messages on the References:\nheader are visible in the readers' MUA to show which thread it is a\nresponse to.  At least a hint like v$n (or mentioning the name of\nthe topic branch if the previous round is already in 'pu') would\nmake the reader realize that the References: header can be used if\nthe reader wants to find in what context the patch is relevant,\nespecially when redoing a change whose previous round is more than a\nweek old, as we see too many changes in a day already.\n\n> This changes the error handling for the options --color-moved-ws\n> and --color-moved-ws to be like the rest of the options.\n>\n> Move the die() call out of parse_color_moved_ws into the parsing\n> of command line options. As the function returns a bit field, change\n> its signature to return an unsigned instead of an int; add a new bit\n> to signal errors. Once the error is signaled, we discard the other\n> bits, such that it doesn't matter if the error bit overlaps with any\n> other bit.\n\nOK.  That sound better than the original.\n\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n> c.f.\n> ./git -c diff.colormovedws=bogus diff HEAD\n> error: unknown color-moved-ws mode 'bogus'\n> fatal: unable to parse 'diff.colormovedws' from command-line config\n\nThese double messages may be something we want to fix eventually,\nbut I think that the issue is not specific to the config callback of\ndiff API, but something the config API needs to support to allow its\nusers produce a better single message (the first part is merely\ngiving a more detailed explanation why Git was unable to parse, and\nshould ideally be folded into the second part).\n\nMore generally, even if git_diff_ui_config() was called, as long as\nwe do not do the \"--color-moved\" and \"--color-moved-ws\" operations,\nthe user shouldn't even get an \"unknown mode\" message, let alone\n\"fatal\".  The above (and other existing uses of \"return -1\"s in the\nsame config callback) should actually become an example of what not\nto do, as \"diff HEAD\" does not *care* what value that variable has.\n\nBut again, fixing that anti-pattern is a much larger change.  Once\nwe move diff.c to the more modern git_config_get*() based interface,\ninstead of the old style git_config() callback based interface, it\nwould become easier to update it so that the complaint will be given\n(and kill the command) only when a variable whose value actually\n_matters_ is unparsable.\n\nThanks.\n"}]}