From f1c9626105d5e4962a5ccaa4620114d03f32ad02 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 17 Aug 2011 22:03:12 -0700 Subject: diff: refactor COLOR_DIFF from a flag into an int This lets us store more than just a bit flag for whether we want color; we can also store whether we want automatic colors. This can be useful for making the automatic-color decision closer to the point of use. This mostly just involves replacing DIFF_OPT_* calls with manipulations of the flag. The biggest exception is that calls to DIFF_OPT_TST must check for "o->use_color > 0", which lets an "unknown" value (i.e., the default) stay at "no color". In the previous code, a value of "-1" was not propagated at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 9038f190ec..cd5ad7525b 100644 --- a/diff.c +++ b/diff.c @@ -583,11 +583,10 @@ static void emit_rewrite_diff(const char *name_a, struct diff_options *o) { int lc_a, lc_b; - int color_diff = DIFF_OPT_TST(o, COLOR_DIFF); const char *name_a_tab, *name_b_tab; - const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO); - const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO); - const char *reset = diff_get_color(color_diff, DIFF_RESET); + const char *metainfo = diff_get_color(o->use_color, DIFF_METAINFO); + const char *fraginfo = diff_get_color(o->use_color, DIFF_FRAGINFO); + const char *reset = diff_get_color(o->use_color, DIFF_RESET); static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT; const char *a_prefix, *b_prefix; char *data_one, *data_two; @@ -623,7 +622,7 @@ static void emit_rewrite_diff(const char *name_a, size_two = fill_textconv(textconv_two, two, &data_two); memset(&ecbdata, 0, sizeof(ecbdata)); - ecbdata.color_diff = color_diff; + ecbdata.color_diff = o->use_color > 0; ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); ecbdata.opt = o; @@ -1004,7 +1003,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata) const char *diff_get_color(int diff_use_color, enum color_diff ix) { - if (diff_use_color) + if (diff_use_color > 0) return diff_colors[ix]; return ""; } @@ -1786,11 +1785,10 @@ static int is_conflict_marker(const char *line, int marker_size, unsigned long l static void checkdiff_consume(void *priv, char *line, unsigned long len) { struct checkdiff_t *data = priv; - int color_diff = DIFF_OPT_TST(data->o, COLOR_DIFF); int marker_size = data->conflict_marker_size; - const char *ws = diff_get_color(color_diff, DIFF_WHITESPACE); - const char *reset = diff_get_color(color_diff, DIFF_RESET); - const char *set = diff_get_color(color_diff, DIFF_FILE_NEW); + const char *ws = diff_get_color(data->o->use_color, DIFF_WHITESPACE); + const char *reset = diff_get_color(data->o->use_color, DIFF_RESET); + const char *set = diff_get_color(data->o->use_color, DIFF_FILE_NEW); char *err; char *line_prefix = ""; struct strbuf *msgbuf; @@ -2135,7 +2133,7 @@ static void builtin_diff(const char *name_a, memset(&xecfg, 0, sizeof(xecfg)); memset(&ecbdata, 0, sizeof(ecbdata)); ecbdata.label_path = lbl; - ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF); + ecbdata.color_diff = o->use_color > 0; ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); if (ecbdata.ws_rule & WS_BLANK_AT_EOF) @@ -2183,7 +2181,7 @@ static void builtin_diff(const char *name_a, break; } } - if (DIFF_OPT_TST(o, COLOR_DIFF)) { + if (o->use_color > 0) { struct diff_words_style *st = ecbdata.diff_words->style; st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD); st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW); @@ -2833,7 +2831,7 @@ static void run_diff_cmd(const char *pgm, */ fill_metainfo(msg, name, other, one, two, o, p, &must_show_header, - DIFF_OPT_TST(o, COLOR_DIFF) && !pgm); + o->use_color > 0 && !pgm); xfrm_msg = msg->len ? msg->buf : NULL; } @@ -2999,8 +2997,7 @@ void diff_setup(struct diff_options *options) options->change = diff_change; options->add_remove = diff_addremove; - if (diff_use_color_default > 0) - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = diff_use_color_default; options->detect_rename = diff_detect_rename_default; if (diff_no_prefix) { @@ -3374,24 +3371,24 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (!strcmp(arg, "--follow")) DIFF_OPT_SET(options, FOLLOW_RENAMES); else if (!strcmp(arg, "--color")) - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = 1; else if (!prefixcmp(arg, "--color=")) { int value = git_config_colorbool(NULL, arg+8, -1); if (value == 0) - DIFF_OPT_CLR(options, COLOR_DIFF); + options->use_color = 0; else if (value > 0) - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = 1; else return error("option `color' expects \"always\", \"auto\", or \"never\""); } else if (!strcmp(arg, "--no-color")) - DIFF_OPT_CLR(options, COLOR_DIFF); + options->use_color = 0; else if (!strcmp(arg, "--color-words")) { - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = 1; options->word_diff = DIFF_WORDS_COLOR; } else if (!prefixcmp(arg, "--color-words=")) { - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = 1; options->word_diff = DIFF_WORDS_COLOR; options->word_regex = arg + 14; } @@ -3404,7 +3401,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) if (!strcmp(type, "plain")) options->word_diff = DIFF_WORDS_PLAIN; else if (!strcmp(type, "color")) { - DIFF_OPT_SET(options, COLOR_DIFF); + options->use_color = 1; options->word_diff = DIFF_WORDS_COLOR; } else if (!strcmp(type, "porcelain")) -- cgit 1.2.3-korg From e269eb7946d0a4ba6a4e175133b5479446ac04a5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 17 Aug 2011 22:03:48 -0700 Subject: git_config_colorbool: refactor stdout_is_tty handling Usually this function figures out for itself whether stdout is a tty. However, it has an extra parameter just to allow git-config to override the auto-detection for its --get-colorbool option. Instead of an extra parameter, let's just use a global variable. This makes calling easier in the common case, and will make refactoring the colorbool code much simpler. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 2 +- builtin/commit.c | 2 +- builtin/config.c | 23 +++++++---------------- builtin/grep.c | 2 +- builtin/show-branch.c | 2 +- color.c | 11 ++++++----- color.h | 8 +++++++- diff.c | 4 ++-- parse-options.c | 2 +- 9 files changed, 27 insertions(+), 29 deletions(-) (limited to 'diff.c') diff --git a/builtin/branch.c b/builtin/branch.c index 3142daa57a..b15fee5e31 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -71,7 +71,7 @@ static int parse_branch_color_slot(const char *var, int ofs) static int git_branch_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "color.branch")) { - branch_use_color = git_config_colorbool(var, value, -1); + branch_use_color = git_config_colorbool(var, value); return 0; } if (!prefixcmp(var, "color.branch.")) { diff --git a/builtin/commit.c b/builtin/commit.c index e1af9b19f0..295803a265 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1144,7 +1144,7 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; } if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) { - s->use_color = git_config_colorbool(k, v, -1); + s->use_color = git_config_colorbool(k, v); return 0; } if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) { diff --git a/builtin/config.c b/builtin/config.c index 211e118d57..5505ced8c1 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -303,24 +303,17 @@ static void get_color(const char *def_color) fputs(parsed_color, stdout); } -static int stdout_is_tty; static int get_colorbool_found; static int get_diff_color_found; static int git_get_colorbool_config(const char *var, const char *value, void *cb) { - if (!strcmp(var, get_colorbool_slot)) { - get_colorbool_found = - git_config_colorbool(var, value, stdout_is_tty); - } - if (!strcmp(var, "diff.color")) { - get_diff_color_found = - git_config_colorbool(var, value, stdout_is_tty); - } - if (!strcmp(var, "color.ui")) { - git_use_color_default = git_config_colorbool(var, value, stdout_is_tty); - return 0; - } + if (!strcmp(var, get_colorbool_slot)) + get_colorbool_found = git_config_colorbool(var, value); + else if (!strcmp(var, "diff.color")) + get_diff_color_found = git_config_colorbool(var, value); + else if (!strcmp(var, "color.ui")) + git_use_color_default = git_config_colorbool(var, value); return 0; } @@ -510,9 +503,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } else if (actions == ACTION_GET_COLORBOOL) { if (argc == 1) - stdout_is_tty = git_config_bool("command line", argv[0]); - else if (argc == 0) - stdout_is_tty = isatty(1); + color_stdout_is_tty = git_config_bool("command line", argv[0]); return get_colorbool(argc != 0); } diff --git a/builtin/grep.c b/builtin/grep.c index 871afaa3c7..c17d7de562 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -316,7 +316,7 @@ static int grep_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "color.grep")) - opt->color = git_config_colorbool(var, value, -1); + opt->color = git_config_colorbool(var, value); else if (!strcmp(var, "color.grep.context")) color = opt->color_context; else if (!strcmp(var, "color.grep.filename")) diff --git a/builtin/show-branch.c b/builtin/show-branch.c index facc63a79e..e6650b4837 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -573,7 +573,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "color.showbranch")) { - showbranch_use_color = git_config_colorbool(var, value, -1); + showbranch_use_color = git_config_colorbool(var, value); return 0; } diff --git a/color.c b/color.c index 3db214c247..67affa4765 100644 --- a/color.c +++ b/color.c @@ -2,6 +2,7 @@ #include "color.h" int git_use_color_default = 0; +int color_stdout_is_tty = -1; /* * The list of available column colors. @@ -157,7 +158,7 @@ void color_parse_mem(const char *value, int value_len, const char *var, die("bad color value '%.*s' for variable '%s'", value_len, value, var); } -int git_config_colorbool(const char *var, const char *value, int stdout_is_tty) +int git_config_colorbool(const char *var, const char *value) { if (value) { if (!strcasecmp(value, "never")) @@ -177,9 +178,9 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty) /* any normal truth value defaults to 'auto' */ auto_color: - if (stdout_is_tty < 0) - stdout_is_tty = isatty(1); - if (stdout_is_tty || (pager_in_use() && pager_use_color)) { + if (color_stdout_is_tty < 0) + color_stdout_is_tty = isatty(1); + if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) { char *term = getenv("TERM"); if (term && strcmp(term, "dumb")) return 1; @@ -190,7 +191,7 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty) int git_color_default_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "color.ui")) { - git_use_color_default = git_config_colorbool(var, value, -1); + git_use_color_default = git_config_colorbool(var, value); return 0; } diff --git a/color.h b/color.h index 68a926a2cd..a190a2522b 100644 --- a/color.h +++ b/color.h @@ -57,12 +57,18 @@ extern int git_use_color_default; extern const char *column_colors_ansi[]; extern const int column_colors_ansi_max; +/* + * Generally the color code will lazily figure this out itself, but + * this provides a mechanism for callers to override autodetection. + */ +extern int color_stdout_is_tty; + /* * Use this instead of git_default_config if you need the value of color.ui. */ int git_color_default_config(const char *var, const char *value, void *cb); -int git_config_colorbool(const char *var, const char *value, int stdout_is_tty); +int git_config_colorbool(const char *var, const char *value); void color_parse(const char *value, const char *var, char *dst); void color_parse_mem(const char *value, int len, const char *var, char *dst); __attribute__((format (printf, 3, 4))) diff --git a/diff.c b/diff.c index cd5ad7525b..1f87f23190 100644 --- a/diff.c +++ b/diff.c @@ -137,7 +137,7 @@ static int git_config_rename(const char *var, const char *value) int git_diff_ui_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) { - diff_use_color_default = git_config_colorbool(var, value, -1); + diff_use_color_default = git_config_colorbool(var, value); return 0; } if (!strcmp(var, "diff.renames")) { @@ -3373,7 +3373,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (!strcmp(arg, "--color")) options->use_color = 1; else if (!prefixcmp(arg, "--color=")) { - int value = git_config_colorbool(NULL, arg+8, -1); + int value = git_config_colorbool(NULL, arg+8); if (value == 0) options->use_color = 0; else if (value > 0) diff --git a/parse-options.c b/parse-options.c index 73bd28ad90..12a5b25d09 100644 --- a/parse-options.c +++ b/parse-options.c @@ -620,7 +620,7 @@ int parse_opt_color_flag_cb(const struct option *opt, const char *arg, if (!arg) arg = unset ? "never" : (const char *)opt->defval; - value = git_config_colorbool(NULL, arg, -1); + value = git_config_colorbool(NULL, arg); if (value < 0) return opterror(opt, "expects \"always\", \"auto\", or \"never\"", 0); -- cgit 1.2.3-korg From daa0c3d9717624a62ce669252be832db12658ec0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 17 Aug 2011 22:04:23 -0700 Subject: color: delay auto-color decision until point of use When we read a color value either from a config file or from the command line, we use git_config_colorbool to convert it from the tristate always/never/auto into a single yes/no boolean value. This has some timing implications with respect to starting a pager. If we start (or decide not to start) the pager before checking the colorbool, everything is fine. Either isatty(1) will give us the right information, or we will properly check for pager_in_use(). However, if we decide to start a pager after we have checked the colorbool, things are not so simple. If stdout is a tty, then we will have already decided to use color. However, the user may also have configured color.pager not to use color with the pager. In this case, we need to actually turn off color. Unfortunately, the pager code has no idea which color variables were turned on (and there are many of them throughout the code, and they may even have been manipulated after the colorbool selection by something like "--color" on the command line). This bug can be seen any time a pager is started after config and command line options are checked. This has affected "git diff" since 89d07f7 (diff: don't run pager if user asked for a diff style exit code, 2007-08-12). It has also affect the log family since 1fda91b (Fix 'git log' early pager startup error case, 2010-08-24). This patch splits the notion of parsing a colorbool and actually checking the configuration. The "use_color" variables now have an additional possible value, GIT_COLOR_AUTO. Users of the variable should use the new "want_color()" wrapper, which will lazily determine and cache the auto-color decision. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 2 +- builtin/config.c | 2 ++ builtin/show-branch.c | 4 ++-- color.c | 20 ++++++++++++++++++-- color.h | 11 +++++++++++ diff.c | 17 +++++++---------- graph.c | 2 +- grep.c | 2 +- log-tree.c | 2 +- t/t7006-pager.sh | 12 ++++++++++++ wt-status.c | 4 +++- 11 files changed, 59 insertions(+), 19 deletions(-) (limited to 'diff.c') diff --git a/builtin/branch.c b/builtin/branch.c index b15fee5e31..d6d3c7d85b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -88,7 +88,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) static const char *branch_get_color(enum color_branch ix) { - if (branch_use_color > 0) + if (want_color(branch_use_color)) return branch_colors[ix]; return ""; } diff --git a/builtin/config.c b/builtin/config.c index 5505ced8c1..3a092966d2 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -330,6 +330,8 @@ static int get_colorbool(int print) get_colorbool_found = git_use_color_default; } + get_colorbool_found = want_color(get_colorbool_found); + if (print) { printf("%s\n", get_colorbool_found ? "true" : "false"); return 0; diff --git a/builtin/show-branch.c b/builtin/show-branch.c index e6650b4837..4b726fabde 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -26,14 +26,14 @@ static const char **default_arg; static const char *get_color_code(int idx) { - if (showbranch_use_color) + if (want_color(showbranch_use_color)) return column_colors_ansi[idx % column_colors_ansi_max]; return ""; } static const char *get_color_reset_code(void) { - if (showbranch_use_color) + if (want_color(showbranch_use_color)) return GIT_COLOR_RESET; return ""; } diff --git a/color.c b/color.c index 67affa4765..85864176a6 100644 --- a/color.c +++ b/color.c @@ -166,7 +166,7 @@ int git_config_colorbool(const char *var, const char *value) if (!strcasecmp(value, "always")) return 1; if (!strcasecmp(value, "auto")) - goto auto_color; + return GIT_COLOR_AUTO; } if (!var) @@ -177,7 +177,11 @@ int git_config_colorbool(const char *var, const char *value) return 0; /* any normal truth value defaults to 'auto' */ - auto_color: + return GIT_COLOR_AUTO; +} + +static int check_auto_color(void) +{ if (color_stdout_is_tty < 0) color_stdout_is_tty = isatty(1); if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) { @@ -188,6 +192,18 @@ int git_config_colorbool(const char *var, const char *value) return 0; } +int want_color(int var) +{ + static int want_auto = -1; + + if (var == GIT_COLOR_AUTO) { + if (want_auto < 0) + want_auto = check_auto_color(); + return want_auto; + } + return var > 0; +} + int git_color_default_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "color.ui")) { diff --git a/color.h b/color.h index a190a2522b..b413e0eace 100644 --- a/color.h +++ b/color.h @@ -48,6 +48,16 @@ struct strbuf; /* A special value meaning "no color selected" */ #define GIT_COLOR_NIL "NIL" +/* + * The first three are chosen to match common usage in the code, and what is + * returned from git_config_colorbool. The "auto" value can be returned from + * config_colorbool, and will be converted by want_color() into either 0 or 1. + */ +#define GIT_COLOR_UNKNOWN -1 +#define GIT_COLOR_NEVER 0 +#define GIT_COLOR_ALWAYS 1 +#define GIT_COLOR_AUTO 2 + /* * This variable stores the value of color.ui */ @@ -69,6 +79,7 @@ extern int color_stdout_is_tty; int git_color_default_config(const char *var, const char *value, void *cb); int git_config_colorbool(const char *var, const char *value); +int want_color(int var); void color_parse(const char *value, const char *var, char *dst); void color_parse_mem(const char *value, int len, const char *var, char *dst); __attribute__((format (printf, 3, 4))) diff --git a/diff.c b/diff.c index 1f87f23190..0496cdc019 100644 --- a/diff.c +++ b/diff.c @@ -622,7 +622,7 @@ static void emit_rewrite_diff(const char *name_a, size_two = fill_textconv(textconv_two, two, &data_two); memset(&ecbdata, 0, sizeof(ecbdata)); - ecbdata.color_diff = o->use_color > 0; + ecbdata.color_diff = want_color(o->use_color); ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); ecbdata.opt = o; @@ -1003,7 +1003,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata) const char *diff_get_color(int diff_use_color, enum color_diff ix) { - if (diff_use_color > 0) + if (want_color(diff_use_color)) return diff_colors[ix]; return ""; } @@ -2133,7 +2133,7 @@ static void builtin_diff(const char *name_a, memset(&xecfg, 0, sizeof(xecfg)); memset(&ecbdata, 0, sizeof(ecbdata)); ecbdata.label_path = lbl; - ecbdata.color_diff = o->use_color > 0; + ecbdata.color_diff = want_color(o->use_color); ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); if (ecbdata.ws_rule & WS_BLANK_AT_EOF) @@ -2181,7 +2181,7 @@ static void builtin_diff(const char *name_a, break; } } - if (o->use_color > 0) { + if (want_color(o->use_color)) { struct diff_words_style *st = ecbdata.diff_words->style; st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD); st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW); @@ -2831,7 +2831,7 @@ static void run_diff_cmd(const char *pgm, */ fill_metainfo(msg, name, other, one, two, o, p, &must_show_header, - o->use_color > 0 && !pgm); + want_color(o->use_color) && !pgm); xfrm_msg = msg->len ? msg->buf : NULL; } @@ -3374,12 +3374,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) options->use_color = 1; else if (!prefixcmp(arg, "--color=")) { int value = git_config_colorbool(NULL, arg+8); - if (value == 0) - options->use_color = 0; - else if (value > 0) - options->use_color = 1; - else + if (value < 0) return error("option `color' expects \"always\", \"auto\", or \"never\""); + options->use_color = value; } else if (!strcmp(arg, "--no-color")) options->use_color = 0; diff --git a/graph.c b/graph.c index 556834a973..7358416a72 100644 --- a/graph.c +++ b/graph.c @@ -347,7 +347,7 @@ static struct commit_list *first_interesting_parent(struct git_graph *graph) static unsigned short graph_get_current_column_color(const struct git_graph *graph) { - if (graph->revs->diffopt.use_color <= 0) + if (!want_color(graph->revs->diffopt.use_color)) return column_colors_max; return graph->default_column_color; } diff --git a/grep.c b/grep.c index d03d9e24c2..e52654b20b 100644 --- a/grep.c +++ b/grep.c @@ -430,7 +430,7 @@ static int word_char(char ch) static void output_color(struct grep_opt *opt, const void *data, size_t size, const char *color) { - if (opt->color && color && color[0]) { + if (want_color(opt->color) && color && color[0]) { opt->output(opt, color, strlen(color)); opt->output(opt, data, size); opt->output(opt, GIT_COLOR_RESET, strlen(GIT_COLOR_RESET)); diff --git a/log-tree.c b/log-tree.c index 9ba8fb2af3..95d6d4080e 100644 --- a/log-tree.c +++ b/log-tree.c @@ -31,7 +31,7 @@ static char decoration_colors[][COLOR_MAXLEN] = { static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix) { - if (decorate_use_color > 0) + if (want_color(decorate_use_color)) return decoration_colors[ix]; return ""; } diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 4884e1b40c..458233693b 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -167,6 +167,18 @@ test_expect_success TTY 'color when writing to a pager' ' colorful paginated.out ' +test_expect_success TTY 'colors are suppressed by color.pager' ' + rm -f paginated.out && + test_config color.ui auto && + test_config color.pager false && + ( + TERM=vt100 && + export TERM && + test_terminal git log + ) && + ! colorful paginated.out +' + test_expect_success 'color when writing to a file intended for a pager' ' rm -f colorful.log && test_config color.ui auto || diff --git a/wt-status.c b/wt-status.c index da4bce5816..f2016dcdcc 100644 --- a/wt-status.c +++ b/wt-status.c @@ -26,7 +26,9 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = { static const char *color(int slot, struct wt_status *s) { - const char *c = s->use_color > 0 ? s->color_palette[slot] : ""; + const char *c = ""; + if (want_color(s->use_color)) + c = s->color_palette[slot]; if (slot == WT_STATUS_ONBRANCH && color_is_nil(c)) c = s->color_palette[WT_STATUS_HEADER]; return c; -- cgit 1.2.3-korg From 3e1dd17a8958cc5fe47a7ca01c9da8f6fae9cb0b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 17 Aug 2011 22:05:08 -0700 Subject: diff: don't load color config in plumbing The diff config callback is split into two functions: one which loads "ui" config, and one which loads "basic" config. The former chains to the latter, as the diff UI config is a superset of the plumbing config. The color.diff variable is only loaded in the UI config. However, the basic config actually chains to git_color_default_config, which loads color.ui. This doesn't actually cause any bugs, because the plumbing diff code does not actually look at the value of color.ui. However, it is somewhat nonsensical, and it makes it difficult to refactor the color code. It probably came about because there is no git_color_config to load only color config, but rather just git_color_default_config, which loads color config and chains to git_default_config. This patch splits out the color-specific portion of git_color_default_config so that the diff UI config can call it directly. This is perhaps better explained by the chaining of callbacks. Before we had: git_diff_ui_config -> git_diff_basic_config -> git_color_default_config -> git_default_config Now we have: git_diff_ui_config -> git_color_config -> git_diff_basic_config -> git_default_config Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- color.c | 10 +++++++++- color.h | 4 +++- diff.c | 5 ++++- 3 files changed, 16 insertions(+), 3 deletions(-) (limited to 'diff.c') diff --git a/color.c b/color.c index 85864176a6..ec96fe1045 100644 --- a/color.c +++ b/color.c @@ -204,13 +204,21 @@ int want_color(int var) return var > 0; } -int git_color_default_config(const char *var, const char *value, void *cb) +int git_color_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "color.ui")) { git_use_color_default = git_config_colorbool(var, value); return 0; } + 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); } diff --git a/color.h b/color.h index b413e0eace..3e515f2a46 100644 --- a/color.h +++ b/color.h @@ -74,8 +74,10 @@ extern const int column_colors_ansi_max; extern int color_stdout_is_tty; /* - * Use this instead of git_default_config if you need the value of color.ui. + * Use the first one if you need only color config; the second is a convenience + * if you are just going to change to git_default_config, too. */ +int git_color_config(const char *var, const char *value, void *cb); int git_color_default_config(const char *var, const char *value, void *cb); int git_config_colorbool(const char *var, const char *value); diff --git a/diff.c b/diff.c index 0496cdc019..052e42adf0 100644 --- a/diff.c +++ b/diff.c @@ -164,6 +164,9 @@ int git_diff_ui_config(const char *var, const char *value, void *cb) if (!strcmp(var, "diff.ignoresubmodules")) handle_ignore_submodules_arg(&default_diff_options, value); + if (git_color_config(var, value, cb) < 0) + return -1; + return git_diff_basic_config(var, value, cb); } @@ -212,7 +215,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) if (!prefixcmp(var, "submodule.")) return parse_submodule_config_option(var, value); - return git_color_default_config(var, value, cb); + return git_default_config(var, value, cb); } static char *quote_two(const char *one, const char *two) -- cgit 1.2.3-korg