* Re: [PATCH 0/13] unraveling the mysteries of color variables
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
@ 2025-09-16 20:12 ` Jeff King
2025-09-16 20:13 ` [PATCH 01/13] color: use GIT_COLOR_* instead of numeric constants Jeff King
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:12 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
On Tue, Sep 16, 2025 at 04:10:37PM -0400, Jeff King wrote:
> [01/13]: color: use GIT_COLOR_* instead of numeric constants
> [02/13]: color: return enum from git_config_colorbool()
> [03/13]: grep: don't treat grep_opt.color as a strict bool
> [04/13]: diff: simplify color_moved check when flushing
> [05/13]: diff: don't use diff_options.use_color as a strict bool
> [06/13]: diff: pass o->use_color directly to fill_metainfo()
> [07/13]: diff: stop passing ecbdata->use_color as boolean
> [08/13]: pretty: use format_commit_context.auto_color as colorbool
> [09/13]: color: use git_colorbool enum to type to store colorbools
> [10/13]: color: return bool from want_color()
> [11/13]: add-interactive: retain colorbool values longer
> [12/13]: config: store want_color() result in a separate bool
> [13/13]: color: convert git_colorbool into a struct
I forgot to mention that these should be applied on top of
jk/add-i-color. Patch 11 touches the same spot in a way that creates
lots of conflicts (the problems there pre-date that recent topic, but it
was expanded from one color variable to two).
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 01/13] color: use GIT_COLOR_* instead of numeric constants
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
2025-09-16 20:12 ` Jeff King
@ 2025-09-16 20:13 ` Jeff King
2025-09-16 20:14 ` [PATCH 02/13] color: return enum from git_config_colorbool() Jeff King
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:13 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
Long ago Git's decision to show color for a subsytem was stored in a
tri-state variable: it could be true (1), false (0), or unknown (-1).
But since daa0c3d971 (color: delay auto-color decision until point of
use, 2011-08-17) we want to carry around a new state, "auto", which
bases the decision on the tty-ness of stdout (rather than collapsing
that "auto" state to a true/false immediately).
That commit introduced a set of GIT_COLOR_* defines to represent each
state: UNKNOWN, ALWAYS, NEVER, and AUTO. But it only used the AUTO
value, and left alone code using bare 0/1/-1 values. And of course since
then we've grown many new spots that use those bare values.
Let's switch all of these to use the named constants. That should make
the code a bit easier to read, as it is more obvious that we're
representing a color decision.
Signed-off-by: Jeff King <peff@peff.net>
---
add-interactive.c | 9 +++++----
advice.c | 2 +-
builtin/add.c | 2 +-
builtin/am.c | 4 ++--
builtin/branch.c | 2 +-
builtin/clean.c | 2 +-
builtin/commit.c | 2 +-
builtin/config.c | 12 ++++++------
builtin/grep.c | 2 +-
builtin/push.c | 2 +-
builtin/range-diff.c | 3 ++-
builtin/show-branch.c | 2 +-
color.c | 12 ++++++------
diff.c | 6 +++---
grep.h | 2 +-
parse-options-cb.c | 2 +-
pretty.c | 2 +-
ref-filter.h | 2 +-
sideband.c | 4 ++--
transport.c | 2 +-
wt-status.c | 6 +++---
21 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index 4604c69140..34c020673e 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -42,7 +42,7 @@ static int check_color_config(struct repository *r, const char *var)
int ret;
if (repo_config_get_value(r, var, &value))
- ret = -1;
+ ret = GIT_COLOR_UNKNOWN;
else
ret = git_config_colorbool(var, value);
@@ -51,7 +51,8 @@ static int check_color_config(struct repository *r, const char *var)
* the value parsed by git_color_config(), which may not have been
* called by the main command.
*/
- if (ret < 0 && !repo_config_get_value(r, "color.ui", &value))
+ if (ret == GIT_COLOR_UNKNOWN &&
+ !repo_config_get_value(r, "color.ui", &value))
ret = git_config_colorbool("color.ui", value);
return want_color(ret);
@@ -130,8 +131,8 @@ void clear_add_i_state(struct add_i_state *s)
FREE_AND_NULL(s->interactive_diff_filter);
FREE_AND_NULL(s->interactive_diff_algorithm);
memset(s, 0, sizeof(*s));
- s->use_color_interactive = -1;
- s->use_color_diff = -1;
+ s->use_color_interactive = GIT_COLOR_UNKNOWN;
+ s->use_color_diff = GIT_COLOR_UNKNOWN;
}
/*
diff --git a/advice.c b/advice.c
index e5f0ff8449..a00aaad9de 100644
--- a/advice.c
+++ b/advice.c
@@ -7,7 +7,7 @@
#include "help.h"
#include "string-list.h"
-static int advice_use_color = -1;
+static int advice_use_color = GIT_COLOR_UNKNOWN;
static char advice_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_YELLOW, /* HINT */
diff --git a/builtin/add.c b/builtin/add.c
index 740c7c4581..4cd3d183f9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -200,7 +200,7 @@ static int edit_patch(struct repository *repo,
argc = setup_revisions(argc, argv, &rev, NULL);
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
- rev.diffopt.use_color = 0;
+ rev.diffopt.use_color = GIT_COLOR_NEVER;
rev.diffopt.flags.ignore_dirty_submodules = 1;
out = xopen(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
rev.diffopt.file = xfdopen(out, "w");
diff --git a/builtin/am.c b/builtin/am.c
index 6073d64ae9..277c2e7937 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1408,7 +1408,7 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
rev_info.no_commit_id = 1;
rev_info.diffopt.flags.binary = 1;
rev_info.diffopt.flags.full_index = 1;
- rev_info.diffopt.use_color = 0;
+ rev_info.diffopt.use_color = GIT_COLOR_NEVER;
rev_info.diffopt.file = fp;
rev_info.diffopt.close_file = 1;
add_pending_object(&rev_info, &commit->object, "");
@@ -1441,7 +1441,7 @@ static void write_index_patch(const struct am_state *state)
rev_info.disable_stdin = 1;
rev_info.no_commit_id = 1;
rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
- rev_info.diffopt.use_color = 0;
+ rev_info.diffopt.use_color = GIT_COLOR_NEVER;
rev_info.diffopt.file = fp;
rev_info.diffopt.close_file = 1;
add_pending_object(&rev_info, &tree->object, "");
diff --git a/builtin/branch.c b/builtin/branch.c
index fa5ced452e..029223df7b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -46,7 +46,7 @@ static struct object_id head_oid;
static int recurse_submodules = 0;
static int submodule_propagate_branches = 0;
-static int branch_use_color = -1;
+static int branch_use_color = GIT_COLOR_UNKNOWN;
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* PLAIN */
diff --git a/builtin/clean.c b/builtin/clean.c
index 38b67923a6..0ac90a3feb 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -64,7 +64,7 @@ static const char *color_interactive_slots[] = {
[CLEAN_COLOR_RESET] = "reset",
};
-static int clean_use_color = -1;
+static int clean_use_color = GIT_COLOR_UNKNOWN;
static char clean_colors[][COLOR_MAXLEN] = {
[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
diff --git a/builtin/commit.c b/builtin/commit.c
index 8a5dee384d..544603a9c7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1016,7 +1016,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
saved_color_setting = s->use_color;
- s->use_color = 0;
+ s->use_color = GIT_COLOR_NEVER;
committable = run_status(s->fp, index_file, prefix, 1, s);
s->use_color = saved_color_setting;
string_list_clear_func(&s->change, change_data_free);
diff --git a/builtin/config.c b/builtin/config.c
index 59fb113b07..c3da3ae210 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -594,23 +594,23 @@ static int get_colorbool(const struct config_location_options *opts,
{
struct get_colorbool_config_data data = {
.get_colorbool_slot = var,
- .get_colorbool_found = -1,
- .get_diff_color_found = -1,
- .get_color_ui_found = -1,
+ .get_colorbool_found = GIT_COLOR_UNKNOWN,
+ .get_diff_color_found = GIT_COLOR_UNKNOWN,
+ .get_color_ui_found = GIT_COLOR_UNKNOWN,
};
config_with_options(git_get_colorbool_config, &data,
&opts->source, the_repository,
&opts->options);
- if (data.get_colorbool_found < 0) {
+ if (data.get_colorbool_found == GIT_COLOR_UNKNOWN) {
if (!strcmp(data.get_colorbool_slot, "color.diff"))
data.get_colorbool_found = data.get_diff_color_found;
- if (data.get_colorbool_found < 0)
+ if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
data.get_colorbool_found = data.get_color_ui_found;
}
- if (data.get_colorbool_found < 0)
+ if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
/* default value if none found in config */
data.get_colorbool_found = GIT_COLOR_AUTO;
diff --git a/builtin/grep.c b/builtin/grep.c
index 5df6537333..1d97eb2a2a 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1091,7 +1091,7 @@ int cmd_grep(int argc,
if (show_in_pager == default_pager)
show_in_pager = git_pager(the_repository, 1);
if (show_in_pager) {
- opt.color = 0;
+ opt.color = GIT_COLOR_NEVER;
opt.name_only = 1;
opt.null_following_name = 1;
opt.output_priv = &path_list;
diff --git a/builtin/push.c b/builtin/push.c
index d0794b7b30..0962b122c7 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -27,7 +27,7 @@ static const char * const push_usage[] = {
NULL,
};
-static int push_use_color = -1;
+static int push_use_color = GIT_COLOR_UNKNOWN;
static char push_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED, /* ERROR */
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index a563abff5f..0d51ddd623 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -6,6 +6,7 @@
#include "parse-options.h"
#include "range-diff.h"
#include "config.h"
+#include "color.h"
static const char * const builtin_range_diff_usage[] = {
@@ -66,7 +67,7 @@ int cmd_range_diff(int argc,
/* force color when --dual-color was used */
if (!simple_color)
- diffopt.use_color = 1;
+ diffopt.use_color = GIT_COLOR_ALWAYS;
/* If `--diff-merges` was specified, imply `--merges` */
if (diff_merges_arg.nr) {
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 1ab7db9d2c..970e78bc2d 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -29,7 +29,7 @@ static const char*const show_branch_usage[] = {
NULL
};
-static int showbranch_use_color = -1;
+static int showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
diff --git a/color.c b/color.c
index 7df8862c71..22aa453fef 100644
--- a/color.c
+++ b/color.c
@@ -373,19 +373,19 @@ int git_config_colorbool(const char *var, const char *value)
{
if (value) {
if (!strcasecmp(value, "never"))
- return 0;
+ return GIT_COLOR_NEVER;
if (!strcasecmp(value, "always"))
- return 1;
+ return GIT_COLOR_ALWAYS;
if (!strcasecmp(value, "auto"))
return GIT_COLOR_AUTO;
}
if (!var)
- return -1;
+ return GIT_COLOR_UNKNOWN;
/* Missing or explicit false to turn off colorization */
if (!git_config_bool(var, value))
- return 0;
+ return GIT_COLOR_NEVER;
/* any normal truth value defaults to 'auto' */
return GIT_COLOR_AUTO;
@@ -418,15 +418,15 @@ int want_color_fd(int fd, int var)
if (fd < 1 || fd >= ARRAY_SIZE(want_auto))
BUG("file descriptor out of range: %d", fd);
- if (var < 0)
+ if (var == GIT_COLOR_UNKNOWN)
var = git_use_color_default;
if (var == GIT_COLOR_AUTO) {
if (want_auto[fd] < 0)
want_auto[fd] = check_auto_color(fd);
return want_auto[fd];
}
- return var;
+ return var == GIT_COLOR_ALWAYS;
}
int git_color_config(const char *var, const char *value, void *cb UNUSED)
diff --git a/diff.c b/diff.c
index 51603117a9..64a4bd23ea 100644
--- a/diff.c
+++ b/diff.c
@@ -57,7 +57,7 @@ static int diff_detect_rename_default;
static int diff_indent_heuristic = 1;
static int diff_rename_limit_default = 1000;
static int diff_suppress_blank_empty;
-static int diff_use_color_default = -1;
+static int diff_use_color_default = GIT_COLOR_UNKNOWN;
static int diff_color_moved_default;
static int diff_color_moved_ws_default;
static int diff_context_default = 3;
@@ -5278,7 +5278,7 @@ static int diff_opt_color_words(const struct option *opt,
struct diff_options *options = opt->value;
BUG_ON_OPT_NEG(unset);
- options->use_color = 1;
+ options->use_color = GIT_COLOR_ALWAYS;
options->word_diff = DIFF_WORDS_COLOR;
options->word_regex = arg;
return 0;
@@ -5600,7 +5600,7 @@ static int diff_opt_word_diff(const struct option *opt,
if (!strcmp(arg, "plain"))
options->word_diff = DIFF_WORDS_PLAIN;
else if (!strcmp(arg, "color")) {
- options->use_color = 1;
+ options->use_color = GIT_COLOR_ALWAYS;
options->word_diff = DIFF_WORDS_COLOR;
}
else if (!strcmp(arg, "porcelain"))
diff --git a/grep.h b/grep.h
index 926c0875c4..43195baab3 100644
--- a/grep.h
+++ b/grep.h
@@ -198,7 +198,7 @@ struct grep_opt {
[GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
}, \
.only_matching = 0, \
- .color = -1, \
+ .color = GIT_COLOR_UNKNOWN, \
.output = std_output, \
}
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 50c8afe412..e13e0a9e33 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -55,7 +55,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);
- if (value < 0)
+ if (value == GIT_COLOR_UNKNOWN)
return error(_("option `%s' expects \"always\", \"auto\", or \"never\""),
opt->long_name);
*(int *)opt->value = value;
diff --git a/pretty.c b/pretty.c
index cee96b9d94..0521deadc0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1462,7 +1462,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
} else {
int ret = parse_color(sb, placeholder, c);
if (ret)
- c->auto_color = 0;
+ c->auto_color = GIT_COLOR_NEVER;
/*
* Otherwise, we decided to treat %C<unknown>
* as a literal string, and the previous
diff --git a/ref-filter.h b/ref-filter.h
index f22ca94b49..644f5c567c 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -111,7 +111,7 @@ struct ref_format {
.exclude = STRVEC_INIT, \
}
#define REF_FORMAT_INIT { \
- .use_color = -1, \
+ .use_color = GIT_COLOR_UNKNOWN, \
}
/* Macros for checking --merged and --no-merged options */
diff --git a/sideband.c b/sideband.c
index 8f15b98a65..3ac87148b9 100644
--- a/sideband.c
+++ b/sideband.c
@@ -29,14 +29,14 @@ static struct keyword_entry keywords[] = {
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
static int use_sideband_colors(void)
{
- static int use_sideband_colors_cached = -1;
+ static int use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
const char *key = "color.remote";
struct strbuf sb = STRBUF_INIT;
const char *value;
int i;
- if (use_sideband_colors_cached >= 0)
+ if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
return use_sideband_colors_cached;
if (!repo_config_get_string_tmp(the_repository, key, &value))
diff --git a/transport.c b/transport.c
index 6ac8aa402b..ea0be4503c 100644
--- a/transport.c
+++ b/transport.c
@@ -30,7 +30,7 @@
#include "color.h"
#include "bundle-uri.h"
-static int transport_use_color = -1;
+static int transport_use_color = GIT_COLOR_UNKNOWN;
static char transport_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED /* REJECTED */
diff --git a/wt-status.c b/wt-status.c
index 21dabab77d..8ffe6d3988 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -148,7 +148,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
memcpy(s->color_palette, default_wt_status_colors,
sizeof(default_wt_status_colors));
s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
- s->use_color = -1;
+ s->use_color = GIT_COLOR_UNKNOWN;
s->relative_paths = 1;
s->branch = refs_resolve_refdup(get_main_ref_store(the_repository),
"HEAD", 0, NULL, NULL);
@@ -1165,7 +1165,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
* before.
*/
if (s->fp != stdout) {
- rev.diffopt.use_color = 0;
+ rev.diffopt.use_color = GIT_COLOR_NEVER;
wt_status_add_cut_line(s);
}
if (s->verbose > 1 && s->committable) {
@@ -2155,7 +2155,7 @@ static void wt_shortstatus_print(struct wt_status *s)
static void wt_porcelain_print(struct wt_status *s)
{
- s->use_color = 0;
+ s->use_color = GIT_COLOR_NEVER;
s->relative_paths = 0;
s->prefix = NULL;
s->no_gettext = 1;
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 02/13] color: return enum from git_config_colorbool()
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
2025-09-16 20:12 ` Jeff King
2025-09-16 20:13 ` [PATCH 01/13] color: use GIT_COLOR_* instead of numeric constants Jeff King
@ 2025-09-16 20:14 ` Jeff King
2025-09-16 20:16 ` [PATCH 03/13] grep: don't treat grep_opt.color as a strict bool Jeff King
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:14 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
The git_config_colorbool() function returns an integer which is always
one of the GIT_COLOR_* constants UNKNOWN, NEVER, ALWAYS, or AUTO. We
define these constants with macros, but let's switch to using an enum.
Even though the compiler does not strictly enforce enum/int conversions,
this should make the intent clearer to human readers. And as a bonus,
enum names are typically available to debuggers, making it more pleasant
to step through the code there.
This patch updates the return type of git_config_colorbool(), but holds
off on updating all of the callers. There's some trickiness to some of
them, and in the meantime it's perfectly fine to assign an enum into an
int.
Signed-off-by: Jeff King <peff@peff.net>
---
color.c | 2 +-
color.h | 12 +++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/color.c b/color.c
index 22aa453fef..f3adce0141 100644
--- a/color.c
+++ b/color.c
@@ -369,7 +369,7 @@ int color_parse_mem(const char *value, int value_len, char *dst)
#undef OUT
}
-int git_config_colorbool(const char *var, const char *value)
+enum git_colorbool git_config_colorbool(const char *var, const char *value)
{
if (value) {
if (!strcasecmp(value, "never"))
diff --git a/color.h b/color.h
index 7ed259a35b..303e2c9a6d 100644
--- a/color.h
+++ b/color.h
@@ -73,10 +73,12 @@ struct strbuf;
* 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
+enum git_colorbool {
+ GIT_COLOR_UNKNOWN = -1,
+ GIT_COLOR_NEVER = 0,
+ GIT_COLOR_ALWAYS = 1,
+ GIT_COLOR_AUTO = 2,
+};
/* A default list of colors to use for commit graphs and show-branch output */
extern const char *column_colors_ansi[];
@@ -98,7 +100,7 @@ int git_color_config(const char *var, const char *value, void *cb);
* GIT_COLOR_ALWAYS for "always" or a positive boolean,
* and GIT_COLOR_AUTO for "auto".
*/
-int git_config_colorbool(const char *var, const char *value);
+enum git_colorbool git_config_colorbool(const char *var, const char *value);
/*
* Return a boolean whether to use color, where the argument 'var' is
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 03/13] grep: don't treat grep_opt.color as a strict bool
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (2 preceding siblings ...)
2025-09-16 20:14 ` [PATCH 02/13] color: return enum from git_config_colorbool() Jeff King
@ 2025-09-16 20:16 ` Jeff King
2025-09-16 20:17 ` [PATCH 04/13] diff: simplify color_moved check when flushing Jeff King
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:16 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In show_line(), we check to see if colors are desired with just:
if (opt->color)
...we want colors...
But this is incorrect. The color field here is really a git_colorbool,
so it may be "true" for GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO. Either of
those _might_ end up true eventually (once we apply default fallbacks
and check stdout's tty), but they may not. E.g.:
git grep foo | cat
will enter the conditional even though we're not going to show colors.
We should collapse it into a true boolean by calling want_color().
It turns out that this does not produce a user-visible bug. We do some
extra processing to isolate the matched portion of the line in order to
colorize it, but ultimately we pass it to our output_color() helper,
which does correctly check want_color(). So we end up with no colors.
But dropping the extra processing saves a measurable amount of time. For
example, running under hyperfine (which redirects to /dev/null, and thus
does not colorize):
Benchmark 1: ./git.old grep a
Time (mean ± σ): 58.7 ms ± 3.5 ms [User: 580.6 ms, System: 74.3 ms]
Range (min … max): 53.5 ms … 67.1 ms 48 runs
Benchmark 2: ./git.new grep a
Time (mean ± σ): 35.5 ms ± 0.9 ms [User: 276.8 ms, System: 73.8 ms]
Range (min … max): 34.3 ms … 39.3 ms 79 runs
Summary
./git.new grep a ran
1.65 ± 0.11 times faster than ./git.old grep a
That's a fairly extreme benchmark, just because it will come up with a
ton of small matches, but it shows that this really does matter.
Signed-off-by: Jeff King <peff@peff.net>
---
If you really want to cheat on the benchmark, grep for a single space,
which is even more common.
grep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index 932647e4a6..c7e1dc1e0e 100644
--- a/grep.c
+++ b/grep.c
@@ -1263,12 +1263,12 @@ static void show_line(struct grep_opt *opt,
*/
show_line_header(opt, name, lno, cno, sign);
}
- if (opt->color || opt->only_matching) {
+ if (want_color(opt->color) || opt->only_matching) {
regmatch_t match;
enum grep_context ctx = GREP_CONTEXT_BODY;
int eflags = 0;
- if (opt->color) {
+ if (want_color(opt->color)) {
if (sign == ':')
match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
else
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 04/13] diff: simplify color_moved check when flushing
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (3 preceding siblings ...)
2025-09-16 20:16 ` [PATCH 03/13] grep: don't treat grep_opt.color as a strict bool Jeff King
@ 2025-09-16 20:17 ` Jeff King
2025-09-16 20:19 ` [PATCH 05/13] diff: don't use diff_options.use_color as a strict bool Jeff King
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:17 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In diff_flush_patch_all_file_pairs(), we set o->emitted_symbols if and
only if o->color_moved is true. That causes the lower-level routines to
fill up o->emitted_symbols, which we then analyze in order to do the
actual colorizing.
But in that final step, we do:
if (o->emitted_symbols) {
if (o->color_moved) {
...actual coloring...
}
...clean up of emitted_symbols...
}
The inner "if" will always trigger, since we set emitted_symbols only
when doing color_moved (it is a little confusing that it is set inside
the diff_options struct, but that is for convenience of passing it to
the lower-level routines; we always clear it at the end of flushing,
since 48edf3a02a (diff: clear emitted_symbols flag after use,
2019-01-24)).
Let's simplify the code a bit by just dropping the inner "if" and
running its block unconditionally.
In theory the current code might be useful if another feature besides
color_moved setup and used emitted_symbols, but it would be easy to
refactor later to handle that. And in the meantime, this makes further
work in this area easier.
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index 64a4bd23ea..e6c85c8491 100644
--- a/diff.c
+++ b/diff.c
@@ -6746,20 +6746,17 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
}
if (o->emitted_symbols) {
- if (o->color_moved) {
- struct mem_pool entry_pool;
- struct moved_entry_list *entry_list;
-
- mem_pool_init(&entry_pool, 1024 * 1024);
- entry_list = add_lines_to_move_detection(o,
- &entry_pool);
- mark_color_as_moved(o, entry_list);
- if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
- dim_moved_lines(o);
-
- mem_pool_discard(&entry_pool, 0);
- free(entry_list);
- }
+ struct mem_pool entry_pool;
+ struct moved_entry_list *entry_list;
+
+ mem_pool_init(&entry_pool, 1024 * 1024);
+ entry_list = add_lines_to_move_detection(o, &entry_pool);
+ mark_color_as_moved(o, entry_list);
+ if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
+ dim_moved_lines(o);
+
+ mem_pool_discard(&entry_pool, 0);
+ free(entry_list);
for (i = 0; i < esm.nr; i++)
emit_diff_symbol_from_struct(o, &esm.buf[i]);
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 05/13] diff: don't use diff_options.use_color as a strict bool
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (4 preceding siblings ...)
2025-09-16 20:17 ` [PATCH 04/13] diff: simplify color_moved check when flushing Jeff King
@ 2025-09-16 20:19 ` Jeff King
2025-09-16 20:20 ` [PATCH 06/13] diff: pass o->use_color directly to fill_metainfo() Jeff King
` (8 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:19 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
We disable --color-moved if color is not in use at all. This happens in
diff_setup_done(), where we set options->color_moved to 0 if
options->use_color is not true. But a strict boolean check here is not
correct; use_color could be GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO, both of
which evaluate to true, even though we may later decide not to show
colors.
We should be using want_color() to convert that git_colorbool into a
true boolean. As it turns out, this does not produce wrong output. Even
though we go to the trouble to detect the moved lines, ultimately we get
the color values from diff_get_color(), which does check want_color().
And so it returns the empty string for each color, and we "color" the
result with nothing.
So the output is correct, but there is a small but measurable
performance cost to doing the line detection. E.g., in git.git before
and after this patch (there are no colors shown because hyperfine
redirects output to /dev/null):
Benchmark 1: ./git.old log --no-merges -p --color-moved -1000
Time (mean ± σ): 1.019 s ± 0.013 s [User: 0.955 s, System: 0.064 s]
Range (min … max): 1.005 s … 1.045 s 10 runs
Benchmark 2: ./git.new log --no-merges -p --color-moved -1000
Time (mean ± σ): 982.9 ms ± 14.5 ms [User: 925.8 ms, System: 57.1 ms]
Range (min … max): 965.1 ms … 1003.2 ms 10 runs
Summary
./git.new log --no-merges -p --color-moved -1000 ran
1.04 ± 0.02 times faster than ./git.old log --no-merges -p --color-moved -1000
Note that the fix is not quite as simple as just calling want_color()
from diff_setup_done(). There's a subtle timing issue that goes back to
daa0c3d971 (color: delay auto-color decision until point of use,
2011-08-17), the commit that adds want_color() in the first place. As
discussed there, we must delay evaluating the colorbool value until all
pager setup is complete.
So instead, we'll leave the "color_moved" field intact in diff_setup_done(),
and modify the point where it is evaluated. Fortunately there is only
one such spot that controls whether we run any of the color-moved code
at all.
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index e6c85c8491..cc3542e7f4 100644
--- a/diff.c
+++ b/diff.c
@@ -4995,8 +4995,7 @@ void diff_setup_done(struct diff_options *options)
if (options->flags.follow_renames)
diff_check_follow_pathspec(&options->pathspec, 1);
- if (!options->use_color ||
- (options->flags.allow_external && external_diff()))
+ if (options->flags.allow_external && external_diff())
options->color_moved = 0;
if (options->filter_not) {
@@ -6733,7 +6732,7 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
if (WSEH_NEW & WS_RULE_MASK)
BUG("WS rules bit mask overlaps with diff symbol flags");
- if (o->color_moved)
+ if (o->color_moved && want_color(o->use_color))
o->emitted_symbols = &esm;
if (o->additional_path_headers)
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 06/13] diff: pass o->use_color directly to fill_metainfo()
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (5 preceding siblings ...)
2025-09-16 20:19 ` [PATCH 05/13] diff: don't use diff_options.use_color as a strict bool Jeff King
@ 2025-09-16 20:20 ` Jeff King
2025-09-16 20:21 ` [PATCH 07/13] diff: stop passing ecbdata->use_color as boolean Jeff King
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:20 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
We pass the use_color parameter of fill_metainfo() as a strict boolean,
using:
want_color(o->use_color) && !pgm
to derive its value. But then inside the function, we pass it to
diff_get_color(), which expects one of the git_colorbool enum values,
and so feeds it to want_color() again.
Even though want_color() produces a strict 0/1 boolean, this doesn't
produce wrong results because want_color() is idempotent. Since
GIT_COLOR_ALWAYS and NEVER are defined as 1 and 0, and because
want_color() passes through those values, evaluating "want_color(foo)"
and "want_color(want_color(foo))" will return the same result.
But as part of a longer strategy to align the types we use for storing
these values, let's pass through the colorbool directly. To handle the
"&&" case here, we'll convert the presence of "pgm" into "NEVER", which
arguably makes the intent of the code more clear anyway.
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index cc3542e7f4..6b12596642 100644
--- a/diff.c
+++ b/diff.c
@@ -4596,7 +4596,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
*/
fill_metainfo(msg, name, other, one, two, o, p,
&must_show_header,
- want_color(o->use_color) && !pgm);
+ pgm ? GIT_COLOR_NEVER : o->use_color);
xfrm_msg = msg->len ? msg->buf : NULL;
}
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 07/13] diff: stop passing ecbdata->use_color as boolean
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (6 preceding siblings ...)
2025-09-16 20:20 ` [PATCH 06/13] diff: pass o->use_color directly to fill_metainfo() Jeff King
@ 2025-09-16 20:21 ` Jeff King
2025-09-16 20:22 ` [PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool Jeff King
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:21 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In emit_hunk_header(), we evaluate ecbdata->color_diff both as a
git_colorbool, passing it to diff_get_color():
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
and as a strict boolean:
const char *reverse = ecbdata->color_diff ? GIT_COLOR_REVERSE : "";
At first glance this seems wrong. Usually we store the color decision as
a git_colorbool, so the second line would get confused by GIT_COLOR_AUTO
(which is boolean true, but may still mean we do not produce color).
However, the second line is correct because our caller sets color_diff
using want_color(), which collapses the colorbool to a strict true/false
boolean. The first line is _also_ correct because of the idempotence of
want_color(). Even though diff_get_color() will pass our true/false
value through want_color() again, the result will be left untouched.
But let's pass through the colorbool itself, which makes it more
consistent with the rest of the diff code. We'll need to then call
want_color() whenever we treat it as a boolean, but there is only such
spot (the one quoted above).
Signed-off-by: Jeff King <peff@peff.net>
---
diff.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index 6b12596642..926429d55b 100644
--- a/diff.c
+++ b/diff.c
@@ -1672,7 +1672,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
- const char *reverse = ecbdata->color_diff ? GIT_COLOR_REVERSE : "";
+ const char *reverse = want_color(ecbdata->color_diff) ? GIT_COLOR_REVERSE : "";
static const char atat[2] = { '@', '@' };
const char *cp, *ep;
struct strbuf msgbuf = STRBUF_INIT;
@@ -1826,7 +1826,7 @@ static void emit_rewrite_diff(const char *name_a,
size_two = fill_textconv(o->repo, textconv_two, two, &data_two);
memset(&ecbdata, 0, sizeof(ecbdata));
- ecbdata.color_diff = want_color(o->use_color);
+ ecbdata.color_diff = o->use_color;
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
ecbdata.opt = o;
if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
@@ -3732,7 +3732,7 @@ static void builtin_diff(const char *name_a,
if (o->flags.suppress_diff_headers)
lbl[0] = NULL;
ecbdata.label_path = lbl;
- ecbdata.color_diff = want_color(o->use_color);
+ ecbdata.color_diff = o->use_color;
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
check_blank_at_eof(&mf1, &mf2, &ecbdata);
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (7 preceding siblings ...)
2025-09-16 20:21 ` [PATCH 07/13] diff: stop passing ecbdata->use_color as boolean Jeff King
@ 2025-09-16 20:22 ` Jeff King
2025-09-16 20:24 ` [PATCH 09/13] color: use git_colorbool enum to type to store colorbools Jeff King
` (5 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:22 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
When we see "%C(auto)" as a format placeholder, we evaluate the "color"
field of our pretty_print_context to decide whether we want color. The
auto_color field of format_commit_context then stores the boolean result
of want_color(), telling us the yes/no of whether we want color.
But the resulting field is passed to various functions which expect a
git_colorbool, like diff_get_color(), that will then pass it to
want_color() again. It's not wrong to do so, since want_color() is
idempotent. But it makes it harder to reason about the types, since we
sometimes confuse colorbools and strict booleans.
Let's instead store auto_color as the original colorbool itself. We'll
have to make sure it is passed through want_color() when it is
evaluated, but there is only one such spot (right next to where we
assign it!). Every other caller just ends up passing it to get
diff_get_color() either directly or through another helper.
Signed-off-by: Jeff King <peff@peff.net>
---
pretty.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pretty.c b/pretty.c
index 0521deadc0..86d69bf877 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1455,8 +1455,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
- c->auto_color = want_color(c->pretty_ctx->color);
- if (c->auto_color && sb->len)
+ c->auto_color = c->pretty_ctx->color;
+ if (want_color(c->auto_color) && sb->len)
strbuf_addstr(sb, GIT_COLOR_RESET);
return 7; /* consumed 7 bytes, "C(auto)" */
} else {
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 09/13] color: use git_colorbool enum to type to store colorbools
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (8 preceding siblings ...)
2025-09-16 20:22 ` [PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool Jeff King
@ 2025-09-16 20:24 ` Jeff King
2025-09-16 23:13 ` [PATCH v1.5 9/13] color: use git_colorbool enum " Jeff King
2025-09-16 20:25 ` [PATCH 10/13] color: return bool from want_color() Jeff King
` (4 subsequent siblings)
14 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:24 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
We traditionally used "int" to store and pass around the values defined
by "enum git_colorbool" (which were originally just #define macros).
Using an int doesn't produce incorrect results, but using the actual
enum makes the intent of the code more clear.
It would be nice if the compiler could catch cases where we used the
enum and an int interchangeably, since it's very easy to accidentally
check the boolean true/false of a colorbool like:
if (branch_use_color)
This is wrong because GIT_COLOR_UNKNOWN and GIT_COLOR_AUTO evaluate to
true in C, even though we may ultimately decide not to use color. But C
is pretty happy to convert between ints and enums (even with various
-Wenum-* warnings). So this sadly doesn't protect us from such mistakes,
but it hopefully does make the code easier to read.
Signed-off-by: Jeff King <peff@peff.net>
---
This actually could come sooner in the series, since the compiler will
not complain about conversions either way. But I thought it made sense to
clean up the weird spots first.
add-interactive.c | 2 +-
advice.c | 2 +-
builtin/branch.c | 2 +-
builtin/clean.c | 2 +-
builtin/commit.c | 2 +-
builtin/config.c | 6 +++---
builtin/push.c | 2 +-
builtin/show-branch.c | 2 +-
color.c | 4 ++--
color.h | 2 +-
combine-diff.c | 2 +-
diff.c | 6 +++---
diff.h | 5 +++--
grep.h | 2 +-
log-tree.c | 4 ++--
log-tree.h | 2 +-
parse-options-cb.c | 2 +-
pretty.c | 6 +++---
pretty.h | 3 ++-
ref-filter.h | 2 +-
sideband.c | 4 ++--
transport.c | 2 +-
wt-status.h | 2 +-
23 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index 34c020673e..000315971e 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -39,7 +39,7 @@ static void init_color(struct repository *r, int use_color,
static int check_color_config(struct repository *r, const char *var)
{
const char *value;
- int ret;
+ enum git_colorbool ret;
if (repo_config_get_value(r, var, &value))
ret = GIT_COLOR_UNKNOWN;
diff --git a/advice.c b/advice.c
index a00aaad9de..0018501b7b 100644
--- a/advice.c
+++ b/advice.c
@@ -7,7 +7,7 @@
#include "help.h"
#include "string-list.h"
-static int advice_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool advice_use_color = GIT_COLOR_UNKNOWN;
static char advice_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_YELLOW, /* HINT */
diff --git a/builtin/branch.c b/builtin/branch.c
index 029223df7b..9fcf04bebb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -46,7 +46,7 @@ static struct object_id head_oid;
static int recurse_submodules = 0;
static int submodule_propagate_branches = 0;
-static int branch_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool branch_use_color = GIT_COLOR_UNKNOWN;
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* PLAIN */
diff --git a/builtin/clean.c b/builtin/clean.c
index 0ac90a3feb..1d5e7e5366 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -64,7 +64,7 @@ static const char *color_interactive_slots[] = {
[CLEAN_COLOR_RESET] = "reset",
};
-static int clean_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool clean_use_color = GIT_COLOR_UNKNOWN;
static char clean_colors[][COLOR_MAXLEN] = {
[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
diff --git a/builtin/commit.c b/builtin/commit.c
index 544603a9c7..d8f21a4c62 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -936,7 +936,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor && include_status) {
int ident_shown = 0;
- int saved_color_setting;
+ enum git_colorbool saved_color_setting;
struct ident_split ci, ai;
const char *hint_cleanup_all = allow_empty_message ?
_("Please enter the commit message for your changes."
diff --git a/builtin/config.c b/builtin/config.c
index c3da3ae210..9e4e4eb2f1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -568,9 +568,9 @@ static void get_color(const struct config_location_options *opts,
}
struct get_colorbool_config_data {
- int get_colorbool_found;
- int get_diff_color_found;
- int get_color_ui_found;
+ enum git_colorbool get_colorbool_found;
+ enum git_colorbool get_diff_color_found;
+ enum git_colorbool get_color_ui_found;
const char *get_colorbool_slot;
};
diff --git a/builtin/push.c b/builtin/push.c
index 0962b122c7..5b6cebbb85 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -27,7 +27,7 @@ static const char * const push_usage[] = {
NULL,
};
-static int push_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool push_use_color = GIT_COLOR_UNKNOWN;
static char push_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED, /* ERROR */
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 970e78bc2d..441babf2e3 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -29,7 +29,7 @@ static const char*const show_branch_usage[] = {
NULL
};
-static int showbranch_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
diff --git a/color.c b/color.c
index f3adce0141..3348ead534 100644
--- a/color.c
+++ b/color.c
@@ -9,7 +9,7 @@
#include "pager.h"
#include "strbuf.h"
-static int git_use_color_default = GIT_COLOR_AUTO;
+static enum git_colorbool git_use_color_default = GIT_COLOR_AUTO;
int color_stdout_is_tty = -1;
/*
@@ -404,7 +404,7 @@ static int check_auto_color(int fd)
return 0;
}
-int want_color_fd(int fd, int var)
+int want_color_fd(int fd, enum git_colorbool var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
diff --git a/color.h b/color.h
index 303e2c9a6d..fcb38c5562 100644
--- a/color.h
+++ b/color.h
@@ -106,7 +106,7 @@ enum git_colorbool git_config_colorbool(const char *var, const char *value);
* Return a boolean whether to use color, where the argument 'var' is
* one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
*/
-int want_color_fd(int fd, int var);
+int want_color_fd(int fd, enum git_colorbool var);
#define want_color(colorbool) want_color_fd(1, (colorbool))
#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
diff --git a/combine-diff.c b/combine-diff.c
index 3878faabe7..21b7fdfff4 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -749,7 +749,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
static void dump_sline(struct sline *sline, const char *line_prefix,
unsigned long cnt, int num_parent,
- int use_color, int result_deleted)
+ enum git_colorbool use_color, int result_deleted)
{
unsigned long mark = (1UL<<num_parent);
unsigned long no_pre_delete = (2UL<<num_parent);
diff --git a/diff.c b/diff.c
index 926429d55b..87fa16b730 100644
--- a/diff.c
+++ b/diff.c
@@ -57,7 +57,7 @@ static int diff_detect_rename_default;
static int diff_indent_heuristic = 1;
static int diff_rename_limit_default = 1000;
static int diff_suppress_blank_empty;
-static int diff_use_color_default = GIT_COLOR_UNKNOWN;
+static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
static int diff_color_moved_default;
static int diff_color_moved_ws_default;
static int diff_context_default = 3;
@@ -2303,7 +2303,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
}
}
-const char *diff_get_color(int diff_use_color, enum color_diff ix)
+const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix)
{
if (want_color(diff_use_color))
return diff_colors[ix];
@@ -4497,7 +4497,7 @@ static void fill_metainfo(struct strbuf *msg,
struct diff_options *o,
struct diff_filepair *p,
int *must_show_header,
- int use_color)
+ enum git_colorbool use_color)
{
const char *set = diff_get_color(use_color, DIFF_METAINFO);
const char *reset = diff_get_color(use_color, DIFF_RESET);
diff --git a/diff.h b/diff.h
index 9bb939a4f1..bccd86a748 100644
--- a/diff.h
+++ b/diff.h
@@ -7,6 +7,7 @@
#include "hash.h"
#include "pathspec.h"
#include "strbuf.h"
+#include "color.h"
struct oidset;
@@ -283,7 +284,7 @@ struct diff_options {
/* diff-filter bits */
unsigned int filter, filter_not;
- int use_color;
+ enum git_colorbool use_color;
/* Number of context lines to generate in patch output. */
int context;
@@ -469,7 +470,7 @@ enum color_diff {
DIFF_FILE_NEW_BOLD = 22,
};
-const char *diff_get_color(int diff_use_color, enum color_diff ix);
+const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff_get_color((o)->use_color, ix)
diff --git a/grep.h b/grep.h
index 43195baab3..13e26a9318 100644
--- a/grep.h
+++ b/grep.h
@@ -159,7 +159,7 @@ struct grep_opt {
int pathname;
int null_following_name;
int only_matching;
- int color;
+ enum git_colorbool color;
int max_depth;
int funcname;
int funcbody;
diff --git a/log-tree.c b/log-tree.c
index 233bf9f227..a2cd5c587b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -57,7 +57,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_GRAFTED] = "grafted",
};
-static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
+static const char *decorate_get_color(enum git_colorbool decorate_use_color, enum decoration_type ix)
{
if (want_color(decorate_use_color))
return decoration_colors[ix];
@@ -341,7 +341,7 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio
*/
void format_decorations(struct strbuf *sb,
const struct commit *commit,
- int use_color,
+ enum git_colorbool use_color,
const struct decoration_options *opts)
{
const struct name_decoration *decoration;
diff --git a/log-tree.h b/log-tree.h
index ebe491c543..1c82380d95 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -26,7 +26,7 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
void show_log(struct rev_info *opt);
void format_decorations(struct strbuf *sb, const struct commit *commit,
- int use_color, const struct decoration_options *opts);
+ enum git_colorbool use_color, const struct decoration_options *opts);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
char **extra_headers_p,
diff --git a/parse-options-cb.c b/parse-options-cb.c
index e13e0a9e33..976cc86385 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -50,7 +50,7 @@ int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
int unset)
{
- int value;
+ enum git_colorbool value;
if (!arg)
arg = unset ? "never" : (const char *)opt->defval;
diff --git a/pretty.c b/pretty.c
index 86d69bf877..e0646bbc5d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -470,7 +470,7 @@ static inline void strbuf_add_with_color(struct strbuf *sb, const char *color,
static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
const char *line, size_t linelen,
- int color, enum grep_context ctx,
+ enum git_colorbool color, enum grep_context ctx,
enum grep_header_field field)
{
const char *buf, *eol, *line_color, *match_color;
@@ -899,7 +899,7 @@ struct format_commit_context {
const char *message;
char *commit_encoding;
size_t width, indent1, indent2;
- int auto_color;
+ enum git_colorbool auto_color;
int padding;
/* These offsets are relative to the start of the commit message. */
@@ -2167,7 +2167,7 @@ static int pp_utf8_width(const char *start, const char *end)
}
static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,
- int color, int tabwidth, const char *line,
+ enum git_colorbool color, int tabwidth, const char *line,
int linelen)
{
const char *tab;
diff --git a/pretty.h b/pretty.h
index df267afe4a..fac699033e 100644
--- a/pretty.h
+++ b/pretty.h
@@ -3,6 +3,7 @@
#include "date.h"
#include "string-list.h"
+#include "color.h"
struct commit;
struct repository;
@@ -46,7 +47,7 @@ struct pretty_print_context {
struct rev_info *rev;
const char *output_encoding;
struct string_list *mailmap;
- int color;
+ enum git_colorbool color;
struct ident_split *from_ident;
unsigned encode_email_headers:1;
struct pretty_print_describe_status *describe_status;
diff --git a/ref-filter.h b/ref-filter.h
index 644f5c567c..81f2c229a9 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -95,7 +95,7 @@ struct ref_format {
const char *format;
const char *rest;
int quote_style;
- int use_color;
+ enum git_colorbool use_color;
/* Internal state to ref-filter */
int need_color_reset_at_eol;
diff --git a/sideband.c b/sideband.c
index 3ac87148b9..ea7c25211e 100644
--- a/sideband.c
+++ b/sideband.c
@@ -27,9 +27,9 @@ static struct keyword_entry keywords[] = {
};
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
-static int use_sideband_colors(void)
+static enum git_colorbool use_sideband_colors(void)
{
- static int use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
+ static enum git_colorbool use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
const char *key = "color.remote";
struct strbuf sb = STRBUF_INIT;
diff --git a/transport.c b/transport.c
index ea0be4503c..c7f06a7382 100644
--- a/transport.c
+++ b/transport.c
@@ -30,7 +30,7 @@
#include "color.h"
#include "bundle-uri.h"
-static int transport_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
static char transport_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED /* REJECTED */
diff --git a/wt-status.h b/wt-status.h
index 4e377ce62b..e40a27214a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -111,7 +111,7 @@ struct wt_status {
int amend;
enum commit_whence whence;
int nowarn;
- int use_color;
+ enum git_colorbool use_color;
int no_gettext;
int display_comment_prefix;
int relative_paths;
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v1.5 9/13] color: use git_colorbool enum type to store colorbools
2025-09-16 20:24 ` [PATCH 09/13] color: use git_colorbool enum to type to store colorbools Jeff King
@ 2025-09-16 23:13 ` Jeff King
0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 23:13 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
On Tue, Sep 16, 2025 at 04:24:09PM -0400, Jeff King wrote:
> Subject: Re: [PATCH 09/13] color: use git_colorbool enum to type to store colorbools
I missed a typo in the subject (I do my final proofread in my MUA as I send them
out, so it's easy to glance past the subject).
Also, I belatedly noticed a CI failure here: "make hdr-check" complains
and we need to squash this in to get the enum definition (I don't think
we can forward declare it because the compiler needs to know what values
it takes to compute the size):
diff --git a/log-tree.h b/log-tree.h
index 1c82380d95..07924be8bc 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -1,6 +1,8 @@
#ifndef LOG_TREE_H
#define LOG_TREE_H
+#include "color.h"
+
struct rev_info;
struct log_info {
So here's a quick re-roll of just patch 9 here. Fingers crossed that I
won't need to re-send the whole thing as a v2 as soon as I get some
review. ;)
-- >8 --
Subject: color: use git_colorbool enum type to store colorbools
We traditionally used "int" to store and pass around the values defined
by "enum git_colorbool" (which were originally just #define macros).
Using an int doesn't produce incorrect results, but using the actual
enum makes the intent of the code more clear.
It would be nice if the compiler could catch cases where we used the
enum and an int interchangeably, since it's very easy to accidentally
check the boolean true/false of a colorbool like:
if (branch_use_color)
This is wrong because GIT_COLOR_UNKNOWN and GIT_COLOR_AUTO evaluate to
true in C, even though we may ultimately decide not to use color. But C
is pretty happy to convert between ints and enums (even with various
-Wenum-* warnings). So this sadly doesn't protect us from such mistakes,
but it hopefully does make the code easier to read.
Signed-off-by: Jeff King <peff@peff.net>
---
add-interactive.c | 2 +-
advice.c | 2 +-
builtin/branch.c | 2 +-
builtin/clean.c | 2 +-
builtin/commit.c | 2 +-
builtin/config.c | 6 +++---
builtin/push.c | 2 +-
builtin/show-branch.c | 2 +-
color.c | 4 ++--
color.h | 2 +-
combine-diff.c | 2 +-
diff.c | 6 +++---
diff.h | 5 +++--
grep.h | 2 +-
log-tree.c | 4 ++--
log-tree.h | 4 +++-
parse-options-cb.c | 2 +-
pretty.c | 6 +++---
pretty.h | 3 ++-
ref-filter.h | 2 +-
sideband.c | 4 ++--
transport.c | 2 +-
wt-status.h | 2 +-
23 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index 34c020673e..000315971e 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -39,7 +39,7 @@ static void init_color(struct repository *r, int use_color,
static int check_color_config(struct repository *r, const char *var)
{
const char *value;
- int ret;
+ enum git_colorbool ret;
if (repo_config_get_value(r, var, &value))
ret = GIT_COLOR_UNKNOWN;
diff --git a/advice.c b/advice.c
index a00aaad9de..0018501b7b 100644
--- a/advice.c
+++ b/advice.c
@@ -7,7 +7,7 @@
#include "help.h"
#include "string-list.h"
-static int advice_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool advice_use_color = GIT_COLOR_UNKNOWN;
static char advice_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_YELLOW, /* HINT */
diff --git a/builtin/branch.c b/builtin/branch.c
index 029223df7b..9fcf04bebb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -46,7 +46,7 @@ static struct object_id head_oid;
static int recurse_submodules = 0;
static int submodule_propagate_branches = 0;
-static int branch_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool branch_use_color = GIT_COLOR_UNKNOWN;
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* PLAIN */
diff --git a/builtin/clean.c b/builtin/clean.c
index 0ac90a3feb..1d5e7e5366 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -64,7 +64,7 @@ static const char *color_interactive_slots[] = {
[CLEAN_COLOR_RESET] = "reset",
};
-static int clean_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool clean_use_color = GIT_COLOR_UNKNOWN;
static char clean_colors[][COLOR_MAXLEN] = {
[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
diff --git a/builtin/commit.c b/builtin/commit.c
index 544603a9c7..d8f21a4c62 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -936,7 +936,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor && include_status) {
int ident_shown = 0;
- int saved_color_setting;
+ enum git_colorbool saved_color_setting;
struct ident_split ci, ai;
const char *hint_cleanup_all = allow_empty_message ?
_("Please enter the commit message for your changes."
diff --git a/builtin/config.c b/builtin/config.c
index c3da3ae210..9e4e4eb2f1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -568,9 +568,9 @@ static void get_color(const struct config_location_options *opts,
}
struct get_colorbool_config_data {
- int get_colorbool_found;
- int get_diff_color_found;
- int get_color_ui_found;
+ enum git_colorbool get_colorbool_found;
+ enum git_colorbool get_diff_color_found;
+ enum git_colorbool get_color_ui_found;
const char *get_colorbool_slot;
};
diff --git a/builtin/push.c b/builtin/push.c
index 0962b122c7..5b6cebbb85 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -27,7 +27,7 @@ static const char * const push_usage[] = {
NULL,
};
-static int push_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool push_use_color = GIT_COLOR_UNKNOWN;
static char push_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED, /* ERROR */
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 970e78bc2d..441babf2e3 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -29,7 +29,7 @@ static const char*const show_branch_usage[] = {
NULL
};
-static int showbranch_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
diff --git a/color.c b/color.c
index f3adce0141..3348ead534 100644
--- a/color.c
+++ b/color.c
@@ -9,7 +9,7 @@
#include "pager.h"
#include "strbuf.h"
-static int git_use_color_default = GIT_COLOR_AUTO;
+static enum git_colorbool git_use_color_default = GIT_COLOR_AUTO;
int color_stdout_is_tty = -1;
/*
@@ -404,7 +404,7 @@ static int check_auto_color(int fd)
return 0;
}
-int want_color_fd(int fd, int var)
+int want_color_fd(int fd, enum git_colorbool var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
diff --git a/color.h b/color.h
index 303e2c9a6d..fcb38c5562 100644
--- a/color.h
+++ b/color.h
@@ -106,7 +106,7 @@ enum git_colorbool git_config_colorbool(const char *var, const char *value);
* Return a boolean whether to use color, where the argument 'var' is
* one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
*/
-int want_color_fd(int fd, int var);
+int want_color_fd(int fd, enum git_colorbool var);
#define want_color(colorbool) want_color_fd(1, (colorbool))
#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
diff --git a/combine-diff.c b/combine-diff.c
index 3878faabe7..21b7fdfff4 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -749,7 +749,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
static void dump_sline(struct sline *sline, const char *line_prefix,
unsigned long cnt, int num_parent,
- int use_color, int result_deleted)
+ enum git_colorbool use_color, int result_deleted)
{
unsigned long mark = (1UL<<num_parent);
unsigned long no_pre_delete = (2UL<<num_parent);
diff --git a/diff.c b/diff.c
index 926429d55b..87fa16b730 100644
--- a/diff.c
+++ b/diff.c
@@ -57,7 +57,7 @@ static int diff_detect_rename_default;
static int diff_indent_heuristic = 1;
static int diff_rename_limit_default = 1000;
static int diff_suppress_blank_empty;
-static int diff_use_color_default = GIT_COLOR_UNKNOWN;
+static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
static int diff_color_moved_default;
static int diff_color_moved_ws_default;
static int diff_context_default = 3;
@@ -2303,7 +2303,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
}
}
-const char *diff_get_color(int diff_use_color, enum color_diff ix)
+const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix)
{
if (want_color(diff_use_color))
return diff_colors[ix];
@@ -4497,7 +4497,7 @@ static void fill_metainfo(struct strbuf *msg,
struct diff_options *o,
struct diff_filepair *p,
int *must_show_header,
- int use_color)
+ enum git_colorbool use_color)
{
const char *set = diff_get_color(use_color, DIFF_METAINFO);
const char *reset = diff_get_color(use_color, DIFF_RESET);
diff --git a/diff.h b/diff.h
index 9bb939a4f1..bccd86a748 100644
--- a/diff.h
+++ b/diff.h
@@ -7,6 +7,7 @@
#include "hash.h"
#include "pathspec.h"
#include "strbuf.h"
+#include "color.h"
struct oidset;
@@ -283,7 +284,7 @@ struct diff_options {
/* diff-filter bits */
unsigned int filter, filter_not;
- int use_color;
+ enum git_colorbool use_color;
/* Number of context lines to generate in patch output. */
int context;
@@ -469,7 +470,7 @@ enum color_diff {
DIFF_FILE_NEW_BOLD = 22,
};
-const char *diff_get_color(int diff_use_color, enum color_diff ix);
+const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff_get_color((o)->use_color, ix)
diff --git a/grep.h b/grep.h
index 43195baab3..13e26a9318 100644
--- a/grep.h
+++ b/grep.h
@@ -159,7 +159,7 @@ struct grep_opt {
int pathname;
int null_following_name;
int only_matching;
- int color;
+ enum git_colorbool color;
int max_depth;
int funcname;
int funcbody;
diff --git a/log-tree.c b/log-tree.c
index 233bf9f227..a2cd5c587b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -57,7 +57,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_GRAFTED] = "grafted",
};
-static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
+static const char *decorate_get_color(enum git_colorbool decorate_use_color, enum decoration_type ix)
{
if (want_color(decorate_use_color))
return decoration_colors[ix];
@@ -341,7 +341,7 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio
*/
void format_decorations(struct strbuf *sb,
const struct commit *commit,
- int use_color,
+ enum git_colorbool use_color,
const struct decoration_options *opts)
{
const struct name_decoration *decoration;
diff --git a/log-tree.h b/log-tree.h
index ebe491c543..07924be8bc 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -1,6 +1,8 @@
#ifndef LOG_TREE_H
#define LOG_TREE_H
+#include "color.h"
+
struct rev_info;
struct log_info {
@@ -26,7 +28,7 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
void show_log(struct rev_info *opt);
void format_decorations(struct strbuf *sb, const struct commit *commit,
- int use_color, const struct decoration_options *opts);
+ enum git_colorbool use_color, const struct decoration_options *opts);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
char **extra_headers_p,
diff --git a/parse-options-cb.c b/parse-options-cb.c
index e13e0a9e33..976cc86385 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -50,7 +50,7 @@ int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
int unset)
{
- int value;
+ enum git_colorbool value;
if (!arg)
arg = unset ? "never" : (const char *)opt->defval;
diff --git a/pretty.c b/pretty.c
index 86d69bf877..e0646bbc5d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -470,7 +470,7 @@ static inline void strbuf_add_with_color(struct strbuf *sb, const char *color,
static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
const char *line, size_t linelen,
- int color, enum grep_context ctx,
+ enum git_colorbool color, enum grep_context ctx,
enum grep_header_field field)
{
const char *buf, *eol, *line_color, *match_color;
@@ -899,7 +899,7 @@ struct format_commit_context {
const char *message;
char *commit_encoding;
size_t width, indent1, indent2;
- int auto_color;
+ enum git_colorbool auto_color;
int padding;
/* These offsets are relative to the start of the commit message. */
@@ -2167,7 +2167,7 @@ static int pp_utf8_width(const char *start, const char *end)
}
static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,
- int color, int tabwidth, const char *line,
+ enum git_colorbool color, int tabwidth, const char *line,
int linelen)
{
const char *tab;
diff --git a/pretty.h b/pretty.h
index df267afe4a..fac699033e 100644
--- a/pretty.h
+++ b/pretty.h
@@ -3,6 +3,7 @@
#include "date.h"
#include "string-list.h"
+#include "color.h"
struct commit;
struct repository;
@@ -46,7 +47,7 @@ struct pretty_print_context {
struct rev_info *rev;
const char *output_encoding;
struct string_list *mailmap;
- int color;
+ enum git_colorbool color;
struct ident_split *from_ident;
unsigned encode_email_headers:1;
struct pretty_print_describe_status *describe_status;
diff --git a/ref-filter.h b/ref-filter.h
index 644f5c567c..81f2c229a9 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -95,7 +95,7 @@ struct ref_format {
const char *format;
const char *rest;
int quote_style;
- int use_color;
+ enum git_colorbool use_color;
/* Internal state to ref-filter */
int need_color_reset_at_eol;
diff --git a/sideband.c b/sideband.c
index 3ac87148b9..ea7c25211e 100644
--- a/sideband.c
+++ b/sideband.c
@@ -27,9 +27,9 @@ static struct keyword_entry keywords[] = {
};
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
-static int use_sideband_colors(void)
+static enum git_colorbool use_sideband_colors(void)
{
- static int use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
+ static enum git_colorbool use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
const char *key = "color.remote";
struct strbuf sb = STRBUF_INIT;
diff --git a/transport.c b/transport.c
index ea0be4503c..c7f06a7382 100644
--- a/transport.c
+++ b/transport.c
@@ -30,7 +30,7 @@
#include "color.h"
#include "bundle-uri.h"
-static int transport_use_color = GIT_COLOR_UNKNOWN;
+static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
static char transport_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED /* REJECTED */
diff --git a/wt-status.h b/wt-status.h
index 4e377ce62b..e40a27214a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -111,7 +111,7 @@ struct wt_status {
int amend;
enum commit_whence whence;
int nowarn;
- int use_color;
+ enum git_colorbool use_color;
int no_gettext;
int display_comment_prefix;
int relative_paths;
--
2.51.0.526.gbfd906bacc
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/13] color: return bool from want_color()
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (9 preceding siblings ...)
2025-09-16 20:24 ` [PATCH 09/13] color: use git_colorbool enum to type to store colorbools Jeff King
@ 2025-09-16 20:25 ` Jeff King
2025-09-16 20:26 ` [PATCH 11/13] add-interactive: retain colorbool values longer Jeff King
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:25 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
The point of want_color() is to take in a git_colorbool enum value and
collapse it down to a single true/false boolean, letting UNKNOWN fall
back to the color.ui default and checking isatty() for AUTO.
Let's make that more clear in the type system by returning a bool rather
than an integer.
This sadly still does not help us much with compiler warnings for using
the two types interchangeably. But it helps make the intent more clear
to a human reader.
We still retain the idempotency of want_color(), because in C a bool
true/false converts to 1/0 when converted to an integer, which
corresponds to GIT_COLOR_ALWAYS and GIT_COLOR_NEVER. So you can store
the bool in a git_colorbool and get the right result (something a few
pieces of code still do, but which we'll clean up in further patches).
Note that we rely on this same bool/int conversion for
check_auto_color(). We cache its results in a tristate int with "-1" as
"not yet set", but we can assign to it (and return it) with implicit
conversions to/from bool.
Signed-off-by: Jeff King <peff@peff.net>
---
color.c | 8 ++++----
color.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/color.c b/color.c
index 3348ead534..07ac8c9d40 100644
--- a/color.c
+++ b/color.c
@@ -391,20 +391,20 @@ enum git_colorbool git_config_colorbool(const char *var, const char *value)
return GIT_COLOR_AUTO;
}
-static int check_auto_color(int fd)
+static bool check_auto_color(int fd)
{
static int color_stderr_is_tty = -1;
int *is_tty_p = fd == 1 ? &color_stdout_is_tty : &color_stderr_is_tty;
if (*is_tty_p < 0)
*is_tty_p = isatty(fd);
if (*is_tty_p || (fd == 1 && pager_in_use() && pager_use_color)) {
if (!is_terminal_dumb())
- return 1;
+ return true;
}
- return 0;
+ return false;
}
-int want_color_fd(int fd, enum git_colorbool var)
+bool want_color_fd(int fd, enum git_colorbool var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
diff --git a/color.h b/color.h
index fcb38c5562..43e6c9ad09 100644
--- a/color.h
+++ b/color.h
@@ -106,7 +106,7 @@ enum git_colorbool git_config_colorbool(const char *var, const char *value);
* Return a boolean whether to use color, where the argument 'var' is
* one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
*/
-int want_color_fd(int fd, enum git_colorbool var);
+bool want_color_fd(int fd, enum git_colorbool var);
#define want_color(colorbool) want_color_fd(1, (colorbool))
#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 11/13] add-interactive: retain colorbool values longer
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (10 preceding siblings ...)
2025-09-16 20:25 ` [PATCH 10/13] color: return bool from want_color() Jeff King
@ 2025-09-16 20:26 ` Jeff King
2025-09-16 20:26 ` [PATCH 12/13] config: store want_color() result in a separate bool Jeff King
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:26 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
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 <peff@peff.net>
---
I guess this could also come earlier in the series, but I found it
easier to understand the conversion if we start using the git_colorbool
value.
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];
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 12/13] config: store want_color() result in a separate bool
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (11 preceding siblings ...)
2025-09-16 20:26 ` [PATCH 11/13] add-interactive: retain colorbool values longer Jeff King
@ 2025-09-16 20:26 ` Jeff King
2025-09-16 20:27 ` [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct Jeff King
2025-09-16 20:28 ` [PATCH 0/13] unraveling the mysteries of color variables Jeff King
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:26 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
The "git config --get-colorbool foo.bar" command not only digs in the
config to find the value of foo.bar, it evaluates the result using
want_color() to check the tty-ness of stdout.
But it stores the bool result of want_color() in the same git_colorbool
that we found in the config. This works in practice because the
git_colorbool enum is a superset of the bool values. But it is an oddity
from a type system perspective.
Let's instead store the result in a separate bool and use that.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/config.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 9e4e4eb2f1..2348a99dd4 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -598,6 +598,7 @@ static int get_colorbool(const struct config_location_options *opts,
.get_diff_color_found = GIT_COLOR_UNKNOWN,
.get_color_ui_found = GIT_COLOR_UNKNOWN,
};
+ bool result;
config_with_options(git_get_colorbool_config, &data,
&opts->source, the_repository,
@@ -614,13 +615,13 @@ static int get_colorbool(const struct config_location_options *opts,
/* default value if none found in config */
data.get_colorbool_found = GIT_COLOR_AUTO;
- data.get_colorbool_found = want_color(data.get_colorbool_found);
+ result = want_color(data.get_colorbool_found);
if (print) {
- printf("%s\n", data.get_colorbool_found ? "true" : "false");
+ printf("%s\n", result ? "true" : "false");
return 0;
} else
- return data.get_colorbool_found ? 0 : 1;
+ return result ? 0 : 1;
}
static void check_write(const struct git_config_source *source)
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (12 preceding siblings ...)
2025-09-16 20:26 ` [PATCH 12/13] config: store want_color() result in a separate bool Jeff King
@ 2025-09-16 20:27 ` Jeff King
2025-09-16 20:35 ` Junio C Hamano
2025-09-16 20:28 ` [PATCH 0/13] unraveling the mysteries of color variables Jeff King
14 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:27 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
There were a few minor bugs fixed earlier in this series that were
caused by confusion between an "enum git_colorbool" (which has four
values) and a real true/false bool. It would be nice if the compiler
alerted us to such problems, but C's weak typing allows implicit
conversions between bool, int, and enums (and even with more recent
-Wenum-* warnings, I couldn't get either gcc or clang to complain about
the buggy constructs).
But there is one way we can bend C's type system to our will: turning
the colorbool variables into structs, which cannot be implicitly
converted to a bool.
Unfortunately, doing so is rather invasive and ugly. Ideally we could
just do this in color.h:
struct git_colorbool {
enum git_colorbool f;
};
typedef struct git_colorbool git_colorbool;
#define GIT_COLOR_AUTO (git_colorbool){GIT_COLOR_AUTO}
/* ...etc... */
and then most calling code would be happy. But:
- We don't use typedefs for enums in our project (so even with that
typedef, we have to drop the "enum" from "enum git_colorbool"
everywhere).
- The anonymous struct syntax above works for assigning and returning
values, but not for initializers. You have to use {GIT_COLOR_AUTO}
without the fake-typecast there.
- Likewise, C does not allow direct struct comparison at all! So for
"foo == GIT_COLOR_AUTO", you must actually dereference foo.f to get
to the field.
So this patch just makes all of the necessary changes throughout the
code base. This is not something we should apply, but it was useful for
flushing out all of the funny spots earlier in the series.
Signed-off-by: Jeff King <peff@peff.net>
---
add-interactive.c | 14 +++++++-------
add-interactive.h | 4 ++--
advice.c | 2 +-
builtin/add.c | 2 +-
builtin/am.c | 4 ++--
builtin/branch.c | 2 +-
builtin/clean.c | 2 +-
builtin/commit.c | 4 ++--
builtin/config.c | 20 ++++++++++----------
builtin/grep.c | 2 +-
builtin/log.c | 4 ++--
builtin/push.c | 2 +-
builtin/range-diff.c | 2 +-
builtin/show-branch.c | 2 +-
color.c | 24 ++++++++++++------------
color.h | 7 ++++---
combine-diff.c | 2 +-
diff.c | 18 +++++++++---------
diff.h | 4 ++--
grep.h | 4 ++--
log-tree.c | 4 ++--
log-tree.h | 2 +-
parse-options-cb.c | 6 +++---
pretty.c | 8 ++++----
pretty.h | 2 +-
ref-filter.h | 4 ++--
sequencer.c | 2 +-
sideband.c | 8 ++++----
transport.c | 2 +-
wt-status.c | 6 +++---
wt-status.h | 2 +-
31 files changed, 86 insertions(+), 85 deletions(-)
diff --git a/add-interactive.c b/add-interactive.c
index 6ffe64c38d..c16270dd60 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -20,7 +20,7 @@
#include "prompt.h"
#include "tree.h"
-static void init_color(struct repository *r, enum git_colorbool use_color,
+static void init_color(struct repository *r, git_colorbool use_color,
const char *section_and_slot, char *dst,
const char *default_color)
{
@@ -36,13 +36,13 @@ static void init_color(struct repository *r, enum git_colorbool use_color,
free(key);
}
-static enum git_colorbool check_color_config(struct repository *r, const char *var)
+static git_colorbool check_color_config(struct repository *r, const char *var)
{
const char *value;
- enum git_colorbool ret;
+ git_colorbool ret;
if (repo_config_get_value(r, var, &value))
- ret = GIT_COLOR_UNKNOWN;
+ ret.f = GIT_COLOR_UNKNOWN;
else
ret = git_config_colorbool(var, value);
@@ -51,7 +51,7 @@ static enum git_colorbool check_color_config(struct repository *r, const char *v
* the value parsed by git_color_config(), which may not have been
* called by the main command.
*/
- if (ret == GIT_COLOR_UNKNOWN &&
+ if (ret.f == GIT_COLOR_UNKNOWN &&
!repo_config_get_value(r, "color.ui", &value))
ret = git_config_colorbool("color.ui", value);
@@ -131,8 +131,8 @@ void clear_add_i_state(struct add_i_state *s)
FREE_AND_NULL(s->interactive_diff_filter);
FREE_AND_NULL(s->interactive_diff_algorithm);
memset(s, 0, sizeof(*s));
- s->use_color_interactive = GIT_COLOR_UNKNOWN;
- s->use_color_diff = GIT_COLOR_UNKNOWN;
+ s->use_color_interactive = (git_colorbool){GIT_COLOR_UNKNOWN};
+ s->use_color_diff = (git_colorbool){GIT_COLOR_UNKNOWN};
}
/*
diff --git a/add-interactive.h b/add-interactive.h
index da49502b76..7016ea2d2f 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -12,8 +12,8 @@ struct add_p_opt {
struct add_i_state {
struct repository *r;
- enum git_colorbool use_color_interactive;
- enum git_colorbool use_color_diff;
+ git_colorbool use_color_interactive;
+ git_colorbool use_color_diff;
char header_color[COLOR_MAXLEN];
char help_color[COLOR_MAXLEN];
char prompt_color[COLOR_MAXLEN];
diff --git a/advice.c b/advice.c
index 0018501b7b..21fa17d6a8 100644
--- a/advice.c
+++ b/advice.c
@@ -7,7 +7,7 @@
#include "help.h"
#include "string-list.h"
-static enum git_colorbool advice_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool advice_use_color = {GIT_COLOR_UNKNOWN};
static char advice_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_YELLOW, /* HINT */
diff --git a/builtin/add.c b/builtin/add.c
index 4cd3d183f9..c9ca3466a3 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -200,7 +200,7 @@ static int edit_patch(struct repository *repo,
argc = setup_revisions(argc, argv, &rev, NULL);
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
- rev.diffopt.use_color = GIT_COLOR_NEVER;
+ rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
rev.diffopt.flags.ignore_dirty_submodules = 1;
out = xopen(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
rev.diffopt.file = xfdopen(out, "w");
diff --git a/builtin/am.c b/builtin/am.c
index 277c2e7937..4c06b44607 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1408,7 +1408,7 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
rev_info.no_commit_id = 1;
rev_info.diffopt.flags.binary = 1;
rev_info.diffopt.flags.full_index = 1;
- rev_info.diffopt.use_color = GIT_COLOR_NEVER;
+ rev_info.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
rev_info.diffopt.file = fp;
rev_info.diffopt.close_file = 1;
add_pending_object(&rev_info, &commit->object, "");
@@ -1441,7 +1441,7 @@ static void write_index_patch(const struct am_state *state)
rev_info.disable_stdin = 1;
rev_info.no_commit_id = 1;
rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
- rev_info.diffopt.use_color = GIT_COLOR_NEVER;
+ rev_info.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
rev_info.diffopt.file = fp;
rev_info.diffopt.close_file = 1;
add_pending_object(&rev_info, &tree->object, "");
diff --git a/builtin/branch.c b/builtin/branch.c
index 9fcf04bebb..f5dd0e00be 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -46,7 +46,7 @@ static struct object_id head_oid;
static int recurse_submodules = 0;
static int submodule_propagate_branches = 0;
-static enum git_colorbool branch_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool branch_use_color = {GIT_COLOR_UNKNOWN};
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_NORMAL, /* PLAIN */
diff --git a/builtin/clean.c b/builtin/clean.c
index 1d5e7e5366..7f025aaea1 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -64,7 +64,7 @@ static const char *color_interactive_slots[] = {
[CLEAN_COLOR_RESET] = "reset",
};
-static enum git_colorbool clean_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool clean_use_color = {GIT_COLOR_UNKNOWN};
static char clean_colors[][COLOR_MAXLEN] = {
[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
diff --git a/builtin/commit.c b/builtin/commit.c
index d8f21a4c62..3c1c12845a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -936,7 +936,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
if (use_editor && include_status) {
int ident_shown = 0;
- enum git_colorbool saved_color_setting;
+ git_colorbool saved_color_setting;
struct ident_split ci, ai;
const char *hint_cleanup_all = allow_empty_message ?
_("Please enter the commit message for your changes."
@@ -1016,7 +1016,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
saved_color_setting = s->use_color;
- s->use_color = GIT_COLOR_NEVER;
+ s->use_color = (git_colorbool){GIT_COLOR_NEVER};
committable = run_status(s->fp, index_file, prefix, 1, s);
s->use_color = saved_color_setting;
string_list_clear_func(&s->change, change_data_free);
diff --git a/builtin/config.c b/builtin/config.c
index 2348a99dd4..4105c4c812 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -568,9 +568,9 @@ static void get_color(const struct config_location_options *opts,
}
struct get_colorbool_config_data {
- enum git_colorbool get_colorbool_found;
- enum git_colorbool get_diff_color_found;
- enum git_colorbool get_color_ui_found;
+ git_colorbool get_colorbool_found;
+ git_colorbool get_diff_color_found;
+ git_colorbool get_color_ui_found;
const char *get_colorbool_slot;
};
@@ -594,26 +594,26 @@ static int get_colorbool(const struct config_location_options *opts,
{
struct get_colorbool_config_data data = {
.get_colorbool_slot = var,
- .get_colorbool_found = GIT_COLOR_UNKNOWN,
- .get_diff_color_found = GIT_COLOR_UNKNOWN,
- .get_color_ui_found = GIT_COLOR_UNKNOWN,
+ .get_colorbool_found = {GIT_COLOR_UNKNOWN},
+ .get_diff_color_found = {GIT_COLOR_UNKNOWN},
+ .get_color_ui_found = {GIT_COLOR_UNKNOWN},
};
bool result;
config_with_options(git_get_colorbool_config, &data,
&opts->source, the_repository,
&opts->options);
- if (data.get_colorbool_found == GIT_COLOR_UNKNOWN) {
+ if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN) {
if (!strcmp(data.get_colorbool_slot, "color.diff"))
data.get_colorbool_found = data.get_diff_color_found;
- if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
+ if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN)
data.get_colorbool_found = data.get_color_ui_found;
}
- if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
+ if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN)
/* default value if none found in config */
- data.get_colorbool_found = GIT_COLOR_AUTO;
+ data.get_colorbool_found = (git_colorbool){GIT_COLOR_AUTO};
result = want_color(data.get_colorbool_found);
diff --git a/builtin/grep.c b/builtin/grep.c
index 1d97eb2a2a..3a6cdf9220 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1091,7 +1091,7 @@ int cmd_grep(int argc,
if (show_in_pager == default_pager)
show_in_pager = git_pager(the_repository, 1);
if (show_in_pager) {
- opt.color = GIT_COLOR_NEVER;
+ opt.color = (git_colorbool){GIT_COLOR_NEVER};
opt.name_only = 1;
opt.null_following_name = 1;
opt.output_priv = &path_list;
diff --git a/builtin/log.c b/builtin/log.c
index c2f8bbf863..960482c2f1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2193,8 +2193,8 @@ int cmd_format_patch(int argc,
output_directory = cfg.config_output_directory;
output_directory = set_outdir(prefix, output_directory);
- if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
- rev.diffopt.use_color = GIT_COLOR_NEVER;
+ if (rev.diffopt.use_color.f != GIT_COLOR_ALWAYS)
+ rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
/*
* We consider <outdir> as 'outside of gitdir', therefore avoid
* applying adjust_shared_perm in s-c-l-d.
diff --git a/builtin/push.c b/builtin/push.c
index 5b6cebbb85..2c84eaac7b 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -27,7 +27,7 @@ static const char * const push_usage[] = {
NULL,
};
-static enum git_colorbool push_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool push_use_color = {GIT_COLOR_UNKNOWN};
static char push_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED, /* ERROR */
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 0d51ddd623..94ceaffac2 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -67,7 +67,7 @@ int cmd_range_diff(int argc,
/* force color when --dual-color was used */
if (!simple_color)
- diffopt.use_color = GIT_COLOR_ALWAYS;
+ diffopt.use_color = (git_colorbool){GIT_COLOR_ALWAYS};
/* If `--diff-merges` was specified, imply `--merges` */
if (diff_merges_arg.nr) {
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 441babf2e3..d5e9eb399a 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -29,7 +29,7 @@ static const char*const show_branch_usage[] = {
NULL
};
-static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool showbranch_use_color = {GIT_COLOR_UNKNOWN};
static struct strvec default_args = STRVEC_INIT;
diff --git a/color.c b/color.c
index 07ac8c9d40..14e994ab19 100644
--- a/color.c
+++ b/color.c
@@ -9,7 +9,7 @@
#include "pager.h"
#include "strbuf.h"
-static enum git_colorbool git_use_color_default = GIT_COLOR_AUTO;
+static git_colorbool git_use_color_default = {GIT_COLOR_AUTO};
int color_stdout_is_tty = -1;
/*
@@ -369,26 +369,26 @@ int color_parse_mem(const char *value, int value_len, char *dst)
#undef OUT
}
-enum git_colorbool git_config_colorbool(const char *var, const char *value)
+git_colorbool git_config_colorbool(const char *var, const char *value)
{
if (value) {
if (!strcasecmp(value, "never"))
- return GIT_COLOR_NEVER;
+ return (git_colorbool){GIT_COLOR_NEVER};
if (!strcasecmp(value, "always"))
- return GIT_COLOR_ALWAYS;
+ return (git_colorbool){GIT_COLOR_ALWAYS};
if (!strcasecmp(value, "auto"))
- return GIT_COLOR_AUTO;
+ return (git_colorbool){GIT_COLOR_AUTO};
}
if (!var)
- return GIT_COLOR_UNKNOWN;
+ return (git_colorbool){GIT_COLOR_UNKNOWN};
/* Missing or explicit false to turn off colorization */
if (!git_config_bool(var, value))
- return GIT_COLOR_NEVER;
+ return (git_colorbool){GIT_COLOR_NEVER};
/* any normal truth value defaults to 'auto' */
- return GIT_COLOR_AUTO;
+ return (git_colorbool){GIT_COLOR_AUTO};
}
static bool check_auto_color(int fd)
@@ -404,7 +404,7 @@ static bool check_auto_color(int fd)
return false;
}
-bool want_color_fd(int fd, enum git_colorbool var)
+bool want_color_fd(int fd, git_colorbool var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
@@ -418,15 +418,15 @@ bool want_color_fd(int fd, enum git_colorbool var)
if (fd < 1 || fd >= ARRAY_SIZE(want_auto))
BUG("file descriptor out of range: %d", fd);
- if (var == GIT_COLOR_UNKNOWN)
+ if (var.f == GIT_COLOR_UNKNOWN)
var = git_use_color_default;
- if (var == GIT_COLOR_AUTO) {
+ if (var.f == GIT_COLOR_AUTO) {
if (want_auto[fd] < 0)
want_auto[fd] = check_auto_color(fd);
return want_auto[fd];
}
- return var == GIT_COLOR_ALWAYS;
+ return var.f == GIT_COLOR_ALWAYS;
}
int git_color_config(const char *var, const char *value, void *cb UNUSED)
diff --git a/color.h b/color.h
index 43e6c9ad09..5e735ce827 100644
--- a/color.h
+++ b/color.h
@@ -73,12 +73,13 @@ struct strbuf;
* 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.
*/
-enum git_colorbool {
+enum git_colorbool_f {
GIT_COLOR_UNKNOWN = -1,
GIT_COLOR_NEVER = 0,
GIT_COLOR_ALWAYS = 1,
GIT_COLOR_AUTO = 2,
};
+typedef struct { enum git_colorbool_f f; } git_colorbool;
/* A default list of colors to use for commit graphs and show-branch output */
extern const char *column_colors_ansi[];
@@ -100,13 +101,13 @@ int git_color_config(const char *var, const char *value, void *cb);
* GIT_COLOR_ALWAYS for "always" or a positive boolean,
* and GIT_COLOR_AUTO for "auto".
*/
-enum git_colorbool git_config_colorbool(const char *var, const char *value);
+git_colorbool git_config_colorbool(const char *var, const char *value);
/*
* Return a boolean whether to use color, where the argument 'var' is
* one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
*/
-bool want_color_fd(int fd, enum git_colorbool var);
+bool want_color_fd(int fd, git_colorbool var);
#define want_color(colorbool) want_color_fd(1, (colorbool))
#define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
diff --git a/combine-diff.c b/combine-diff.c
index 21b7fdfff4..d70939ed71 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -749,7 +749,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
static void dump_sline(struct sline *sline, const char *line_prefix,
unsigned long cnt, int num_parent,
- enum git_colorbool use_color, int result_deleted)
+ git_colorbool use_color, int result_deleted)
{
unsigned long mark = (1UL<<num_parent);
unsigned long no_pre_delete = (2UL<<num_parent);
diff --git a/diff.c b/diff.c
index 87fa16b730..59a9286aea 100644
--- a/diff.c
+++ b/diff.c
@@ -57,7 +57,7 @@ static int diff_detect_rename_default;
static int diff_indent_heuristic = 1;
static int diff_rename_limit_default = 1000;
static int diff_suppress_blank_empty;
-static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
+static git_colorbool diff_use_color_default = {GIT_COLOR_UNKNOWN};
static int diff_color_moved_default;
static int diff_color_moved_ws_default;
static int diff_context_default = 3;
@@ -595,7 +595,7 @@ static struct diff_tempfile {
} diff_temp[2];
struct emit_callback {
- int color_diff;
+ git_colorbool color_diff;
unsigned ws_rule;
int blank_at_eof_in_preimage;
int blank_at_eof_in_postimage;
@@ -2303,7 +2303,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
}
}
-const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix)
+const char *diff_get_color(git_colorbool diff_use_color, enum color_diff ix)
{
if (want_color(diff_use_color))
return diff_colors[ix];
@@ -4497,7 +4497,7 @@ static void fill_metainfo(struct strbuf *msg,
struct diff_options *o,
struct diff_filepair *p,
int *must_show_header,
- enum git_colorbool use_color)
+ git_colorbool use_color)
{
const char *set = diff_get_color(use_color, DIFF_METAINFO);
const char *reset = diff_get_color(use_color, DIFF_RESET);
@@ -4596,7 +4596,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
*/
fill_metainfo(msg, name, other, one, two, o, p,
&must_show_header,
- pgm ? GIT_COLOR_NEVER : o->use_color);
+ pgm ? (git_colorbool){GIT_COLOR_NEVER} : o->use_color);
xfrm_msg = msg->len ? msg->buf : NULL;
}
@@ -5277,7 +5277,7 @@ static int diff_opt_color_words(const struct option *opt,
struct diff_options *options = opt->value;
BUG_ON_OPT_NEG(unset);
- options->use_color = GIT_COLOR_ALWAYS;
+ options->use_color = (git_colorbool){GIT_COLOR_ALWAYS};
options->word_diff = DIFF_WORDS_COLOR;
options->word_regex = arg;
return 0;
@@ -5458,8 +5458,8 @@ static enum parse_opt_result diff_opt_output(struct parse_opt_ctx_t *ctx,
path = prefix_filename(ctx->prefix, arg);
options->file = xfopen(path, "w");
options->close_file = 1;
- if (options->use_color != GIT_COLOR_ALWAYS)
- options->use_color = GIT_COLOR_NEVER;
+ if (options->use_color.f != GIT_COLOR_ALWAYS)
+ options->use_color = (git_colorbool){GIT_COLOR_NEVER};
free(path);
return 0;
}
@@ -5599,7 +5599,7 @@ static int diff_opt_word_diff(const struct option *opt,
if (!strcmp(arg, "plain"))
options->word_diff = DIFF_WORDS_PLAIN;
else if (!strcmp(arg, "color")) {
- options->use_color = GIT_COLOR_ALWAYS;
+ options->use_color = (git_colorbool){GIT_COLOR_ALWAYS};
options->word_diff = DIFF_WORDS_COLOR;
}
else if (!strcmp(arg, "porcelain"))
diff --git a/diff.h b/diff.h
index bccd86a748..197dcb997f 100644
--- a/diff.h
+++ b/diff.h
@@ -284,7 +284,7 @@ struct diff_options {
/* diff-filter bits */
unsigned int filter, filter_not;
- enum git_colorbool use_color;
+ git_colorbool use_color;
/* Number of context lines to generate in patch output. */
int context;
@@ -470,7 +470,7 @@ enum color_diff {
DIFF_FILE_NEW_BOLD = 22,
};
-const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix);
+const char *diff_get_color(git_colorbool diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff_get_color((o)->use_color, ix)
diff --git a/grep.h b/grep.h
index 13e26a9318..0cc2806cae 100644
--- a/grep.h
+++ b/grep.h
@@ -159,7 +159,7 @@ struct grep_opt {
int pathname;
int null_following_name;
int only_matching;
- enum git_colorbool color;
+ git_colorbool color;
int max_depth;
int funcname;
int funcbody;
@@ -198,7 +198,7 @@ struct grep_opt {
[GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
}, \
.only_matching = 0, \
- .color = GIT_COLOR_UNKNOWN, \
+ .color = {GIT_COLOR_UNKNOWN}, \
.output = std_output, \
}
diff --git a/log-tree.c b/log-tree.c
index a2cd5c587b..df41c5c963 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -57,7 +57,7 @@ static const char *color_decorate_slots[] = {
[DECORATION_GRAFTED] = "grafted",
};
-static const char *decorate_get_color(enum git_colorbool decorate_use_color, enum decoration_type ix)
+static const char *decorate_get_color(git_colorbool decorate_use_color, enum decoration_type ix)
{
if (want_color(decorate_use_color))
return decoration_colors[ix];
@@ -341,7 +341,7 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio
*/
void format_decorations(struct strbuf *sb,
const struct commit *commit,
- enum git_colorbool use_color,
+ git_colorbool use_color,
const struct decoration_options *opts)
{
const struct name_decoration *decoration;
diff --git a/log-tree.h b/log-tree.h
index 1c82380d95..43ab208a7d 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -26,7 +26,7 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
void show_log(struct rev_info *opt);
void format_decorations(struct strbuf *sb, const struct commit *commit,
- enum git_colorbool use_color, const struct decoration_options *opts);
+ git_colorbool use_color, const struct decoration_options *opts);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
char **extra_headers_p,
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 976cc86385..5f0e027139 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -50,15 +50,15 @@ int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
int unset)
{
- enum git_colorbool value;
+ git_colorbool value;
if (!arg)
arg = unset ? "never" : (const char *)opt->defval;
value = git_config_colorbool(NULL, arg);
- if (value == GIT_COLOR_UNKNOWN)
+ if (value.f == GIT_COLOR_UNKNOWN)
return error(_("option `%s' expects \"always\", \"auto\", or \"never\""),
opt->long_name);
- *(int *)opt->value = value;
+ *(git_colorbool *)opt->value = value;
return 0;
}
diff --git a/pretty.c b/pretty.c
index e0646bbc5d..fcf7ee3d78 100644
--- a/pretty.c
+++ b/pretty.c
@@ -470,7 +470,7 @@ static inline void strbuf_add_with_color(struct strbuf *sb, const char *color,
static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
const char *line, size_t linelen,
- enum git_colorbool color, enum grep_context ctx,
+ git_colorbool color, enum grep_context ctx,
enum grep_header_field field)
{
const char *buf, *eol, *line_color, *match_color;
@@ -899,7 +899,7 @@ struct format_commit_context {
const char *message;
char *commit_encoding;
size_t width, indent1, indent2;
- enum git_colorbool auto_color;
+ git_colorbool auto_color;
int padding;
/* These offsets are relative to the start of the commit message. */
@@ -1462,7 +1462,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
} else {
int ret = parse_color(sb, placeholder, c);
if (ret)
- c->auto_color = GIT_COLOR_NEVER;
+ c->auto_color = (git_colorbool){GIT_COLOR_NEVER};
/*
* Otherwise, we decided to treat %C<unknown>
* as a literal string, and the previous
@@ -2167,7 +2167,7 @@ static int pp_utf8_width(const char *start, const char *end)
}
static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,
- enum git_colorbool color, int tabwidth, const char *line,
+ git_colorbool color, int tabwidth, const char *line,
int linelen)
{
const char *tab;
diff --git a/pretty.h b/pretty.h
index fac699033e..915c866c53 100644
--- a/pretty.h
+++ b/pretty.h
@@ -47,7 +47,7 @@ struct pretty_print_context {
struct rev_info *rev;
const char *output_encoding;
struct string_list *mailmap;
- enum git_colorbool color;
+ git_colorbool color;
struct ident_split *from_ident;
unsigned encode_email_headers:1;
struct pretty_print_describe_status *describe_status;
diff --git a/ref-filter.h b/ref-filter.h
index 81f2c229a9..66f96a330a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -95,7 +95,7 @@ struct ref_format {
const char *format;
const char *rest;
int quote_style;
- enum git_colorbool use_color;
+ git_colorbool use_color;
/* Internal state to ref-filter */
int need_color_reset_at_eol;
@@ -111,7 +111,7 @@ struct ref_format {
.exclude = STRVEC_INIT, \
}
#define REF_FORMAT_INIT { \
- .use_color = GIT_COLOR_UNKNOWN, \
+ .use_color = {GIT_COLOR_UNKNOWN}, \
}
/* Macros for checking --merged and --no-merged options */
diff --git a/sequencer.c b/sequencer.c
index 9ae40a91b2..56999de1ec 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3732,7 +3732,7 @@ static int make_patch(struct repository *r,
log_tree_opt.disable_stdin = 1;
log_tree_opt.no_commit_id = 1;
log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
- log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
+ log_tree_opt.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
if (!log_tree_opt.diffopt.file)
res |= error_errno(_("could not open '%s'"),
rebase_path_patch());
diff --git a/sideband.c b/sideband.c
index ea7c25211e..fb7b6a7e46 100644
--- a/sideband.c
+++ b/sideband.c
@@ -27,24 +27,24 @@ static struct keyword_entry keywords[] = {
};
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
-static enum git_colorbool use_sideband_colors(void)
+static git_colorbool use_sideband_colors(void)
{
- static enum git_colorbool use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
+ static git_colorbool use_sideband_colors_cached = {GIT_COLOR_UNKNOWN};
const char *key = "color.remote";
struct strbuf sb = STRBUF_INIT;
const char *value;
int i;
- if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
+ if (use_sideband_colors_cached.f != GIT_COLOR_UNKNOWN)
return use_sideband_colors_cached;
if (!repo_config_get_string_tmp(the_repository, key, &value))
use_sideband_colors_cached = git_config_colorbool(key, value);
else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value))
use_sideband_colors_cached = git_config_colorbool("color.ui", value);
else
- use_sideband_colors_cached = GIT_COLOR_AUTO;
+ use_sideband_colors_cached = (git_colorbool){GIT_COLOR_AUTO};
for (i = 0; i < ARRAY_SIZE(keywords); i++) {
strbuf_reset(&sb);
diff --git a/transport.c b/transport.c
index c7f06a7382..a5f98ba6b6 100644
--- a/transport.c
+++ b/transport.c
@@ -30,7 +30,7 @@
#include "color.h"
#include "bundle-uri.h"
-static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
+static git_colorbool transport_use_color = {GIT_COLOR_UNKNOWN};
static char transport_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
GIT_COLOR_RED /* REJECTED */
diff --git a/wt-status.c b/wt-status.c
index 8ffe6d3988..0927b88734 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -148,7 +148,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
memcpy(s->color_palette, default_wt_status_colors,
sizeof(default_wt_status_colors));
s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
- s->use_color = GIT_COLOR_UNKNOWN;
+ s->use_color = (git_colorbool){GIT_COLOR_UNKNOWN};
s->relative_paths = 1;
s->branch = refs_resolve_refdup(get_main_ref_store(the_repository),
"HEAD", 0, NULL, NULL);
@@ -1165,7 +1165,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
* before.
*/
if (s->fp != stdout) {
- rev.diffopt.use_color = GIT_COLOR_NEVER;
+ rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
wt_status_add_cut_line(s);
}
if (s->verbose > 1 && s->committable) {
@@ -2155,7 +2155,7 @@ static void wt_shortstatus_print(struct wt_status *s)
static void wt_porcelain_print(struct wt_status *s)
{
- s->use_color = GIT_COLOR_NEVER;
+ s->use_color = (git_colorbool){GIT_COLOR_NEVER};
s->relative_paths = 0;
s->prefix = NULL;
s->no_gettext = 1;
diff --git a/wt-status.h b/wt-status.h
index e40a27214a..5364eaa1f8 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -111,7 +111,7 @@ struct wt_status {
int amend;
enum commit_whence whence;
int nowarn;
- enum git_colorbool use_color;
+ git_colorbool use_color;
int no_gettext;
int display_comment_prefix;
int relative_paths;
--
2.51.0.527.g34bc42dacd
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct
2025-09-16 20:27 ` [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct Jeff King
@ 2025-09-16 20:35 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2025-09-16 20:35 UTC (permalink / raw)
To: Jeff King; +Cc: git, Patrick Steinhardt
Jeff King <peff@peff.net> writes:
> and then most calling code would be happy. But:
>
> - We don't use typedefs for enums in our project (so even with that
> typedef, we have to drop the "enum" from "enum git_colorbool"
> everywhere).
>
> - The anonymous struct syntax above works for assigning and returning
> values, but not for initializers. You have to use {GIT_COLOR_AUTO}
> without the fake-typecast there.
>
> - Likewise, C does not allow direct struct comparison at all! So for
> "foo == GIT_COLOR_AUTO", you must actually dereference foo.f to get
> to the field.
>
> So this patch just makes all of the necessary changes throughout the
> code base. This is not something we should apply, but it was useful for
> flushing out all of the funny spots earlier in the series.
Great show of technique to help others. It is somewhat surprising
that the patch can be this *small*, given the above ;-)
Amused, but as requested, will not apply.
Thanks.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> add-interactive.c | 14 +++++++-------
> add-interactive.h | 4 ++--
> advice.c | 2 +-
> builtin/add.c | 2 +-
> builtin/am.c | 4 ++--
> builtin/branch.c | 2 +-
> builtin/clean.c | 2 +-
> builtin/commit.c | 4 ++--
> builtin/config.c | 20 ++++++++++----------
> builtin/grep.c | 2 +-
> builtin/log.c | 4 ++--
> builtin/push.c | 2 +-
> builtin/range-diff.c | 2 +-
> builtin/show-branch.c | 2 +-
> color.c | 24 ++++++++++++------------
> color.h | 7 ++++---
> combine-diff.c | 2 +-
> diff.c | 18 +++++++++---------
> diff.h | 4 ++--
> grep.h | 4 ++--
> log-tree.c | 4 ++--
> log-tree.h | 2 +-
> parse-options-cb.c | 6 +++---
> pretty.c | 8 ++++----
> pretty.h | 2 +-
> ref-filter.h | 4 ++--
> sequencer.c | 2 +-
> sideband.c | 8 ++++----
> transport.c | 2 +-
> wt-status.c | 6 +++---
> wt-status.h | 2 +-
> 31 files changed, 86 insertions(+), 85 deletions(-)
>
> diff --git a/add-interactive.c b/add-interactive.c
> index 6ffe64c38d..c16270dd60 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -20,7 +20,7 @@
> #include "prompt.h"
> #include "tree.h"
>
> -static void init_color(struct repository *r, enum git_colorbool use_color,
> +static void init_color(struct repository *r, git_colorbool use_color,
> const char *section_and_slot, char *dst,
> const char *default_color)
> {
> @@ -36,13 +36,13 @@ static void init_color(struct repository *r, enum git_colorbool use_color,
> free(key);
> }
>
> -static enum git_colorbool check_color_config(struct repository *r, const char *var)
> +static git_colorbool check_color_config(struct repository *r, const char *var)
> {
> const char *value;
> - enum git_colorbool ret;
> + git_colorbool ret;
>
> if (repo_config_get_value(r, var, &value))
> - ret = GIT_COLOR_UNKNOWN;
> + ret.f = GIT_COLOR_UNKNOWN;
> else
> ret = git_config_colorbool(var, value);
>
> @@ -51,7 +51,7 @@ static enum git_colorbool check_color_config(struct repository *r, const char *v
> * the value parsed by git_color_config(), which may not have been
> * called by the main command.
> */
> - if (ret == GIT_COLOR_UNKNOWN &&
> + if (ret.f == GIT_COLOR_UNKNOWN &&
> !repo_config_get_value(r, "color.ui", &value))
> ret = git_config_colorbool("color.ui", value);
>
> @@ -131,8 +131,8 @@ void clear_add_i_state(struct add_i_state *s)
> FREE_AND_NULL(s->interactive_diff_filter);
> FREE_AND_NULL(s->interactive_diff_algorithm);
> memset(s, 0, sizeof(*s));
> - s->use_color_interactive = GIT_COLOR_UNKNOWN;
> - s->use_color_diff = GIT_COLOR_UNKNOWN;
> + s->use_color_interactive = (git_colorbool){GIT_COLOR_UNKNOWN};
> + s->use_color_diff = (git_colorbool){GIT_COLOR_UNKNOWN};
> }
>
> /*
> diff --git a/add-interactive.h b/add-interactive.h
> index da49502b76..7016ea2d2f 100644
> --- a/add-interactive.h
> +++ b/add-interactive.h
> @@ -12,8 +12,8 @@ struct add_p_opt {
>
> struct add_i_state {
> struct repository *r;
> - enum git_colorbool use_color_interactive;
> - enum git_colorbool use_color_diff;
> + git_colorbool use_color_interactive;
> + git_colorbool use_color_diff;
> char header_color[COLOR_MAXLEN];
> char help_color[COLOR_MAXLEN];
> char prompt_color[COLOR_MAXLEN];
> diff --git a/advice.c b/advice.c
> index 0018501b7b..21fa17d6a8 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -7,7 +7,7 @@
> #include "help.h"
> #include "string-list.h"
>
> -static enum git_colorbool advice_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool advice_use_color = {GIT_COLOR_UNKNOWN};
> static char advice_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_RESET,
> GIT_COLOR_YELLOW, /* HINT */
> diff --git a/builtin/add.c b/builtin/add.c
> index 4cd3d183f9..c9ca3466a3 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -200,7 +200,7 @@ static int edit_patch(struct repository *repo,
>
> argc = setup_revisions(argc, argv, &rev, NULL);
> rev.diffopt.output_format = DIFF_FORMAT_PATCH;
> - rev.diffopt.use_color = GIT_COLOR_NEVER;
> + rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> rev.diffopt.flags.ignore_dirty_submodules = 1;
> out = xopen(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
> rev.diffopt.file = xfdopen(out, "w");
> diff --git a/builtin/am.c b/builtin/am.c
> index 277c2e7937..4c06b44607 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1408,7 +1408,7 @@ static void write_commit_patch(const struct am_state *state, struct commit *comm
> rev_info.no_commit_id = 1;
> rev_info.diffopt.flags.binary = 1;
> rev_info.diffopt.flags.full_index = 1;
> - rev_info.diffopt.use_color = GIT_COLOR_NEVER;
> + rev_info.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> rev_info.diffopt.file = fp;
> rev_info.diffopt.close_file = 1;
> add_pending_object(&rev_info, &commit->object, "");
> @@ -1441,7 +1441,7 @@ static void write_index_patch(const struct am_state *state)
> rev_info.disable_stdin = 1;
> rev_info.no_commit_id = 1;
> rev_info.diffopt.output_format = DIFF_FORMAT_PATCH;
> - rev_info.diffopt.use_color = GIT_COLOR_NEVER;
> + rev_info.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> rev_info.diffopt.file = fp;
> rev_info.diffopt.close_file = 1;
> add_pending_object(&rev_info, &tree->object, "");
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 9fcf04bebb..f5dd0e00be 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -46,7 +46,7 @@ static struct object_id head_oid;
> static int recurse_submodules = 0;
> static int submodule_propagate_branches = 0;
>
> -static enum git_colorbool branch_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool branch_use_color = {GIT_COLOR_UNKNOWN};
> static char branch_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_RESET,
> GIT_COLOR_NORMAL, /* PLAIN */
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 1d5e7e5366..7f025aaea1 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -64,7 +64,7 @@ static const char *color_interactive_slots[] = {
> [CLEAN_COLOR_RESET] = "reset",
> };
>
> -static enum git_colorbool clean_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool clean_use_color = {GIT_COLOR_UNKNOWN};
> static char clean_colors[][COLOR_MAXLEN] = {
> [CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
> [CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d8f21a4c62..3c1c12845a 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -936,7 +936,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> strbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));
> if (use_editor && include_status) {
> int ident_shown = 0;
> - enum git_colorbool saved_color_setting;
> + git_colorbool saved_color_setting;
> struct ident_split ci, ai;
> const char *hint_cleanup_all = allow_empty_message ?
> _("Please enter the commit message for your changes."
> @@ -1016,7 +1016,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> status_printf_ln(s, GIT_COLOR_NORMAL, "%s", ""); /* Add new line for clarity */
>
> saved_color_setting = s->use_color;
> - s->use_color = GIT_COLOR_NEVER;
> + s->use_color = (git_colorbool){GIT_COLOR_NEVER};
> committable = run_status(s->fp, index_file, prefix, 1, s);
> s->use_color = saved_color_setting;
> string_list_clear_func(&s->change, change_data_free);
> diff --git a/builtin/config.c b/builtin/config.c
> index 2348a99dd4..4105c4c812 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -568,9 +568,9 @@ static void get_color(const struct config_location_options *opts,
> }
>
> struct get_colorbool_config_data {
> - enum git_colorbool get_colorbool_found;
> - enum git_colorbool get_diff_color_found;
> - enum git_colorbool get_color_ui_found;
> + git_colorbool get_colorbool_found;
> + git_colorbool get_diff_color_found;
> + git_colorbool get_color_ui_found;
> const char *get_colorbool_slot;
> };
>
> @@ -594,26 +594,26 @@ static int get_colorbool(const struct config_location_options *opts,
> {
> struct get_colorbool_config_data data = {
> .get_colorbool_slot = var,
> - .get_colorbool_found = GIT_COLOR_UNKNOWN,
> - .get_diff_color_found = GIT_COLOR_UNKNOWN,
> - .get_color_ui_found = GIT_COLOR_UNKNOWN,
> + .get_colorbool_found = {GIT_COLOR_UNKNOWN},
> + .get_diff_color_found = {GIT_COLOR_UNKNOWN},
> + .get_color_ui_found = {GIT_COLOR_UNKNOWN},
> };
> bool result;
>
> config_with_options(git_get_colorbool_config, &data,
> &opts->source, the_repository,
> &opts->options);
>
> - if (data.get_colorbool_found == GIT_COLOR_UNKNOWN) {
> + if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN) {
> if (!strcmp(data.get_colorbool_slot, "color.diff"))
> data.get_colorbool_found = data.get_diff_color_found;
> - if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
> + if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN)
> data.get_colorbool_found = data.get_color_ui_found;
> }
>
> - if (data.get_colorbool_found == GIT_COLOR_UNKNOWN)
> + if (data.get_colorbool_found.f == GIT_COLOR_UNKNOWN)
> /* default value if none found in config */
> - data.get_colorbool_found = GIT_COLOR_AUTO;
> + data.get_colorbool_found = (git_colorbool){GIT_COLOR_AUTO};
>
> result = want_color(data.get_colorbool_found);
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 1d97eb2a2a..3a6cdf9220 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -1091,7 +1091,7 @@ int cmd_grep(int argc,
> if (show_in_pager == default_pager)
> show_in_pager = git_pager(the_repository, 1);
> if (show_in_pager) {
> - opt.color = GIT_COLOR_NEVER;
> + opt.color = (git_colorbool){GIT_COLOR_NEVER};
> opt.name_only = 1;
> opt.null_following_name = 1;
> opt.output_priv = &path_list;
> diff --git a/builtin/log.c b/builtin/log.c
> index c2f8bbf863..960482c2f1 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2193,8 +2193,8 @@ int cmd_format_patch(int argc,
> output_directory = cfg.config_output_directory;
> output_directory = set_outdir(prefix, output_directory);
>
> - if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
> - rev.diffopt.use_color = GIT_COLOR_NEVER;
> + if (rev.diffopt.use_color.f != GIT_COLOR_ALWAYS)
> + rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> /*
> * We consider <outdir> as 'outside of gitdir', therefore avoid
> * applying adjust_shared_perm in s-c-l-d.
> diff --git a/builtin/push.c b/builtin/push.c
> index 5b6cebbb85..2c84eaac7b 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -27,7 +27,7 @@ static const char * const push_usage[] = {
> NULL,
> };
>
> -static enum git_colorbool push_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool push_use_color = {GIT_COLOR_UNKNOWN};
> static char push_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_RESET,
> GIT_COLOR_RED, /* ERROR */
> diff --git a/builtin/range-diff.c b/builtin/range-diff.c
> index 0d51ddd623..94ceaffac2 100644
> --- a/builtin/range-diff.c
> +++ b/builtin/range-diff.c
> @@ -67,7 +67,7 @@ int cmd_range_diff(int argc,
>
> /* force color when --dual-color was used */
> if (!simple_color)
> - diffopt.use_color = GIT_COLOR_ALWAYS;
> + diffopt.use_color = (git_colorbool){GIT_COLOR_ALWAYS};
>
> /* If `--diff-merges` was specified, imply `--merges` */
> if (diff_merges_arg.nr) {
> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index 441babf2e3..d5e9eb399a 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -29,7 +29,7 @@ static const char*const show_branch_usage[] = {
> NULL
> };
>
> -static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool showbranch_use_color = {GIT_COLOR_UNKNOWN};
>
> static struct strvec default_args = STRVEC_INIT;
>
> diff --git a/color.c b/color.c
> index 07ac8c9d40..14e994ab19 100644
> --- a/color.c
> +++ b/color.c
> @@ -9,7 +9,7 @@
> #include "pager.h"
> #include "strbuf.h"
>
> -static enum git_colorbool git_use_color_default = GIT_COLOR_AUTO;
> +static git_colorbool git_use_color_default = {GIT_COLOR_AUTO};
> int color_stdout_is_tty = -1;
>
> /*
> @@ -369,26 +369,26 @@ int color_parse_mem(const char *value, int value_len, char *dst)
> #undef OUT
> }
>
> -enum git_colorbool git_config_colorbool(const char *var, const char *value)
> +git_colorbool git_config_colorbool(const char *var, const char *value)
> {
> if (value) {
> if (!strcasecmp(value, "never"))
> - return GIT_COLOR_NEVER;
> + return (git_colorbool){GIT_COLOR_NEVER};
> if (!strcasecmp(value, "always"))
> - return GIT_COLOR_ALWAYS;
> + return (git_colorbool){GIT_COLOR_ALWAYS};
> if (!strcasecmp(value, "auto"))
> - return GIT_COLOR_AUTO;
> + return (git_colorbool){GIT_COLOR_AUTO};
> }
>
> if (!var)
> - return GIT_COLOR_UNKNOWN;
> + return (git_colorbool){GIT_COLOR_UNKNOWN};
>
> /* Missing or explicit false to turn off colorization */
> if (!git_config_bool(var, value))
> - return GIT_COLOR_NEVER;
> + return (git_colorbool){GIT_COLOR_NEVER};
>
> /* any normal truth value defaults to 'auto' */
> - return GIT_COLOR_AUTO;
> + return (git_colorbool){GIT_COLOR_AUTO};
> }
>
> static bool check_auto_color(int fd)
> @@ -404,7 +404,7 @@ static bool check_auto_color(int fd)
> return false;
> }
>
> -bool want_color_fd(int fd, enum git_colorbool var)
> +bool want_color_fd(int fd, git_colorbool var)
> {
> /*
> * NEEDSWORK: This function is sometimes used from multiple threads, and
> @@ -418,15 +418,15 @@ bool want_color_fd(int fd, enum git_colorbool var)
> if (fd < 1 || fd >= ARRAY_SIZE(want_auto))
> BUG("file descriptor out of range: %d", fd);
>
> - if (var == GIT_COLOR_UNKNOWN)
> + if (var.f == GIT_COLOR_UNKNOWN)
> var = git_use_color_default;
>
> - if (var == GIT_COLOR_AUTO) {
> + if (var.f == GIT_COLOR_AUTO) {
> if (want_auto[fd] < 0)
> want_auto[fd] = check_auto_color(fd);
> return want_auto[fd];
> }
> - return var == GIT_COLOR_ALWAYS;
> + return var.f == GIT_COLOR_ALWAYS;
> }
>
> int git_color_config(const char *var, const char *value, void *cb UNUSED)
> diff --git a/color.h b/color.h
> index 43e6c9ad09..5e735ce827 100644
> --- a/color.h
> +++ b/color.h
> @@ -73,12 +73,13 @@ struct strbuf;
> * 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.
> */
> -enum git_colorbool {
> +enum git_colorbool_f {
> GIT_COLOR_UNKNOWN = -1,
> GIT_COLOR_NEVER = 0,
> GIT_COLOR_ALWAYS = 1,
> GIT_COLOR_AUTO = 2,
> };
> +typedef struct { enum git_colorbool_f f; } git_colorbool;
>
> /* A default list of colors to use for commit graphs and show-branch output */
> extern const char *column_colors_ansi[];
> @@ -100,13 +101,13 @@ int git_color_config(const char *var, const char *value, void *cb);
> * GIT_COLOR_ALWAYS for "always" or a positive boolean,
> * and GIT_COLOR_AUTO for "auto".
> */
> -enum git_colorbool git_config_colorbool(const char *var, const char *value);
> +git_colorbool git_config_colorbool(const char *var, const char *value);
>
> /*
> * Return a boolean whether to use color, where the argument 'var' is
> * one of GIT_COLOR_UNKNOWN, GIT_COLOR_NEVER, GIT_COLOR_ALWAYS, GIT_COLOR_AUTO.
> */
> -bool want_color_fd(int fd, enum git_colorbool var);
> +bool want_color_fd(int fd, git_colorbool var);
> #define want_color(colorbool) want_color_fd(1, (colorbool))
> #define want_color_stderr(colorbool) want_color_fd(2, (colorbool))
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 21b7fdfff4..d70939ed71 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -749,7 +749,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
>
> static void dump_sline(struct sline *sline, const char *line_prefix,
> unsigned long cnt, int num_parent,
> - enum git_colorbool use_color, int result_deleted)
> + git_colorbool use_color, int result_deleted)
> {
> unsigned long mark = (1UL<<num_parent);
> unsigned long no_pre_delete = (2UL<<num_parent);
> diff --git a/diff.c b/diff.c
> index 87fa16b730..59a9286aea 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -57,7 +57,7 @@ static int diff_detect_rename_default;
> static int diff_indent_heuristic = 1;
> static int diff_rename_limit_default = 1000;
> static int diff_suppress_blank_empty;
> -static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
> +static git_colorbool diff_use_color_default = {GIT_COLOR_UNKNOWN};
> static int diff_color_moved_default;
> static int diff_color_moved_ws_default;
> static int diff_context_default = 3;
> @@ -595,7 +595,7 @@ static struct diff_tempfile {
> } diff_temp[2];
>
> struct emit_callback {
> - int color_diff;
> + git_colorbool color_diff;
> unsigned ws_rule;
> int blank_at_eof_in_preimage;
> int blank_at_eof_in_postimage;
> @@ -2303,7 +2303,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
> }
> }
>
> -const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix)
> +const char *diff_get_color(git_colorbool diff_use_color, enum color_diff ix)
> {
> if (want_color(diff_use_color))
> return diff_colors[ix];
> @@ -4497,7 +4497,7 @@ static void fill_metainfo(struct strbuf *msg,
> struct diff_options *o,
> struct diff_filepair *p,
> int *must_show_header,
> - enum git_colorbool use_color)
> + git_colorbool use_color)
> {
> const char *set = diff_get_color(use_color, DIFF_METAINFO);
> const char *reset = diff_get_color(use_color, DIFF_RESET);
> @@ -4596,7 +4596,7 @@ static void run_diff_cmd(const struct external_diff *pgm,
> */
> fill_metainfo(msg, name, other, one, two, o, p,
> &must_show_header,
> - pgm ? GIT_COLOR_NEVER : o->use_color);
> + pgm ? (git_colorbool){GIT_COLOR_NEVER} : o->use_color);
> xfrm_msg = msg->len ? msg->buf : NULL;
> }
>
> @@ -5277,7 +5277,7 @@ static int diff_opt_color_words(const struct option *opt,
> struct diff_options *options = opt->value;
>
> BUG_ON_OPT_NEG(unset);
> - options->use_color = GIT_COLOR_ALWAYS;
> + options->use_color = (git_colorbool){GIT_COLOR_ALWAYS};
> options->word_diff = DIFF_WORDS_COLOR;
> options->word_regex = arg;
> return 0;
> @@ -5458,8 +5458,8 @@ static enum parse_opt_result diff_opt_output(struct parse_opt_ctx_t *ctx,
> path = prefix_filename(ctx->prefix, arg);
> options->file = xfopen(path, "w");
> options->close_file = 1;
> - if (options->use_color != GIT_COLOR_ALWAYS)
> - options->use_color = GIT_COLOR_NEVER;
> + if (options->use_color.f != GIT_COLOR_ALWAYS)
> + options->use_color = (git_colorbool){GIT_COLOR_NEVER};
> free(path);
> return 0;
> }
> @@ -5599,7 +5599,7 @@ static int diff_opt_word_diff(const struct option *opt,
> if (!strcmp(arg, "plain"))
> options->word_diff = DIFF_WORDS_PLAIN;
> else if (!strcmp(arg, "color")) {
> - options->use_color = GIT_COLOR_ALWAYS;
> + options->use_color = (git_colorbool){GIT_COLOR_ALWAYS};
> options->word_diff = DIFF_WORDS_COLOR;
> }
> else if (!strcmp(arg, "porcelain"))
> diff --git a/diff.h b/diff.h
> index bccd86a748..197dcb997f 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -284,7 +284,7 @@ struct diff_options {
> /* diff-filter bits */
> unsigned int filter, filter_not;
>
> - enum git_colorbool use_color;
> + git_colorbool use_color;
>
> /* Number of context lines to generate in patch output. */
> int context;
> @@ -470,7 +470,7 @@ enum color_diff {
> DIFF_FILE_NEW_BOLD = 22,
> };
>
> -const char *diff_get_color(enum git_colorbool diff_use_color, enum color_diff ix);
> +const char *diff_get_color(git_colorbool diff_use_color, enum color_diff ix);
> #define diff_get_color_opt(o, ix) \
> diff_get_color((o)->use_color, ix)
>
> diff --git a/grep.h b/grep.h
> index 13e26a9318..0cc2806cae 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -159,7 +159,7 @@ struct grep_opt {
> int pathname;
> int null_following_name;
> int only_matching;
> - enum git_colorbool color;
> + git_colorbool color;
> int max_depth;
> int funcname;
> int funcbody;
> @@ -198,7 +198,7 @@ struct grep_opt {
> [GREP_COLOR_SEP] = GIT_COLOR_CYAN, \
> }, \
> .only_matching = 0, \
> - .color = GIT_COLOR_UNKNOWN, \
> + .color = {GIT_COLOR_UNKNOWN}, \
> .output = std_output, \
> }
>
> diff --git a/log-tree.c b/log-tree.c
> index a2cd5c587b..df41c5c963 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -57,7 +57,7 @@ static const char *color_decorate_slots[] = {
> [DECORATION_GRAFTED] = "grafted",
> };
>
> -static const char *decorate_get_color(enum git_colorbool decorate_use_color, enum decoration_type ix)
> +static const char *decorate_get_color(git_colorbool decorate_use_color, enum decoration_type ix)
> {
> if (want_color(decorate_use_color))
> return decoration_colors[ix];
> @@ -341,7 +341,7 @@ static void show_name(struct strbuf *sb, const struct name_decoration *decoratio
> */
> void format_decorations(struct strbuf *sb,
> const struct commit *commit,
> - enum git_colorbool use_color,
> + git_colorbool use_color,
> const struct decoration_options *opts)
> {
> const struct name_decoration *decoration;
> diff --git a/log-tree.h b/log-tree.h
> index 1c82380d95..43ab208a7d 100644
> --- a/log-tree.h
> +++ b/log-tree.h
> @@ -26,7 +26,7 @@ int log_tree_diff_flush(struct rev_info *);
> int log_tree_commit(struct rev_info *, struct commit *);
> void show_log(struct rev_info *opt);
> void format_decorations(struct strbuf *sb, const struct commit *commit,
> - enum git_colorbool use_color, const struct decoration_options *opts);
> + git_colorbool use_color, const struct decoration_options *opts);
> void show_decorations(struct rev_info *opt, struct commit *commit);
> void log_write_email_headers(struct rev_info *opt, struct commit *commit,
> char **extra_headers_p,
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index 976cc86385..5f0e027139 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -50,15 +50,15 @@ int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
> int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
> int unset)
> {
> - enum git_colorbool value;
> + git_colorbool value;
>
> if (!arg)
> arg = unset ? "never" : (const char *)opt->defval;
> value = git_config_colorbool(NULL, arg);
> - if (value == GIT_COLOR_UNKNOWN)
> + if (value.f == GIT_COLOR_UNKNOWN)
> return error(_("option `%s' expects \"always\", \"auto\", or \"never\""),
> opt->long_name);
> - *(int *)opt->value = value;
> + *(git_colorbool *)opt->value = value;
> return 0;
> }
>
> diff --git a/pretty.c b/pretty.c
> index e0646bbc5d..fcf7ee3d78 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -470,7 +470,7 @@ static inline void strbuf_add_with_color(struct strbuf *sb, const char *color,
>
> static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
> const char *line, size_t linelen,
> - enum git_colorbool color, enum grep_context ctx,
> + git_colorbool color, enum grep_context ctx,
> enum grep_header_field field)
> {
> const char *buf, *eol, *line_color, *match_color;
> @@ -899,7 +899,7 @@ struct format_commit_context {
> const char *message;
> char *commit_encoding;
> size_t width, indent1, indent2;
> - enum git_colorbool auto_color;
> + git_colorbool auto_color;
> int padding;
>
> /* These offsets are relative to the start of the commit message. */
> @@ -1462,7 +1462,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
> } else {
> int ret = parse_color(sb, placeholder, c);
> if (ret)
> - c->auto_color = GIT_COLOR_NEVER;
> + c->auto_color = (git_colorbool){GIT_COLOR_NEVER};
> /*
> * Otherwise, we decided to treat %C<unknown>
> * as a literal string, and the previous
> @@ -2167,7 +2167,7 @@ static int pp_utf8_width(const char *start, const char *end)
> }
>
> static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,
> - enum git_colorbool color, int tabwidth, const char *line,
> + git_colorbool color, int tabwidth, const char *line,
> int linelen)
> {
> const char *tab;
> diff --git a/pretty.h b/pretty.h
> index fac699033e..915c866c53 100644
> --- a/pretty.h
> +++ b/pretty.h
> @@ -47,7 +47,7 @@ struct pretty_print_context {
> struct rev_info *rev;
> const char *output_encoding;
> struct string_list *mailmap;
> - enum git_colorbool color;
> + git_colorbool color;
> struct ident_split *from_ident;
> unsigned encode_email_headers:1;
> struct pretty_print_describe_status *describe_status;
> diff --git a/ref-filter.h b/ref-filter.h
> index 81f2c229a9..66f96a330a 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -95,7 +95,7 @@ struct ref_format {
> const char *format;
> const char *rest;
> int quote_style;
> - enum git_colorbool use_color;
> + git_colorbool use_color;
>
> /* Internal state to ref-filter */
> int need_color_reset_at_eol;
> @@ -111,7 +111,7 @@ struct ref_format {
> .exclude = STRVEC_INIT, \
> }
> #define REF_FORMAT_INIT { \
> - .use_color = GIT_COLOR_UNKNOWN, \
> + .use_color = {GIT_COLOR_UNKNOWN}, \
> }
>
> /* Macros for checking --merged and --no-merged options */
> diff --git a/sequencer.c b/sequencer.c
> index 9ae40a91b2..56999de1ec 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3732,7 +3732,7 @@ static int make_patch(struct repository *r,
> log_tree_opt.disable_stdin = 1;
> log_tree_opt.no_commit_id = 1;
> log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
> - log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
> + log_tree_opt.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> if (!log_tree_opt.diffopt.file)
> res |= error_errno(_("could not open '%s'"),
> rebase_path_patch());
> diff --git a/sideband.c b/sideband.c
> index ea7c25211e..fb7b6a7e46 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -27,24 +27,24 @@ static struct keyword_entry keywords[] = {
> };
>
> /* Returns a color setting (GIT_COLOR_NEVER, etc). */
> -static enum git_colorbool use_sideband_colors(void)
> +static git_colorbool use_sideband_colors(void)
> {
> - static enum git_colorbool use_sideband_colors_cached = GIT_COLOR_UNKNOWN;
> + static git_colorbool use_sideband_colors_cached = {GIT_COLOR_UNKNOWN};
>
> const char *key = "color.remote";
> struct strbuf sb = STRBUF_INIT;
> const char *value;
> int i;
>
> - if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
> + if (use_sideband_colors_cached.f != GIT_COLOR_UNKNOWN)
> return use_sideband_colors_cached;
>
> if (!repo_config_get_string_tmp(the_repository, key, &value))
> use_sideband_colors_cached = git_config_colorbool(key, value);
> else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value))
> use_sideband_colors_cached = git_config_colorbool("color.ui", value);
> else
> - use_sideband_colors_cached = GIT_COLOR_AUTO;
> + use_sideband_colors_cached = (git_colorbool){GIT_COLOR_AUTO};
>
> for (i = 0; i < ARRAY_SIZE(keywords); i++) {
> strbuf_reset(&sb);
> diff --git a/transport.c b/transport.c
> index c7f06a7382..a5f98ba6b6 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -30,7 +30,7 @@
> #include "color.h"
> #include "bundle-uri.h"
>
> -static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN;
> +static git_colorbool transport_use_color = {GIT_COLOR_UNKNOWN};
> static char transport_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_RESET,
> GIT_COLOR_RED /* REJECTED */
> diff --git a/wt-status.c b/wt-status.c
> index 8ffe6d3988..0927b88734 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -148,7 +148,7 @@ void wt_status_prepare(struct repository *r, struct wt_status *s)
> memcpy(s->color_palette, default_wt_status_colors,
> sizeof(default_wt_status_colors));
> s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
> - s->use_color = GIT_COLOR_UNKNOWN;
> + s->use_color = (git_colorbool){GIT_COLOR_UNKNOWN};
> s->relative_paths = 1;
> s->branch = refs_resolve_refdup(get_main_ref_store(the_repository),
> "HEAD", 0, NULL, NULL);
> @@ -1165,7 +1165,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
> * before.
> */
> if (s->fp != stdout) {
> - rev.diffopt.use_color = GIT_COLOR_NEVER;
> + rev.diffopt.use_color = (git_colorbool){GIT_COLOR_NEVER};
> wt_status_add_cut_line(s);
> }
> if (s->verbose > 1 && s->committable) {
> @@ -2155,7 +2155,7 @@ static void wt_shortstatus_print(struct wt_status *s)
>
> static void wt_porcelain_print(struct wt_status *s)
> {
> - s->use_color = GIT_COLOR_NEVER;
> + s->use_color = (git_colorbool){GIT_COLOR_NEVER};
> s->relative_paths = 0;
> s->prefix = NULL;
> s->no_gettext = 1;
> diff --git a/wt-status.h b/wt-status.h
> index e40a27214a..5364eaa1f8 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -111,7 +111,7 @@ struct wt_status {
> int amend;
> enum commit_whence whence;
> int nowarn;
> - enum git_colorbool use_color;
> + git_colorbool use_color;
> int no_gettext;
> int display_comment_prefix;
> int relative_paths;
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/13] unraveling the mysteries of color variables
2025-09-16 20:10 [PATCH 0/13] unraveling the mysteries of color variables Jeff King
` (13 preceding siblings ...)
2025-09-16 20:27 ` [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct Jeff King
@ 2025-09-16 20:28 ` Jeff King
14 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2025-09-16 20:28 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
On Tue, Sep 16, 2025 at 04:10:37PM -0400, Jeff King wrote:
> The good news is that my digging did uncover two minor bugs, which are
> fixed here in patches 2 and 3. And the series should improve the
> readability of the code by using named constants and appropriate types
> more consistently.
Either counting is hard, or my brain is fried from looking at this color
code. At any rate, the fixes are in patches 3 and 5:
> [03/13]: grep: don't treat grep_opt.color as a strict bool
> [05/13]: diff: don't use diff_options.use_color as a strict bool
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread