From 9d241b01132c17a44adda2d762b37adf3625bdd7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 16 Sep 2025 16:26:24 -0400 Subject: [PATCH] add-interactive: retain colorbool values longer Most of the diff code stores the decision about whether to show color as a git_colorbool, and evaluates it at point-of-use with want_color(). This timing is important for reasons explained in daa0c3d971 (color: delay auto-color decision until point of use, 2011-08-17). The add-interactive code instead converts immediately to strict boolean values using want_color(), and then evaluates those. This isn't wrong. Even though we pass the bool values to diff_use_color(), which expects a colorbool, the values are compatible. But it is unlike the rest of the color code, and is questionable from a type-system perspective (but C's typing between enums, ints, and bools is weak enough that the compiler does not complain). Let's switch it to the more usual way of calling want_color() at the point of use. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- add-interactive.c | 14 +++++++------- add-interactive.h | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 000315971e..6ffe64c38d 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -20,14 +20,14 @@ #include "prompt.h" #include "tree.h" -static void init_color(struct repository *r, int use_color, +static void init_color(struct repository *r, enum git_colorbool use_color, const char *section_and_slot, char *dst, const char *default_color) { char *key = xstrfmt("color.%s", section_and_slot); const char *value; - if (!use_color) + if (!want_color(use_color)) dst[0] = '\0'; else if (repo_config_get_value(r, key, &value) || color_parse(value, dst)) @@ -36,7 +36,7 @@ static void init_color(struct repository *r, int use_color, free(key); } -static int check_color_config(struct repository *r, const char *var) +static enum git_colorbool check_color_config(struct repository *r, const char *var) { const char *value; enum git_colorbool ret; @@ -55,7 +55,7 @@ static int check_color_config(struct repository *r, const char *var) !repo_config_get_value(r, "color.ui", &value)) ret = git_config_colorbool("color.ui", value); - return want_color(ret); + return ret; } void init_add_i_state(struct add_i_state *s, struct repository *r, @@ -76,7 +76,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r, init_color(r, s->use_color_interactive, "interactive.error", s->error_color, GIT_COLOR_BOLD_RED); strlcpy(s->reset_color_interactive, - s->use_color_interactive ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + want_color(s->use_color_interactive) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); s->use_color_diff = check_color_config(r, "color.diff"); @@ -93,7 +93,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r, init_color(r, s->use_color_diff, "diff.new", s->file_new_color, diff_get_color(s->use_color_diff, DIFF_FILE_NEW)); strlcpy(s->reset_color_diff, - s->use_color_diff ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + want_color(s->use_color_diff) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); FREE_AND_NULL(s->interactive_diff_filter); repo_config_get_string(r, "interactive.difffilter", @@ -1211,7 +1211,7 @@ int run_add_i(struct repository *r, const struct pathspec *ps, * When color was asked for, use the prompt color for * highlighting, otherwise use square brackets. */ - if (s.use_color_interactive) { + if (want_color(s.use_color_interactive)) { data.color = s.prompt_color; data.reset = s.reset_color_interactive; } diff --git a/add-interactive.h b/add-interactive.h index ceadfa6bb6..da49502b76 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -12,8 +12,8 @@ struct add_p_opt { struct add_i_state { struct repository *r; - int use_color_interactive; - int use_color_diff; + enum git_colorbool use_color_interactive; + enum git_colorbool use_color_diff; char header_color[COLOR_MAXLEN]; char help_color[COLOR_MAXLEN]; char prompt_color[COLOR_MAXLEN];