git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool

From
Jeff King <peff@peff.net>
Date
Sep 16, 2025, 20:22 UTC
Message-ID
<20250916202226.GH612873@coredump.intra.peff.net>
In-Reply-To
<20250916201036.GA612463@coredump.intra.peff.net>

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>
---
 pretty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pretty.c b/pretty.c
index 0521deadc0..86d69bf877 100644
--- 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 {
-- 
2.51.0.527.g34bc42dacd
Previous: Jeff KingNext: Jeff King
Message 10 of 18 in “unraveling the mysteries of color variables”
  1. 0/13 unraveling the mysteries of color variablesJeff King, Sep 16, 2025
  2. Jeff KingSep 16, 2025
  3. 01/13 color: use GIT_COLOR_* instead of numeric constantsJeff King, Sep 16, 2025
  4. 02/13 color: return enum from git_config_colorbool()Jeff King, Sep 16, 2025
  5. 03/13 grep: don't treat grep_opt.color as a strict boolJeff King, Sep 16, 2025
  6. 04/13 diff: simplify color_moved check when flushingJeff King, Sep 16, 2025
  7. 05/13 diff: don't use diff_options.use_color as a strict boolJeff King, Sep 16, 2025
  8. 06/13 diff: pass o->use_color directly to fill_metainfo()Jeff King, Sep 16, 2025
  9. 07/13 diff: stop passing ecbdata->use_color as booleanJeff King, Sep 16, 2025
  10. 08/13 pretty: use format_commit_context.auto_color as colorboolJeff King, Sep 16, 2025
  11. 09/13 color: use git_colorbool enum to type to store colorboolsJeff King, Sep 16, 2025
  12. 9/13 color: use git_colorbool enum type to store colorboolsJeff King, Sep 16, 2025
  13. 10/13 color: return bool from want_color()Jeff King, Sep 16, 2025
  14. 11/13 add-interactive: retain colorbool values longerJeff King, Sep 16, 2025
  15. 12/13 config: store want_color() result in a separate boolJeff King, Sep 16, 2025
  16. 13/13 color: convert git_colorbool into a structJeff King, Sep 16, 2025
  17. Junio C HamanoSep 16, 2025
  18. Jeff KingSep 16, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.