]> git.ipfire.org Git - thirdparty/git.git/commitdiff
grep: don't treat grep_opt.color as a strict bool
authorJeff King <peff@peff.net>
Tue, 16 Sep 2025 20:16:00 +0000 (16:16 -0400)
committerJunio C Hamano <gitster@pobox.com>
Tue, 16 Sep 2025 20:37:05 +0000 (13:37 -0700)
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep.c

diff --git a/grep.c b/grep.c
index 932647e4a6580b52973c90b8d94494e3bb31bbdc..c7e1dc1e0ee4fea88f9e768ea04401320a497ea9 100644 (file)
--- 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