Skip to content

Commit 33c643b

Browse files
peffgitster
authored andcommitted
Revert "color: check color.ui in git_default_config()"
This reverts commit 136c8c8. That commit was trying to address a bug caused by 4c7f181 (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 4c7f181 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 136c8c8, fixing the regression to "add -p". This leaves the problem from 4c7f181 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 1d4b12f commit 33c643b

File tree

8 files changed

+17
-9
lines changed

8 files changed

+17
-9
lines changed

builtin/branch.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ static int git_branch_config(const char *var, const char *value, void *cb)
9292
return config_error_nonbool(var);
9393
return color_parse(value, branch_colors[slot]);
9494
}
95-
return git_default_config(var, value, cb);
95+
return git_color_default_config(var, value, cb);
9696
}
9797

9898
static const char *branch_get_color(enum color_branch ix)

builtin/clean.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ static int git_clean_config(const char *var, const char *value, void *cb)
125125
return 0;
126126
}
127127

128-
return git_default_config(var, value, cb);
128+
/* inspect the color.ui config variable and others */
129+
return git_color_default_config(var, value, cb);
129130
}
130131

131132
static const char *clean_get_color(enum color_clean ix)

builtin/grep.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ static int wait_all(void)
284284
static int grep_cmd_config(const char *var, const char *value, void *cb)
285285
{
286286
int st = grep_config(var, value, cb);
287-
if (git_default_config(var, value, cb) < 0)
287+
if (git_color_default_config(var, value, cb) < 0)
288288
st = -1;
289289

290290
if (!strcmp(var, "grep.threads")) {

builtin/show-branch.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
554554
return 0;
555555
}
556556

557-
return git_default_config(var, value, cb);
557+
return git_color_default_config(var, value, cb);
558558
}
559559

560560
static int omit_in_dense(struct commit *commit, struct commit **rev, int n)

color.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,14 @@ int git_color_config(const char *var, const char *value, void *cb)
361361
return 0;
362362
}
363363

364+
int git_color_default_config(const char *var, const char *value, void *cb)
365+
{
366+
if (git_color_config(var, value, cb) < 0)
367+
return -1;
368+
369+
return git_default_config(var, value, cb);
370+
}
371+
364372
void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
365373
{
366374
if (*color)

config.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include "string-list.h"
1717
#include "utf8.h"
1818
#include "dir.h"
19-
#include "color.h"
2019

2120
struct config_source {
2221
struct config_source *prev;
@@ -1351,9 +1350,6 @@ int git_default_config(const char *var, const char *value, void *dummy)
13511350
if (starts_with(var, "advice."))
13521351
return git_default_advice_config(var, value);
13531352

1354-
if (git_color_config(var, value, dummy) < 0)
1355-
return -1;
1356-
13571353
if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
13581354
pager_use_color = git_config_bool(var,value);
13591355
return 0;

diff.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
299299
return 0;
300300
}
301301

302+
if (git_color_config(var, value, cb) < 0)
303+
return -1;
304+
302305
return git_diff_basic_config(var, value, cb);
303306
}
304307

t/t3701-add-interactive.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ test_expect_success 'hunk-editing handles custom comment char' '
483483
git diff --exit-code
484484
'
485485

486-
test_expect_failure 'add -p works even with color.ui=always' '
486+
test_expect_success 'add -p works even with color.ui=always' '
487487
git reset --hard &&
488488
echo change >>file &&
489489
test_config color.ui always &&

0 commit comments

Comments
 (0)