Skip to content

Commit 5e9ddd3

Browse files
peffgitster
authored andcommitted
pretty: use format_commit_context.auto_color as colorbool
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 955000d commit 5e9ddd3

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

pretty.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,8 +1455,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
14551455
switch (placeholder[0]) {
14561456
case 'C':
14571457
if (starts_with(placeholder + 1, "(auto)")) {
1458-
c->auto_color = want_color(c->pretty_ctx->color);
1459-
if (c->auto_color && sb->len)
1458+
c->auto_color = c->pretty_ctx->color;
1459+
if (want_color(c->auto_color) && sb->len)
14601460
strbuf_addstr(sb, GIT_COLOR_RESET);
14611461
return 7; /* consumed 7 bytes, "C(auto)" */
14621462
} else {

0 commit comments

Comments
 (0)