@matheusbsbs / matheus-bagliano / commits / 955000d917

diff: stop passing ecbdata->use_color as boolean

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 16, 2025 at 16:21 UTC 955000d91718eee5abd005dd43ed035a5115d870

1 file changed +3 -3

diff.c

+3 -3

@@ -1678,7 +1678,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,

1678 const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);

1679 const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);

1680 const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);

1681 - const char *reverse = ecbdata->color_diff ? GIT_COLOR_REVERSE : "";

1681 + const char *reverse = want_color(ecbdata->color_diff) ? GIT_COLOR_REVERSE : "";

1682 static const char atat[2] = { '@', '@' };

1683 const char *cp, *ep;

1684 struct strbuf msgbuf = STRBUF_INIT;

@@ -1832,7 +1832,7 @@ static void emit_rewrite_diff(const char *name_a,

1832 size_two = fill_textconv(o->repo, textconv_two, two, &data_two);

1833

1834 memset(&ecbdata, 0, sizeof(ecbdata));

1835 - ecbdata.color_diff = want_color(o->use_color);

1835 + ecbdata.color_diff = o->use_color;

1836 ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);

1837 ecbdata.opt = o;

1838 if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {

@@ -3729,7 +3729,7 @@ static void builtin_diff(const char *name_a,

3729 if (o->flags.suppress_diff_headers)

3730 lbl[0] = NULL;

3731 ecbdata.label_path = lbl;

3732 - ecbdata.color_diff = want_color(o->use_color);

3732 + ecbdata.color_diff = o->use_color;

3733 ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);

3734 if (ecbdata.ws_rule & WS_BLANK_AT_EOF)

3735 check_blank_at_eof(&mf1, &mf2, &ecbdata);