Skip to content

Commit 955000d

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 12df3c2 commit 955000d

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

diff.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
16781678
const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
16791679
const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
16801680
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 : "";
16821682
static const char atat[2] = { '@', '@' };
16831683
const char *cp, *ep;
16841684
struct strbuf msgbuf = STRBUF_INIT;
@@ -1832,7 +1832,7 @@ static void emit_rewrite_diff(const char *name_a,
18321832
size_two = fill_textconv(o->repo, textconv_two, two, &data_two);
18331833

18341834
memset(&ecbdata, 0, sizeof(ecbdata));
1835-
ecbdata.color_diff = want_color(o->use_color);
1835+
ecbdata.color_diff = o->use_color;
18361836
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
18371837
ecbdata.opt = o;
18381838
if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
@@ -3729,7 +3729,7 @@ static void builtin_diff(const char *name_a,
37293729
if (o->flags.suppress_diff_headers)
37303730
lbl[0] = NULL;
37313731
ecbdata.label_path = lbl;
3732-
ecbdata.color_diff = want_color(o->use_color);
3732+
ecbdata.color_diff = o->use_color;
37333733
ecbdata.ws_rule = whitespace_rule(o->repo->index, name_b);
37343734
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
37353735
check_blank_at_eof(&mf1, &mf2, &ecbdata);

0 commit comments

Comments
 (0)