[PATCH 05/13] diff: don't use diff_options.use_color as a strict bool

We disable --color-moved if color is not in use at all. This happens in diff_setup_done(), where we set options->color_moved to 0 if options->use_color is not true. But a strict boolean check here is not correct; use_color could be GIT_COLOR_UNKNOWN or GIT_COLOR_AUTO, both of which evaluate to true, even though we may later decide not to show colors.

We should be using want_color() to convert that git_colorbool into a true boolean. As it turns out, this does not produce wrong output. Even though we go to the trouble to detect the moved lines, ultimately we get the color values from diff_get_color(), which does check want_color(). And so it returns the empty string for each color, and we "color" the result with nothing.

So the output is correct, but there is a small but measurable performance cost to doing the line detection. E.g., in git.git before and after this patch (there are no colors shown because hyperfine redirects output to /dev/null):

  Benchmark 1: ./git.old log --no-merges -p --color-moved -1000
    Time (mean ± σ):      1.019 s ±  0.013 s    [User: 0.955 s, System: 0.064 s]
    Range (min … max):    1.005 s …  1.045 s    10 runs

  Benchmark 2: ./git.new log --no-merges -p --color-moved -1000
    Time (mean ± σ):     982.9 ms ±  14.5 ms    [User: 925.8 ms, System: 57.1 ms]
    Range (min … max):   965.1 ms … 1003.2 ms    10 runs

  Summary
    ./git.new log --no-merges -p --color-moved -1000 ran
      1.04 ± 0.02 times faster than ./git.old log --no-merges -p --color-moved -1000

Note that the fix is not quite as simple as just calling want_color() from diff_setup_done(). There's a subtle timing issue that goes back to daa0c3d971 (color: delay auto-color decision until point of use, 2011-08-17), the commit that adds want_color() in the first place. As discussed there, we must delay evaluating the colorbool value until all pager setup is complete.

So instead, we'll leave the "color_moved" field intact in diff_setup_done(), and modify the point where it is evaluated. Fortunately there is only one such spot that controls whether we run any of the color-moved code at all.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)