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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/branch.c b/builtin/branch.c
index 16d391b407c9fa688b48a3e28f77800dcc1e2e0c..1969c7116cd1ae7cf3cf0f84eaf6ed333170113b 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -92,7 +92,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
                        return config_error_nonbool(var);
                return color_parse(value, branch_colors[slot]);
        }
-       return git_default_config(var, value, cb);
+       return git_color_default_config(var, value, cb);
 }
 
 static const char *branch_get_color(enum color_branch ix)

diff --git a/builtin/clean.c b/builtin/clean.c
index c1bafda5b63324b0aba7400535cc0ba745e46004..057fc97fe4494338e6a85ac9f695563f1fcb9596 100644 (file)
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -125,7 +125,8 @@ static int git_clean_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       return git_default_config(var, value, cb);
+       /* inspect the color.ui config variable and others */
+       return git_color_default_config(var, value, cb);
 }
 
 static const char *clean_get_color(enum color_clean ix)

diff --git a/builtin/grep.c b/builtin/grep.c
index 42ff87065a0e72c999488d2844463f901fd73a5c..7e79eb1a754a5f604829e3e9e6d98d83712be9b6 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -284,7 +284,7 @@ static int wait_all(void)
 static int grep_cmd_config(const char *var, const char *value, void *cb)
 {
        int st = grep_config(var, value, cb);
-       if (git_default_config(var, value, cb) < 0)
+       if (git_color_default_config(var, value, cb) < 0)
                st = -1;
 
        if (!strcmp(var, "grep.threads")) {

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 28f245c8cccbc20c96a189516e9df77d238c7e96..7073a3eb9769cae1f00e714a93528b33ca651d9c 100644 (file)
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -554,7 +554,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       return git_default_config(var, value, cb);
+       return git_color_default_config(var, value, cb);
 }
 
 static int omit_in_dense(struct commit *commit, struct commit **rev, int n)

diff --git a/color.c b/color.c
index 7aa8b076f045e5c0b74826153030e2923c0fc56e..31b6207a00de42a386e98c5656209ea7a010abe4 100644 (file)
--- 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)

diff --git a/config.c b/config.c
index acebc85d81372f15d3d490d84dc751f59bde26c4..231f9a750b96deda8d62f5a5debb384f40481566 100644 (file)
--- a/config.c
+++ b/config.c
@@ -16,7 +16,6 @@
 #include "string-list.h"
 #include "utf8.h"
 #include "dir.h"
-#include "color.h"
 
 struct config_source {
        struct config_source *prev;
@@ -1351,9 +1350,6 @@ int git_default_config(const char *var, const char *value, void *dummy)
        if (starts_with(var, "advice."))
                return git_default_advice_config(var, value);
 
-       if (git_color_config(var, value, dummy) < 0)
-               return -1;
-
        if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
                pager_use_color = git_config_bool(var,value);
                return 0;

diff --git a/diff.c b/diff.c
index 9c382580306e340ed6333f96bc4919c4c507a7b9..85e714f6c68d24e11228b69d2511c49811c979b4 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -299,6 +299,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
+       if (git_color_config(var, value, cb) < 0)
+               return -1;
+
        return git_diff_basic_config(var, value, cb);
 }
 

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 87ffd4d43c4aa9ef4e1cae63e078e708ddbd4b27..a49c12c79b28fac8545dffcbaba8e8905c8d6222 100755 (executable)
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -483,7 +483,7 @@ test_expect_success 'hunk-editing handles custom comment char' '
        git diff --exit-code
 '
 
-test_expect_failure 'add -p works even with color.ui=always' '
+test_expect_success 'add -p works even with color.ui=always' '
        git reset --hard &&
        echo change >>file &&
        test_config color.ui always &&