From 2c1acdf6c9f6668dc520e75709af8447587eb4d0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 Oct 2017 13:23:24 -0400 Subject: Revert "color: make "always" the same as "auto" in config" This reverts commit 6be4595edb8e5b616c6e8b9fbc78b0f831fa2a87. That commit weakened the "always" setting of color config so that it acted as "auto". This was meant to solve regressions in v2.14.2 in which setting "color.ui=always" in the on-disk config broke scripts like add--interactive, because the plumbing diff commands began to generate color output. This was due to 136c8c8b8f (color: check color.ui in git_default_config(), 2017-07-13), which was in turn trying to fix issues caused by 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10). But in weakening "always", we created even more problems, as people expect to be able to use "git -c color.ui=always" to force color (especially because some commands don't have their own --color flag). We can fix that by special-casing the command-line "-c", but now things are getting pretty confusing. Instead of piling hacks upon hacks, let's start peeling off the hacks. The first step is dropping the weakening of "always", which this revert does. Note that we could actually revert the whole series merged in by da15b78e52642bd45fd5513ab0000fdf2e58a6f4. Most of that series consists of preparations to the tests to handle the weakening of "-c color.ui=always". But it's worth keeping for a few reasons: - there are some other preparatory cleanups, like e433749d86 (test-terminal: set TERM=vt100, 2017-10-03) - it adds "--color" options more consistently in 0c88bf5050 (provide --color option for all ref-filter users, 2017-10-03) - some of the cases dropping "-c" end up being more robust and realistic tests, as in 01c94e9001 (t7508: use test_terminal for color output, 2017-10-03) - the preferred tool for overriding config is "--color", and we should be modeling that consistently We can individually revert the few commits necessary to restore some useful tests (which will be done on top of this patch). Note that this isn't a pure revert; we'll keep the test added in t3701, but mark it as failure for now. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- color.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'color.c') diff --git a/color.c b/color.c index 17e2713f96..7aa8b076f0 100644 --- a/color.c +++ b/color.c @@ -308,7 +308,7 @@ int git_config_colorbool(const char *var, const char *value) if (!strcasecmp(value, "never")) return 0; if (!strcasecmp(value, "always")) - return var ? GIT_COLOR_AUTO : 1; + return 1; if (!strcasecmp(value, "auto")) return GIT_COLOR_AUTO; } -- cgit v1.2.3 From 33c643bb083133376f3fdcb190ebc58f9eef12bb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 13 Oct 2017 13:24:31 -0400 Subject: Revert "color: check color.ui in git_default_config()" This reverts commit 136c8c8b8fa39f1315713248473dececf20f8fe7. That commit was trying to address a bug caused by 4c7f1819b3 (make color.ui default to 'auto', 2013-06-10), in which plumbing like diff-tree defaulted to "auto" color, but did not respect a "color.ui" directive to disable it. But it also meant that we started respecting "color.ui" set to "always". This was a known problem, but 4c7f1819b3 argued that nobody ought to be doing that. However, that turned out to be wrong, and we got a number of bug reports related to "add -p" regressing in v2.14.2. Let's revert 136c8c8b8, fixing the regression to "add -p". This leaves the problem from 4c7f1819b3 unfixed, but: 1. It's a pretty obscure problem in the first place. I only noticed it while working on the color code, and we haven't got a single bug report or complaint about it. 2. We can make a more moderate fix on top by respecting "never" but not "always" for plumbing commands. This is just the minimal fix to go back to the working state we had before v2.14.2. Note that this isn't a pure revert. We now have a test in t3701 which shows off the "add -p" regression. This can be flipped to success. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- color.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'color.c') diff --git a/color.c b/color.c index 7aa8b076f0..31b6207a00 100644 --- a/color.c +++ b/color.c @@ -361,6 +361,14 @@ int git_color_config(const char *var, const char *value, void *cb) return 0; } +int git_color_default_config(const char *var, const char *value, void *cb) +{ + if (git_color_config(var, value, cb) < 0) + return -1; + + return git_default_config(var, value, cb); +} + void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb) { if (*color) -- cgit v1.2.3