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>

diff --git a/diff.c b/diff.c
index 505819c6c62234bf3cf9721929badd65ad398b02..3544be2318ca664f7c1e583b2b5a194099dd4615 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1678,7 +1678,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;
@@ -1832,7 +1832,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) {
@@ -3729,7 +3729,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);