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

[PATCH 03/13] grep: don't treat grep_opt.color as a strict bool

From
Jeff King <peff@peff.net>
Date
Sep 16, 2025, 20:16 UTC
Message-ID
<20250916201600.GC612873@coredump.intra.peff.net>
In-Reply-To
<20250916201036.GA612463@coredump.intra.peff.net>
In show_line(), we check to see if colors are desired with just:
  if (opt->color)
     ...we want colors...

But this is incorrect. The color field here is really a git_colorbool, so it may be "true" for GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO. Either of those _might_ end up true eventually (once we apply default fallbacks and check stdout's tty), but they may not. E.g.:

  git grep foo | cat

will enter the conditional even though we're not going to show colors. We should collapse it into a true boolean by calling want_color().

It turns out that this does not produce a user-visible bug. We do some extra processing to isolate the matched portion of the line in order to colorize it, but ultimately we pass it to our output_color() helper, which does correctly check want_color(). So we end up with no colors.

But dropping the extra processing saves a measurable amount of time. For example, running under hyperfine (which redirects to /dev/null, and thus does not colorize):

  Benchmark 1: ./git.old grep a
    Time (mean ± σ):      58.7 ms ±   3.5 ms    [User: 580.6 ms, System: 74.3 ms]
    Range (min … max):    53.5 ms …  67.1 ms    48 runs
  Benchmark 2: ./git.new grep a
    Time (mean ± σ):      35.5 ms ±   0.9 ms    [User: 276.8 ms, System: 73.8 ms]
    Range (min … max):    34.3 ms …  39.3 ms    79 runs
  Summary
    ./git.new grep a ran
      1.65 ± 0.11 times faster than ./git.old grep a

That's a fairly extreme benchmark, just because it will come up with a ton of small matches, but it shows that this really does matter.

Signed-off-by: Jeff King <peff@peff.net>
---
If you really want to cheat on the benchmark, grep for a single space,
which is even more common.
 grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index 932647e4a6..c7e1dc1e0e 100644
--- a/grep.c
+++ b/grep.c
@@ -1263,12 +1263,12 @@ static void show_line(struct grep_opt *opt,
 		 */
 		show_line_header(opt, name, lno, cno, sign);
 	}
-	if (opt->color || opt->only_matching) {
+	if (want_color(opt->color) || opt->only_matching) {
 		regmatch_t match;
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int eflags = 0;
 
-		if (opt->color) {
+		if (want_color(opt->color)) {
 			if (sign == ':')
 				match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
 			else
-- 
2.51.0.527.g34bc42dacd
Previous: Jeff KingNext: Jeff King
Message 5 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.