From: Jeff King <peff@peff•net>
To: git@vger•kernel.org
Cc: Patrick Steinhardt <ps@pks•im>
Subject: [PATCH 07/13] diff: stop passing ecbdata->use_color as boolean
Date: Tue, 16 Sep 2025 16:21:20 -0400 [thread overview]
Message-ID: <20250916202120.GG612873@coredump.intra.peff.net> (raw)
In-Reply-To: <20250916201036.GA612463@coredump.intra.peff.net>
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
next prev parent reply other threads:[~2025-09-16 20:21 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` [PATCH 02/13] color: return enum from git_config_colorbool() Jeff King
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 ` [PATCH 04/13] diff: simplify color_moved check when flushing Jeff King
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 ` [PATCH 06/13] diff: pass o->use_color directly to fill_metainfo() Jeff King
2025-09-16 20:21 ` Jeff King [this message]
2025-09-16 20:22 ` [PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool Jeff King
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 ` [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
2025-09-16 20:26 ` [PATCH 11/13] add-interactive: retain colorbool values longer Jeff King
2025-09-16 20:26 ` [PATCH 12/13] config: store want_color() result in a separate bool 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:35 ` Junio C Hamano
2025-09-16 20:28 ` [PATCH 0/13] unraveling the mysteries of color variables Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250916202120.GG612873@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox