Skip to content

Commit 136c8c8

Browse files
peffgitster
authored andcommitted
color: check color.ui in git_default_config()
Back in prehistoric times, our decision on whether or not to show color by default relied on using a config callback that either did or didn't load color config like color.diff. When we introduced color.ui, we put it in the same boat: commands had to manually respect it by using git_color_config() or its git_color_default_config() convenience wrapper. But in 4c7f181 (make color.ui default to 'auto', 2013-06-10), that changed. Since then, we default color.ui to auto in all programs, meaning that even plumbing commands like "git diff-tree --pretty" might colorize the output. Nobody seems to have complained in the intervening years, presumably because the "is stdout a tty" check does a good job of catching the right cases. But that leaves an interesting curiosity: color.ui defaults to auto even in plumbing, but you can't actually _disable_ the color via config. So if you really hate color and set "color.ui" to false, diff-tree will still show color (but porcelain like git-diff won't). Nobody noticed that either, probably because very few people disable color. One could argue that the plumbing should _always_ disable color unless an explicit --color option is given on the command line. But in practice, this creates a lot of complications for scripts which do want plumbing to show user-visible output. They can't just pass "--color" blindly; they need to check the user's config and decide what to send. Given that nobody has complained about the current behavior, let's assume it's a good path, and follow it to its conclusion: supporting color.ui everywhere. Note that you can create havoc by setting color.ui=always in your config, but that's more or less already the case. We could disallow it entirely, but it is handy for one-offs like: git -c color.ui=always foo >not-a-tty when "foo" does not take a --color option itself. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ab7ded3 commit 136c8c8

File tree

7 files changed

+8
-16
lines changed

7 files changed

+8
-16
lines changed

builtin/branch.c

+1-1
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_color_default_config(var, value, cb);
95+
return git_default_config(var, value, cb);
9696
}
9797

9898
static const char *branch_get_color(enum color_branch ix)

builtin/clean.c

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

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

132131
static const char *clean_get_color(enum color_clean ix)

builtin/grep.c

+1-1
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_color_default_config(var, value, cb) < 0)
287+
if (git_default_config(var, value, cb) < 0)
288288
st = -1;
289289

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

builtin/show-branch.c

+1-1
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_color_default_config(var, value, cb);
557+
return git_default_config(var, value, cb);
558558
}
559559

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

color.c

-8
Original file line numberDiff line numberDiff line change
@@ -361,14 +361,6 @@ 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-
372364
void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
373365
{
374366
if (*color)

config.c

+4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "string-list.h"
1717
#include "utf8.h"
1818
#include "dir.h"
19+
#include "color.h"
1920

2021
struct config_source {
2122
struct config_source *prev;
@@ -1350,6 +1351,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
13501351
if (starts_with(var, "advice."))
13511352
return git_default_advice_config(var, value);
13521353

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

diff.c

-3
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,6 @@ 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-
305302
return git_diff_basic_config(var, value, cb);
306303
}
307304

0 commit comments

Comments
 (0)