]> git.ipfire.org Git - thirdparty/git.git/commitdiff
pretty: use format_commit_context.auto_color as colorbool
authorJeff King <peff@peff.net>
Tue, 16 Sep 2025 20:22:26 +0000 (16:22 -0400)
committerJunio C Hamano <gitster@pobox.com>
Tue, 16 Sep 2025 20:37:06 +0000 (13:37 -0700)
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
pretty.c

index 0521deadc0cb6679013c7ab6597248ea4d4897eb..86d69bf8772de3e5a66a8f1038acc81dd0aa7b92 100644 (file)
--- 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 {