Volume XXII, number 280Wednesday, October 7, 2026Latest message 53 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patch, 8 partssome diff-highlight tweaks

28 messages between Mar 20, 2026 and Mar 24, 2026, from Jeff King, Tian Yuchen, Junio C Hamano, Patrick Steinhardt.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Jeff KingMar 20, 2026, 00:41 UTC on lore

Here are a few small changes to diff-highlight. The main motivation is working better with diff-so-fancy, which uses DiffHighlight.pm under the hood. But it was a good opportunity to polish up the tests and README, and the final patch implements a small optimization I'd been meaning to do for a while.

I based these on the bugfix patch I sent a few days ago in:
  https://lore.kernel.org/git/20260317230223.GA716496@coredump.intra.peff.net/

They don't _need_ to come after that, but there are otherwise textual conflicts as they both add new tests in the same spot.

  [1/8]: diff-highlight: mention build instructions
  [2/8]: diff-highlight: drop perl version dependency back to 5.8
  [3/8]: diff-highlight: check diff-highlight exit status in tests
  [4/8]: t: add matching negative attributes to test_decode_color
  [5/8]: diff-highlight: use test_decode_color in tests
  [6/8]: diff-highlight: test color config
  [7/8]: diff-highlight: allow module callers to pass in color config
  [8/8]: diff-highlight: fetch all config with one process
 contrib/diff-highlight/DiffHighlight.pm       | 57 ++++++++++++----
 contrib/diff-highlight/README                 | 19 +++++-
 .../diff-highlight/t/t9400-diff-highlight.sh  | 67 +++++++++++++------
 t/test-lib-functions.sh                       |  3 +
 4 files changed, 111 insertions(+), 35 deletions(-)
-Peff
Jeff KingMar 20, 2026, 00:42 UTC in reply to Jeff King on lore

[PATCH 1/8] diff-highlight: mention build instructions

Once upon a time, this was just a script in a directory that could be run directly. That changed in 0c977dbc81 (diff-highlight: split code into module, 2017-06-15). Let's update the README to make it more clear that you need to run make.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/README | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
Show changes to contrib/diff-highlight/README +12 −1
diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 1db4440e68..9c89146fb0 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -39,10 +39,21 @@ visually distracting.  Non-diff lines and existing diff coloration is
 preserved; the intent is that the output should look exactly the same as
 the input, except for the occasional highlight.
 
+Build/Install
+-------------
+
+You can build the `diff-highlight` script by running `make` from within
+the diff-highlight directory. There is no `make install` target; you can
+copy the built script to your $PATH.
+
+You can run diff-highlight's internal tests by running `make test`. Note
+that you must also build Git itself first (by running `make` from the
+top-level of the project).
+
 Use
 ---
 
-You can try out the diff-highlight program with:
+You can try out the built diff-highlight program with:
 
 ---------------------------------------------
 git log -p --color | /path/to/diff-highlight

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:42 UTC in reply to Jeff King on lore

[PATCH 2/8] diff-highlight: drop perl version dependency back to 5.8

From: Scott Baker <scott@perturb.org>

The diff-highlight code does not rely on any perl features beyond what perl 5.8 provides. We bumped it to v5.26 along with the rest of the project's perl scripts in 702d8c1f3b (Require Perl 5.26.0, 2024-10-23).

There's some value in just having a uniform baseline for the project, but I think diff-highlight is special here:

  - it's in a contrib/ directory that is not frequently touched, so
    there is little risk of Git developers getting annoyed that modern
    perl features are not available
  - it provides a module used by other projects. In particular,
    diff-so-fancy relies on DiffHighlight.pm but does not otherwise
    require a perl version more modern than 5.8.
Let's drop back to the more conservative requirement.
Signed-off-by: Scott Baker <scott@perturb.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/diff-highlight/DiffHighlight.pm +1 −1
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index f0607a4b68..a5e5de3b18 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -1,6 +1,6 @@
 package DiffHighlight;
 
-require v5.26;
+require v5.008;
 use warnings FATAL => 'all';
 use strict;
 

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:43 UTC in reply to Jeff King on lore

[PATCH 3/8] diff-highlight: check diff-highlight exit status in tests

When testing diff-highlight, we pipe the output through a sanitizing function. This loses the exit status of diff-highlight itself, which could mean we are missing cases where it crashes or exits unexpectedly. Use an extra tempfile to avoid the pipe.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +4 −2
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 2a9b68cf3b..42d331c6cd 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -41,8 +41,10 @@ dh_test () {
 		git show >commit.raw
 	} >/dev/null &&
 
-	"$DIFF_HIGHLIGHT" <diff.raw | test_strip_patch_header >diff.act &&
-	"$DIFF_HIGHLIGHT" <commit.raw | test_strip_patch_header >commit.act &&
+	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
+	test_strip_patch_header <diff.hi >diff.act
+	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
+	test_strip_patch_header <commit.hi >commit.act &&
 	test_cmp patch.exp diff.act &&
 	test_cmp patch.exp commit.act
 }

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:43 UTC in reply to Jeff King on lore

[PATCH 4/8] t: add matching negative attributes to test_decode_color

Most of the ANSI color attributes have an "off" variant. We don't use these yet in our test suite, so we never bothered to decode them. Add the ones that match the attributes we encode so we can make use of them.

There are even more attributes not covered on the positive side, so this is meant to be useful but not all-inclusive.

Note that "nobold" and "nodim" are the same code, so I've decoded this as "normal intensity".

Signed-off-by: Jeff King <peff@peff.net>
---
This is the only patch that touches anything outside of
contrib/diff-highlight, but hopefully it is uncontroversial. ;)
 t/test-lib-functions.sh | 3 +++
 1 file changed, 3 insertions(+)
Show changes to t/test-lib-functions.sh +3 −0
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 14e238d24d..f3af10fb7e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -48,6 +48,9 @@ test_decode_color () {
 			if (n == 2) return "FAINT";
 			if (n == 3) return "ITALIC";
 			if (n == 7) return "REVERSE";
+			if (n == 22) return "NORMAL_INTENSITY";
+			if (n == 23) return "NOITALIC";
+			if (n == 27) return "NOREVERSE";
 			if (n == 30) return "BLACK";
 			if (n == 31) return "RED";
 			if (n == 32) return "GREEN";

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:44 UTC in reply to Jeff King on lore

[PATCH 5/8] diff-highlight: use test_decode_color in tests

The diff-highlight tests use raw color bytes when comparing expected and actual output. Let's use test_decode_color, which is our usual technique in other tests. It makes reading test output diffs a bit easier, since you're not relying on your terminal to interpret the result (or worse, interpreting characters yourself via "cat -A").

This will also make it easier to add tests with new colors/attributes, without having to pre-define the byte sequences ourselves.

Signed-off-by: Jeff King <peff@peff.net>
---
 .../diff-highlight/t/t9400-diff-highlight.sh  | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +17 −20
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 42d331c6cd..ba80cda7c8 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)
 TEST_DIRECTORY="$CURR_DIR"/../../../t
 DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
 
-CW="$(printf "\033[7m")"	# white
-CR="$(printf "\033[27m")"	# reset
-
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . "$TEST_DIRECTORY"/test-lib.sh
@@ -42,9 +39,9 @@ dh_test () {
 	} >/dev/null &&
 
 	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
-	test_strip_patch_header <diff.hi >diff.act
+	test_strip_patch_header <diff.hi | test_decode_color >diff.act
 	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
-	test_strip_patch_header <commit.hi >commit.act &&
+	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
 	test_cmp patch.exp diff.act &&
 	test_cmp patch.exp commit.act
 }
@@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-${CW}b${CR}bb
-		+${CW}0${CR}bb
+		-<REVERSE>b<NOREVERSE>bb
+		+<REVERSE>0<NOREVERSE>bb
 		 ccc
 	EOF
 '
@@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-bb${CW}b${CR}
-		+bb${CW}0${CR}
+		-bb<REVERSE>b<NOREVERSE>
+		+bb<REVERSE>0<NOREVERSE>
 		 ccc
 	EOF
 '
@@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-b${CW}b${CR}b
-		+b${CW}0${CR}b
+		-b<REVERSE>b<NOREVERSE>b
+		+b<REVERSE>0<NOREVERSE>b
 		 ccc
 	EOF
 '
@@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-b${CW}b${CR}b
-		+b${CW}0${CR}b
+		-b<REVERSE>b<NOREVERSE>b
+		+b<REVERSE>0<NOREVERSE>b
 		+ccc
 	EOF
 '
@@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '
 	echo "unic${o_stroke}de" >b &&
 	dh_test a b <<-EOF
 		@@ -1 +1 @@
-		-unic${CW}${o_accent}${CR}de
-		+unic${CW}${o_stroke}${CR}de
+		-unic<REVERSE>${o_accent}<NOREVERSE>de
+		+unic<REVERSE>${o_stroke}<NOREVERSE>de
 	EOF
 '
 
@@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '
 	echo "unico${combine_circum}de" >b &&
 	dh_test a b <<-EOF
 		@@ -1 +1 @@
-		-unic${CW}o${combine_accent}${CR}de
-		+unic${CW}o${combine_circum}${CR}de
+		-unic<REVERSE>o${combine_accent}<NOREVERSE>de
+		+unic<REVERSE>o${combine_circum}<NOREVERSE>de
 	EOF
 '
 
@@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '
 	+++ b/file
 	@@ -1,3 +1,3 @@
 	 before
-	-the ${CW}old${CR} line
-	+the ${CW}new${CR} line
+	-the <REVERSE>old<NOREVERSE> line
+	+the <REVERSE>new<NOREVERSE> line
 	 -leading dash
 	EOF
 	git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw &&
-	trim_graph <actual.raw | sed -n "/^---/,\$p" >actual &&
+	trim_graph <actual.raw | sed -n "/^---/,\$p" | test_decode_color >actual &&
 	test_cmp expect actual
 '
 

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:45 UTC in reply to Jeff King on lore

[PATCH 6/8] diff-highlight: test color config

We added configurable colors long ago in bca45fbc1f (diff-highlight: allow configurable colors, 2014-11-20), but never actually tested it. Since we'll be touching the color code in a moment, this is a good time to beef up the tests.

Note that we cover both the highlight/reset style used by the default colors, as well as the normal/highlight style added by that commit (which was previously totally untested).

Signed-off-by: Jeff King <peff@peff.net>
---
 .../diff-highlight/t/t9400-diff-highlight.sh  | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +28 −0
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index ba80cda7c8..828d59e9c6 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -350,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' '
 	EOF
 '
 
+test_expect_success 'configure set/reset colors' '
+	test_config color.diff-highlight.oldhighlight bold &&
+	test_config color.diff-highlight.oldreset nobold &&
+	test_config color.diff-highlight.newhighlight italic &&
+	test_config color.diff-highlight.newreset noitalic &&
+	echo "prefix a suffix" >a &&
+	echo "prefix b suffix" >b &&
+	dh_test a b <<-\EOF
+	@@ -1 +1 @@
+	-prefix <BOLD>a<NORMAL_INTENSITY> suffix
+	+prefix <ITALIC>b<NOITALIC> suffix
+	EOF
+'
+
+test_expect_success 'configure normal/highlight colors' '
+	test_config color.diff-highlight.oldnormal red &&
+	test_config color.diff-highlight.oldhighlight magenta &&
+	test_config color.diff-highlight.newnormal green &&
+	test_config color.diff-highlight.newhighlight yellow &&
+	echo "prefix a suffix" >a &&
+	echo "prefix b suffix" >b &&
+	dh_test a b <<-\EOF
+	@@ -1 +1 @@
+	<RED>-prefix <RESET><MAGENTA>a<RESET><RED> suffix<RESET>
+	<GREEN>+prefix <RESET><YELLOW>b<RESET><GREEN> suffix<RESET>
+	EOF
+'
+
 test_done

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:47 UTC in reply to Jeff King on lore

[PATCH 7/8] diff-highlight: allow module callers to pass in color config

From: Scott Baker <scott@perturb.org>

Users of the module may want to pass in their own color config for a few obvious reasons:

  - they are pulling the config from different variables than
    diff-highlight itself uses
  - they are loading the config in a more efficient way (say, by parsing
    git-config --list) and don't want to incur the six (!) git-config
    calls that DiffHighlight.pm runs to check all config

Let's allow users of the module to pass in the color config, and lazy-load it when needed if they haven't.

Signed-off-by: Scott Baker <scott@perturb.org>
Signed-off-by: Jeff King <peff@peff.net>
---
The next commit improves the six-process situation, but I think
diff-so-fancy will still want this, as it uses a single invocation
which checks other non-color config (and already bit the bullet on
implementing its own color parsing).
 contrib/diff-highlight/DiffHighlight.pm | 41 +++++++++++++++++--------
 contrib/diff-highlight/README           |  6 ++++
 2 files changed, 35 insertions(+), 12 deletions(-)
Show changes to 2 files +35 −12

contrib/diff-highlight/DiffHighlight.pm, contrib/diff-highlight/README

diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index a5e5de3b18..96369eadf9 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -9,18 +9,11 @@ package DiffHighlight;
 
 my $NULL = File::Spec->devnull();
 
-# Highlight by reversing foreground and background. You could do
-# other things like bold or underline if you prefer.
-my @OLD_HIGHLIGHT = (
-	color_config('color.diff-highlight.oldnormal'),
-	color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
-	color_config('color.diff-highlight.oldreset', "\x1b[27m")
-);
-my @NEW_HIGHLIGHT = (
-	color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
-	color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
-	color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
-);
+# The color theme is initially set to nothing here to allow outside callers
+# to set the colors for their application. If nothing is sent in we use
+# colors from git config in load_color_config().
+our @OLD_HIGHLIGHT = ();
+our @NEW_HIGHLIGHT = ();
 
 my $RESET = "\x1b[m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
@@ -170,6 +163,29 @@ sub show_hunk {
 	$line_cb->(@queue);
 }
 
+sub load_color_config {
+	# If the colors were NOT set from outside this module we load them on-demand
+	# from the git config. Note that only one of elements 0 and 2 in each
+	# array is used (depending on whether you are doing set/unset on an
+	# attribute, or specifying normal vs highlighted coloring). So we use
+	# element 1 as our check for whether colors were passed in; it should
+	# always be set if you want highlighting to do anything.
+	if (!defined $OLD_HIGHLIGHT[1]) {
+		@OLD_HIGHLIGHT = (
+			color_config('color.diff-highlight.oldnormal'),
+			color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
+			color_config('color.diff-highlight.oldreset', "\x1b[27m")
+		);
+	}
+	if (!defined $NEW_HIGHLIGHT[1]) {
+		@NEW_HIGHLIGHT = (
+			color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
+			color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
+			color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
+		);
+	};
+}
+
 sub highlight_pair {
 	my @a = split_line(shift);
 	my @b = split_line(shift);
@@ -218,6 +234,7 @@ sub highlight_pair {
 	}
 
 	if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) {
+		load_color_config();
 		return highlight_line(\@a, $pa, $sa, \@OLD_HIGHLIGHT),
 		       highlight_line(\@b, $pb, $sb, \@NEW_HIGHLIGHT);
 	}
diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 9c89146fb0..ed8d876a18 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -138,6 +138,12 @@ Your script may set up one or more of the following variables:
     processing a logical chunk of input). The default function flushes
     stdout.
 
+  - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these
+    arrays specify the normal, highlighted, and reset colors (in that order)
+    for old/new lines. If unset, values will be retrieved by calling `git
+    config` (see "Color Config" above). Note that these should be the literal
+    color bytes (starting with an ANSI escape code), not color names.
+
 The script may then feed lines, one at a time, to DiffHighlight::handle_line().
 When lines are done processing, they will be fed to $line_cb. Note that
 DiffHighlight may queue up many input lines (to analyze a whole hunk)

-- 
2.53.0.945.ge67b727e8d
Jeff KingMar 20, 2026, 00:48 UTC in reply to Jeff King on lore

[PATCH 8/8] diff-highlight: fetch all config with one process

When diff-highlight was written, there was no way to fetch multiple config keys _and_ have them interpreted as colors. So we were stuck with either invoking git-config once for each config key, or fetching them all and converting human-readable color names into ANSI codes ourselves.

I chose the former, but it means that diff-highlight kicks off 6 git-config processes (even if you haven't configured anything, it has to check each one).

But since Git 2.18.0, we can do:
   git config --type=color --get-regexp=^color\.diff-highlight\.
to get all of them in one shot.

Note that any callers which pass in colors directly to the module via @OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are unaffected; those colors suppress any config lookup we'd do ourselves.

You can see the effect like:
  # diff-highlight suppresses git-config's stderr, so dump
  # trace through descriptor 3
  git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null
which drops from 6 lines down to 1.
Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 26 ++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
Show changes to contrib/diff-highlight/DiffHighlight.pm +19 −7
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index 96369eadf9..a22ba7a851 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -131,8 +131,20 @@ sub highlight_stdin {
 # of it being used in other settings. Let's handle our own
 # fallback, which means we will work even if git can't be run.
 sub color_config {
+	our $cached_config;
 	my ($key, $default) = @_;
-	my $s = `git config --get-color $key 2>$NULL`;
+
+	if (!defined $cached_config) {
+		$cached_config = {};
+		my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`;
+		for my $line (split /\n/, $data) {
+			my ($key, $color) = split ' ', $line, 2;
+			$key =~ s/^color\.diff-highlight\.// or next;
+			$cached_config->{$key} = $color;
+		}
+	}
+
+	my $s = $cached_config->{$key};
 	return length($s) ? $s : $default;
 }
 
@@ -172,16 +184,16 @@ sub load_color_config {
 	# always be set if you want highlighting to do anything.
 	if (!defined $OLD_HIGHLIGHT[1]) {
 		@OLD_HIGHLIGHT = (
-			color_config('color.diff-highlight.oldnormal'),
-			color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
-			color_config('color.diff-highlight.oldreset', "\x1b[27m")
+			color_config('oldnormal'),
+			color_config('oldhighlight', "\x1b[7m"),
+			color_config('oldreset', "\x1b[27m")
 		);
 	}
 	if (!defined $NEW_HIGHLIGHT[1]) {
 		@NEW_HIGHLIGHT = (
-			color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
-			color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
-			color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
+			color_config('newnormal', $OLD_HIGHLIGHT[0]),
+			color_config('newhighlight', $OLD_HIGHLIGHT[1]),
+			color_config('newreset', $OLD_HIGHLIGHT[2])
 		);
 	};
 }

-- 
2.53.0.945.ge67b727e8d
Tian YuchenMar 22, 2026, 17:18 UTC in reply to Jeff King on lore

Re: [PATCH 8/8] diff-highlight: fetch all config with one process

Hi Jeff,
On 3/20/26 08:48, Jeff King wrote:
Show 43 quoted lines
> When diff-highlight was written, there was no way to fetch multiple
> config keys _and_ have them interpreted as colors. So we were stuck
> with either invoking git-config once for each config key, or fetching
> them all and converting human-readable color names into ANSI codes
> ourselves.
> 
> I chose the former, but it means that diff-highlight kicks off 6
> git-config processes (even if you haven't configured anything, it has to
> check each one).
> 
> But since Git 2.18.0, we can do:
> 
>     git config --type=color --get-regexp=^color\.diff-highlight\.
> 
> to get all of them in one shot.
> 
> Note that any callers which pass in colors directly to the module via
> @OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are
> unaffected; those colors suppress any config lookup we'd do ourselves.
> 
> You can see the effect like:
> 
>    # diff-highlight suppresses git-config's stderr, so dump
>    # trace through descriptor 3
>    git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null
> 
> which drops from 6 lines down to 1.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>   contrib/diff-highlight/DiffHighlight.pm | 26 ++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
> index 96369eadf9..a22ba7a851 100644
> --- a/contrib/diff-highlight/DiffHighlight.pm
> +++ b/contrib/diff-highlight/DiffHighlight.pm
> @@ -131,8 +131,20 @@ sub highlight_stdin {
>   # of it being used in other settings. Let's handle our own
>   # fallback, which means we will work even if git can't be run.
>   sub color_config {
> +	our $cached_config;
>   	my ($key, $default) = @_;
$key...
Show 7 quoted lines
> -	my $s = `git config --get-color $key 2>$NULL`;
> +
> +	if (!defined $cached_config) {
> +		$cached_config = {};
> +		my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`;
> +		for my $line (split /\n/, $data) {
> +			my ($key, $color) = split ' ', $line, 2;

...another $key. I think it would be better to change the name here. What do you think?

Show 5 quoted lines
> +			$key =~ s/^color\.diff-highlight\.// or next;
> +			$cached_config->{$key} = $color;
> +		}
> +	}
> +
...
> +	my $s = $cached_config->{$key};
>   	return length($s) ? $s : $default;
>   }
>  
Something doesn't feel quite right here.

If the user has not configured color.diff-highlight.*, the expression git config --type=color --get-regexp=^color\.diff-highlight\. will not find a match and should not output anything. In this case, %cached_config->{$key} becomes undef, length() returns 0, and a warning is issued.

But we have "use warnings FATAL => 'all'". This situation will result in a fatal error, which I don't think is what we want.

Show 23 quoted lines
> @@ -172,16 +184,16 @@ sub load_color_config {
>   	# always be set if you want highlighting to do anything.
>   	if (!defined $OLD_HIGHLIGHT[1]) {
>   		@OLD_HIGHLIGHT = (
> -			color_config('color.diff-highlight.oldnormal'),
> -			color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
> -			color_config('color.diff-highlight.oldreset', "\x1b[27m")
> +			color_config('oldnormal'),
> +			color_config('oldhighlight', "\x1b[7m"),
> +			color_config('oldreset', "\x1b[27m")
>   		);
>   	}
>   	if (!defined $NEW_HIGHLIGHT[1]) {
>   		@NEW_HIGHLIGHT = (
> -			color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
> -			color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
> -			color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
> +			color_config('newnormal', $OLD_HIGHLIGHT[0]),
> +			color_config('newhighlight', $OLD_HIGHLIGHT[1]),
> +			color_config('newreset', $OLD_HIGHLIGHT[2])
>   		);
>   	};
>   }
Regards,
Yuchen
Tian YuchenMar 22, 2026, 17:24 UTC in reply to Jeff King on lore

Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests

On 3/20/26 08:44, Jeff King wrote:
Show 34 quoted lines
> The diff-highlight tests use raw color bytes when comparing expected and
> actual output. Let's use test_decode_color, which is our usual technique
> in other tests. It makes reading test output diffs a bit easier, since
> you're not relying on your terminal to interpret the result (or worse,
> interpreting characters yourself via "cat -A").
> 
> This will also make it easier to add tests with new colors/attributes,
> without having to pre-define the byte sequences ourselves.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>   .../diff-highlight/t/t9400-diff-highlight.sh  | 37 +++++++++----------
>   1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> index 42d331c6cd..ba80cda7c8 100755
> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)
>   TEST_DIRECTORY="$CURR_DIR"/../../../t
>   DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
>   
> -CW="$(printf "\033[7m")"	# white
> -CR="$(printf "\033[27m")"	# reset
> -
>   GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
>   export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>   . "$TEST_DIRECTORY"/test-lib.sh
> @@ -42,9 +39,9 @@ dh_test () {
>   	} >/dev/null &&
>   
>   	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
> -	test_strip_patch_header <diff.hi >diff.act
> +	test_strip_patch_header <diff.hi | test_decode_color >diff.act

Although this is just simple text filtering and leaving it as is wouldn’t cause any problems IMO, why not go ahead and add the && while you’re at it?

I've noticed that there are several missing &&.
Show 88 quoted lines
>   	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
> -	test_strip_patch_header <commit.hi >commit.act &&
> +	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
>   	test_cmp patch.exp diff.act &&
>   	test_cmp patch.exp commit.act
>   }
> @@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-${CW}b${CR}bb
> -		+${CW}0${CR}bb
> +		-<REVERSE>b<NOREVERSE>bb
> +		+<REVERSE>0<NOREVERSE>bb
>   		 ccc
>   	EOF
>   '
> @@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-bb${CW}b${CR}
> -		+bb${CW}0${CR}
> +		-bb<REVERSE>b<NOREVERSE>
> +		+bb<REVERSE>0<NOREVERSE>
>   		 ccc
>   	EOF
>   '
> @@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-b${CW}b${CR}b
> -		+b${CW}0${CR}b
> +		-b<REVERSE>b<NOREVERSE>b
> +		+b<REVERSE>0<NOREVERSE>b
>   		 ccc
>   	EOF
>   '
> @@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-b${CW}b${CR}b
> -		+b${CW}0${CR}b
> +		-b<REVERSE>b<NOREVERSE>b
> +		+b<REVERSE>0<NOREVERSE>b
>   		+ccc
>   	EOF
>   '
> @@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '
>   	echo "unic${o_stroke}de" >b &&
>   	dh_test a b <<-EOF
>   		@@ -1 +1 @@
> -		-unic${CW}${o_accent}${CR}de
> -		+unic${CW}${o_stroke}${CR}de
> +		-unic<REVERSE>${o_accent}<NOREVERSE>de
> +		+unic<REVERSE>${o_stroke}<NOREVERSE>de
>   	EOF
>   '
>   
> @@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '
>   	echo "unico${combine_circum}de" >b &&
>   	dh_test a b <<-EOF
>   		@@ -1 +1 @@
> -		-unic${CW}o${combine_accent}${CR}de
> -		+unic${CW}o${combine_circum}${CR}de
> +		-unic<REVERSE>o${combine_accent}<NOREVERSE>de
> +		+unic<REVERSE>o${combine_circum}<NOREVERSE>de
>   	EOF
>   '
>   
> @@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '
>   	+++ b/file
>   	@@ -1,3 +1,3 @@
>   	 before
> -	-the ${CW}old${CR} line
> -	+the ${CW}new${CR} line
> +	-the <REVERSE>old<NOREVERSE> line
> +	+the <REVERSE>new<NOREVERSE> line
>   	 -leading dash
>   	EOF
>   	git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw &&
> -	trim_graph <actual.raw | sed -n "/^---/,\$p" >actual &&
> +	trim_graph <actual.raw | sed -n "/^---/,\$p" | test_decode_color >actual &&
>   	test_cmp expect actual
>   '
>   
Thanks,
Yuchen
Jeff KingMar 22, 2026, 20:45 UTC in reply to Tian Yuchen on lore

Re: [PATCH 8/8] diff-highlight: fetch all config with one process

On Mon, Mar 23, 2026 at 01:18:30AM +0800, Tian Yuchen wrote:
Show 10 quoted lines
> > -	my $s = `git config --get-color $key 2>$NULL`;
> > +
> > +	if (!defined $cached_config) {
> > +		$cached_config = {};
> > +		my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`;
> > +		for my $line (split /\n/, $data) {
> > +			my ($key, $color) = split ' ', $line, 2;
> 
> ...another $key. I think it would be better to change the name here. What do
> you think?

I noticed it, too, but didn't have a better name (in fact they are of the same type, just two different contexts). Shadowing seemed less bad to me than using a mis-matched name.

Show 10 quoted lines
> > +	my $s = $cached_config->{$key};
> >   	return length($s) ? $s : $default;
> >   }
> 
> Something doesn't feel quite right here.
> 
> If the user has not configured color.diff-highlight.*, the expression git
> config --type=color --get-regexp=^color\.diff-highlight\. will not find a
> match and should not output anything. In this case, %cached_config->{$key}
> becomes undef, length() returns 0, and a warning is issued.

The length() of undef is also undef (and documented in "perldoc -f length"). But either way, length($s) will be false, and we will return $default, not $s.

I don't get any warning on perl 5.40.1. Are you seeing one on a different version?

-Peff
Jeff KingMar 22, 2026, 20:47 UTC in reply to Tian Yuchen on lore

Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests

On Mon, Mar 23, 2026 at 01:24:00AM +0800, Tian Yuchen wrote:
Show 8 quoted lines
> > @@ -42,9 +39,9 @@ dh_test () {
> >   	} >/dev/null &&
> >   	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
> > -	test_strip_patch_header <diff.hi >diff.act
> > +	test_strip_patch_header <diff.hi | test_decode_color >diff.act
> 
> Although this is just simple text filtering and leaving it as is wouldn’t
> cause any problems IMO, why not go ahead and add the && while you’re at it?

The bug is in an earlier commit (patch 3), which breaks apart the pipe but doesn't add the necessary &&. And it's more than just text filtering; it breaks the &&-chain, so we miss the exit code of $DIFF_HIGHLIGHT (which was the whole point of patch 3).

chainlint doesn't find it because we're in a helper function, not directinly in a test snippet.

I'll send a revised series to fix it, but...
> I've noticed that there are several missing &&.
Where else do you see?

Or do you mean that we should not pipe text filtering commands? There I'd disagree. We are not likely to see a failure from 'sed', and if we do, the fact that the output does not match would catch it. And the cost of breaking every command down without pipes means having to manage lots of intermediate files.

-Peff
Tian YuchenMar 23, 2026, 05:39 UTC in reply to Jeff King on lore

Re: [PATCH 8/8] diff-highlight: fetch all config with one process

On 3/23/26 04:45, Jeff King wrote:
> I noticed it, too, but didn't have a better name (in fact they are of
> the same type, just two different contexts). Shadowing seemed less bad
> to me than using a mis-matched name. 
Fair enough! Let's leave it as it is ;)
Show 6 quoted lines
> The length() of undef is also undef (and documented in "perldoc -f
> length"). But either way, length($s) will be false, and we will return
> $default, not $s.
> 
> I don't get any warning on perl 5.40.1. Are you seeing one on a
> different version?
It's mainly because I saw that you changed the required version earlier：

-require v5.26; +require v5.008;

I clearly remember that in older versions of Perl, the length function behaved differently than it does now.

	use strict;
	use warnings FATAL => 'uninitialized';
	my $x;
	print "length = ", length($x), "\n";
In 5.8.8 the output is:
	Use of uninitialized value in length at test.pl line 5.
And in 5.38.2 the output is:
	Use of uninitialized value in print at test.pl line 5. length =
	
In the first case, the code will throw an error and exit. Although I 
haven't compiled this patch under version 5.8.8 yet, I suspect there 
will be issues.
> 
> -Peff
Regards, Yuchen
Tian YuchenMar 23, 2026, 05:48 UTC in reply to Jeff King on lore

Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests

On 3/23/26 04:47, Jeff King wrote:
Show 32 quoted lines
> On Mon, Mar 23, 2026 at 01:24:00AM +0800, Tian Yuchen wrote:
> 
>>> @@ -42,9 +39,9 @@ dh_test () {
>>>    	} >/dev/null &&
>>>    	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
>>> -	test_strip_patch_header <diff.hi >diff.act
>>> +	test_strip_patch_header <diff.hi | test_decode_color >diff.act
>>
>> Although this is just simple text filtering and leaving it as is wouldn’t
>> cause any problems IMO, why not go ahead and add the && while you’re at it?
> 
> The bug is in an earlier commit (patch 3), which breaks apart the pipe
> but doesn't add the necessary &&. And it's more than just text
> filtering; it breaks the &&-chain, so we miss the exit code of
> $DIFF_HIGHLIGHT (which was the whole point of patch 3).
> 
> chainlint doesn't find it because we're in a helper function, not
> directinly in a test snippet.
> 
> I'll send a revised series to fix it, but...
> 
>> I've noticed that there are several missing &&.
> 
> Where else do you see?
> 
> Or do you mean that we should not pipe text filtering commands? There
> I'd disagree. We are not likely to see a failure from 'sed', and if we
> do, the fact that the output does not match would catch it. And the cost
> of breaking every command down without pipes means having to manage lots
> of intermediate files.
> 
> -Peff
Oh, I see.

To be honest, I didn't think about it in that much detail. I just noticed this tiny issue and wanted to take the opportunity to remind you to check other parts as well. I'm not saying we should remove those pipe commands :P

Thanks for the explanation about chainlint.
Regards, Yuchen
Jeff KingMar 23, 2026, 05:53 UTC in reply to Tian Yuchen on lore

Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests

On Mon, Mar 23, 2026 at 01:48:49PM +0800, Tian Yuchen wrote:
> To be honest, I didn't think about it in that much detail. I just noticed
> this tiny issue and wanted to take the opportunity to remind you to check
> other parts as well. I'm not saying we should remove those pipe commands :P

Ah, OK. I think that is the only spot, then. Thanks for pointing it out! Without the fix it was missing the whole point of patch 3. ;)

-Peff
Jeff KingMar 23, 2026, 05:57 UTC in reply to Tian Yuchen on lore

Re: [PATCH 8/8] diff-highlight: fetch all config with one process

On Mon, Mar 23, 2026 at 01:39:44PM +0800, Tian Yuchen wrote:
Show 10 quoted lines
> > I don't get any warning on perl 5.40.1. Are you seeing one on a
> > different version?
> 
> It's mainly because I saw that you changed the required version earlier：
> 
> -require v5.26;
> +require v5.008;
> 
> I clearly remember that in older versions of Perl, the length function
> behaved differently than it does now.

Heh, it figures that dropping back the version requirement would bite me immediately. ;)

> In 5.8.8 the output is:
> 
> 	Use of uninitialized value in length at test.pl line 5.
Yeah, looks like it changed in 5.12:
  https://www.effectiveperlprogramming.com/2010/09/in-perl-v5-12-lengthundef-returns-undef/

I had actually written it using exists() originally, but then dropped it to keep the diff smaller. Which is a silly reason. I've switched it to use:

  return defined($s) ? $s : $default;

which I think captures the intent pretty clearly. You could also use "$s || $default", but I try to avoid that because of surprise-false values. In modern perl you could just use "//", but that wasn't added until v5.10.

Thanks for pointing it out!
-Peff
Jeff KingMar 23, 2026, 06:01 UTC in reply to Jeff King on lore

[PATCH v2 0/8] some diff-highlight tweaks

Here's a re-roll based on the review from Yuchen. The two changes are:
  1. Added a missing &&-chain in patch 3 (which cascades into patch 6).
  2. Avoid length(undef), since old perl versions will warn about it.
Patch list and range diff below.
 1:  c59dd0aac9 =  1:  c59dd0aac9 contrib/diff-highlight: do not highlight identical pairs
 2:  16aa04fc6d =  2:  16aa04fc6d diff-highlight: mention build instructions
 3:  55788fac3a =  3:  55788fac3a diff-highlight: drop perl version dependency back to 5.8
 4:  7c2af2348b !  4:  1101c94f65 diff-highlight: check diff-highlight exit status in tests
    @@ contrib/diff-highlight/t/t9400-diff-highlight.sh: dh_test () {
     -	"$DIFF_HIGHLIGHT" <diff.raw | test_strip_patch_header >diff.act &&
     -	"$DIFF_HIGHLIGHT" <commit.raw | test_strip_patch_header >commit.act &&
     +	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
    -+	test_strip_patch_header <diff.hi >diff.act
    ++	test_strip_patch_header <diff.hi >diff.act &&
     +	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
     +	test_strip_patch_header <commit.hi >commit.act &&
      	test_cmp patch.exp diff.act &&
 5:  52f0358329 =  5:  65420b8b79 t: add matching negative attributes to test_decode_color
 6:  0f1aacf264 !  6:  60977c32f6 diff-highlight: use test_decode_color in tests
    @@ contrib/diff-highlight/t/t9400-diff-highlight.sh: dh_test () {
      	} >/dev/null &&
      
      	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
    --	test_strip_patch_header <diff.hi >diff.act
    -+	test_strip_patch_header <diff.hi | test_decode_color >diff.act
    +-	test_strip_patch_header <diff.hi >diff.act &&
    ++	test_strip_patch_header <diff.hi | test_decode_color >diff.act &&
      	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
     -	test_strip_patch_header <commit.hi >commit.act &&
     +	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
 7:  8bad893f09 =  7:  bf33329640 diff-highlight: test color config
 8:  179f83d791 =  8:  ea71a8b648 diff-highlight: allow module callers to pass in color config
 9:  b8ff37b193 !  9:  aab7912ca2 diff-highlight: fetch all config with one process
    @@ contrib/diff-highlight/DiffHighlight.pm: sub highlight_stdin {
     +	our $cached_config;
      	my ($key, $default) = @_;
     -	my $s = `git config --get-color $key 2>$NULL`;
    +-	return length($s) ? $s : $default;
     +
     +	if (!defined $cached_config) {
     +		$cached_config = {};
    @@ contrib/diff-highlight/DiffHighlight.pm: sub highlight_stdin {
     +	}
     +
     +	my $s = $cached_config->{$key};
    - 	return length($s) ? $s : $default;
    ++	return defined($s) ? $s : $default;
      }
      
    + sub show_hunk {
     @@ contrib/diff-highlight/DiffHighlight.pm: sub load_color_config {
      	# always be set if you want highlighting to do anything.
      	if (!defined $OLD_HIGHLIGHT[1]) {
  [1/8]: diff-highlight: mention build instructions
  [2/8]: diff-highlight: drop perl version dependency back to 5.8
  [3/8]: diff-highlight: check diff-highlight exit status in tests
  [4/8]: t: add matching negative attributes to test_decode_color
  [5/8]: diff-highlight: use test_decode_color in tests
  [6/8]: diff-highlight: test color config
  [7/8]: diff-highlight: allow module callers to pass in color config
  [8/8]: diff-highlight: fetch all config with one process
 contrib/diff-highlight/DiffHighlight.pm       | 59 +++++++++++-----
 contrib/diff-highlight/README                 | 19 +++++-
 .../diff-highlight/t/t9400-diff-highlight.sh  | 67 +++++++++++++------
 t/test-lib-functions.sh                       |  3 +
 4 files changed, 112 insertions(+), 36 deletions(-)
Jeff KingMar 23, 2026, 06:01 UTC in reply to Jeff King on lore

[PATCH v2 1/8] diff-highlight: mention build instructions

Once upon a time, this was just a script in a directory that could be run directly. That changed in 0c977dbc81 (diff-highlight: split code into module, 2017-06-15). Let's update the README to make it more clear that you need to run make.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/README | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
Show changes to contrib/diff-highlight/README +12 −1
diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 1db4440e68..9c89146fb0 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -39,10 +39,21 @@ visually distracting.  Non-diff lines and existing diff coloration is
 preserved; the intent is that the output should look exactly the same as
 the input, except for the occasional highlight.
 
+Build/Install
+-------------
+
+You can build the `diff-highlight` script by running `make` from within
+the diff-highlight directory. There is no `make install` target; you can
+copy the built script to your $PATH.
+
+You can run diff-highlight's internal tests by running `make test`. Note
+that you must also build Git itself first (by running `make` from the
+top-level of the project).
+
 Use
 ---
 
-You can try out the diff-highlight program with:
+You can try out the built diff-highlight program with:
 
 ---------------------------------------------
 git log -p --color | /path/to/diff-highlight

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 2/8] diff-highlight: drop perl version dependency back to 5.8

From: Scott Baker <scott@perturb.org>

The diff-highlight code does not rely on any perl features beyond what perl 5.8 provides. We bumped it to v5.26 along with the rest of the project's perl scripts in 702d8c1f3b (Require Perl 5.26.0, 2024-10-23).

There's some value in just having a uniform baseline for the project, but I think diff-highlight is special here:

  - it's in a contrib/ directory that is not frequently touched, so
    there is little risk of Git developers getting annoyed that modern
    perl features are not available
  - it provides a module used by other projects. In particular,
    diff-so-fancy relies on DiffHighlight.pm but does not otherwise
    require a perl version more modern than 5.8.
Let's drop back to the more conservative requirement.
Signed-off-by: Scott Baker <scott@perturb.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/diff-highlight/DiffHighlight.pm +1 −1
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index f0607a4b68..a5e5de3b18 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -1,6 +1,6 @@
 package DiffHighlight;
 
-require v5.26;
+require v5.008;
 use warnings FATAL => 'all';
 use strict;
 

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 3/8] diff-highlight: check diff-highlight exit status in tests

When testing diff-highlight, we pipe the output through a sanitizing function. This loses the exit status of diff-highlight itself, which could mean we are missing cases where it crashes or exits unexpectedly. Use an extra tempfile to avoid the pipe.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +4 −2
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 2a9b68cf3b..7ebff8b18f 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -41,8 +41,10 @@ dh_test () {
 		git show >commit.raw
 	} >/dev/null &&
 
-	"$DIFF_HIGHLIGHT" <diff.raw | test_strip_patch_header >diff.act &&
-	"$DIFF_HIGHLIGHT" <commit.raw | test_strip_patch_header >commit.act &&
+	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
+	test_strip_patch_header <diff.hi >diff.act &&
+	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
+	test_strip_patch_header <commit.hi >commit.act &&
 	test_cmp patch.exp diff.act &&
 	test_cmp patch.exp commit.act
 }

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 4/8] t: add matching negative attributes to test_decode_color

Most of the ANSI color attributes have an "off" variant. We don't use these yet in our test suite, so we never bothered to decode them. Add the ones that match the attributes we encode so we can make use of them.

There are even more attributes not covered on the positive side, so this is meant to be useful but not all-inclusive.

Note that "nobold" and "nodim" are the same code, so I've decoded this as "normal intensity".

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib-functions.sh | 3 +++
 1 file changed, 3 insertions(+)
Show changes to t/test-lib-functions.sh +3 −0
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 14e238d24d..f3af10fb7e 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -48,6 +48,9 @@ test_decode_color () {
 			if (n == 2) return "FAINT";
 			if (n == 3) return "ITALIC";
 			if (n == 7) return "REVERSE";
+			if (n == 22) return "NORMAL_INTENSITY";
+			if (n == 23) return "NOITALIC";
+			if (n == 27) return "NOREVERSE";
 			if (n == 30) return "BLACK";
 			if (n == 31) return "RED";
 			if (n == 32) return "GREEN";

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 5/8] diff-highlight: use test_decode_color in tests

The diff-highlight tests use raw color bytes when comparing expected and actual output. Let's use test_decode_color, which is our usual technique in other tests. It makes reading test output diffs a bit easier, since you're not relying on your terminal to interpret the result (or worse, interpreting characters yourself via "cat -A").

This will also make it easier to add tests with new colors/attributes, without having to pre-define the byte sequences ourselves.

Signed-off-by: Jeff King <peff@peff.net>
---
 .../diff-highlight/t/t9400-diff-highlight.sh  | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +17 −20
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 7ebff8b18f..4f3d55a26e 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)
 TEST_DIRECTORY="$CURR_DIR"/../../../t
 DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
 
-CW="$(printf "\033[7m")"	# white
-CR="$(printf "\033[27m")"	# reset
-
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . "$TEST_DIRECTORY"/test-lib.sh
@@ -42,9 +39,9 @@ dh_test () {
 	} >/dev/null &&
 
 	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
-	test_strip_patch_header <diff.hi >diff.act &&
+	test_strip_patch_header <diff.hi | test_decode_color >diff.act &&
 	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
-	test_strip_patch_header <commit.hi >commit.act &&
+	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
 	test_cmp patch.exp diff.act &&
 	test_cmp patch.exp commit.act
 }
@@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-${CW}b${CR}bb
-		+${CW}0${CR}bb
+		-<REVERSE>b<NOREVERSE>bb
+		+<REVERSE>0<NOREVERSE>bb
 		 ccc
 	EOF
 '
@@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-bb${CW}b${CR}
-		+bb${CW}0${CR}
+		-bb<REVERSE>b<NOREVERSE>
+		+bb<REVERSE>0<NOREVERSE>
 		 ccc
 	EOF
 '
@@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-b${CW}b${CR}b
-		+b${CW}0${CR}b
+		-b<REVERSE>b<NOREVERSE>b
+		+b<REVERSE>0<NOREVERSE>b
 		 ccc
 	EOF
 '
@@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '
 	dh_test a b <<-EOF
 		@@ -1,3 +1,3 @@
 		 aaa
-		-b${CW}b${CR}b
-		+b${CW}0${CR}b
+		-b<REVERSE>b<NOREVERSE>b
+		+b<REVERSE>0<NOREVERSE>b
 		+ccc
 	EOF
 '
@@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '
 	echo "unic${o_stroke}de" >b &&
 	dh_test a b <<-EOF
 		@@ -1 +1 @@
-		-unic${CW}${o_accent}${CR}de
-		+unic${CW}${o_stroke}${CR}de
+		-unic<REVERSE>${o_accent}<NOREVERSE>de
+		+unic<REVERSE>${o_stroke}<NOREVERSE>de
 	EOF
 '
 
@@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '
 	echo "unico${combine_circum}de" >b &&
 	dh_test a b <<-EOF
 		@@ -1 +1 @@
-		-unic${CW}o${combine_accent}${CR}de
-		+unic${CW}o${combine_circum}${CR}de
+		-unic<REVERSE>o${combine_accent}<NOREVERSE>de
+		+unic<REVERSE>o${combine_circum}<NOREVERSE>de
 	EOF
 '
 
@@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '
 	+++ b/file
 	@@ -1,3 +1,3 @@
 	 before
-	-the ${CW}old${CR} line
-	+the ${CW}new${CR} line
+	-the <REVERSE>old<NOREVERSE> line
+	+the <REVERSE>new<NOREVERSE> line
 	 -leading dash
 	EOF
 	git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw &&
-	trim_graph <actual.raw | sed -n "/^---/,\$p" >actual &&
+	trim_graph <actual.raw | sed -n "/^---/,\$p" | test_decode_color >actual &&
 	test_cmp expect actual
 '
 

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 6/8] diff-highlight: test color config

We added configurable colors long ago in bca45fbc1f (diff-highlight: allow configurable colors, 2014-11-20), but never actually tested it. Since we'll be touching the color code in a moment, this is a good time to beef up the tests.

Note that we cover both the highlight/reset style used by the default colors, as well as the normal/highlight style added by that commit (which was previously totally untested).

Signed-off-by: Jeff King <peff@peff.net>
---
 .../diff-highlight/t/t9400-diff-highlight.sh  | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
Show changes to contrib/diff-highlight/t/t9400-diff-highlight.sh +28 −0
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 4f3d55a26e..b38fe2196a 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -350,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' '
 	EOF
 '
 
+test_expect_success 'configure set/reset colors' '
+	test_config color.diff-highlight.oldhighlight bold &&
+	test_config color.diff-highlight.oldreset nobold &&
+	test_config color.diff-highlight.newhighlight italic &&
+	test_config color.diff-highlight.newreset noitalic &&
+	echo "prefix a suffix" >a &&
+	echo "prefix b suffix" >b &&
+	dh_test a b <<-\EOF
+	@@ -1 +1 @@
+	-prefix <BOLD>a<NORMAL_INTENSITY> suffix
+	+prefix <ITALIC>b<NOITALIC> suffix
+	EOF
+'
+
+test_expect_success 'configure normal/highlight colors' '
+	test_config color.diff-highlight.oldnormal red &&
+	test_config color.diff-highlight.oldhighlight magenta &&
+	test_config color.diff-highlight.newnormal green &&
+	test_config color.diff-highlight.newhighlight yellow &&
+	echo "prefix a suffix" >a &&
+	echo "prefix b suffix" >b &&
+	dh_test a b <<-\EOF
+	@@ -1 +1 @@
+	<RED>-prefix <RESET><MAGENTA>a<RESET><RED> suffix<RESET>
+	<GREEN>+prefix <RESET><YELLOW>b<RESET><GREEN> suffix<RESET>
+	EOF
+'
+
 test_done

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 7/8] diff-highlight: allow module callers to pass in color config

From: Scott Baker <scott@perturb.org>

Users of the module may want to pass in their own color config for a few obvious reasons:

  - they are pulling the config from different variables than
    diff-highlight itself uses
  - they are loading the config in a more efficient way (say, by parsing
    git-config --list) and don't want to incur the six (!) git-config
    calls that DiffHighlight.pm runs to check all config

Let's allow users of the module to pass in the color config, and lazy-load it when needed if they haven't.

Signed-off-by: Scott Baker <scott@perturb.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 41 +++++++++++++++++--------
 contrib/diff-highlight/README           |  6 ++++
 2 files changed, 35 insertions(+), 12 deletions(-)
Show changes to 2 files +35 −12

contrib/diff-highlight/DiffHighlight.pm, contrib/diff-highlight/README

diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index a5e5de3b18..96369eadf9 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -9,18 +9,11 @@ package DiffHighlight;
 
 my $NULL = File::Spec->devnull();
 
-# Highlight by reversing foreground and background. You could do
-# other things like bold or underline if you prefer.
-my @OLD_HIGHLIGHT = (
-	color_config('color.diff-highlight.oldnormal'),
-	color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
-	color_config('color.diff-highlight.oldreset', "\x1b[27m")
-);
-my @NEW_HIGHLIGHT = (
-	color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
-	color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
-	color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
-);
+# The color theme is initially set to nothing here to allow outside callers
+# to set the colors for their application. If nothing is sent in we use
+# colors from git config in load_color_config().
+our @OLD_HIGHLIGHT = ();
+our @NEW_HIGHLIGHT = ();
 
 my $RESET = "\x1b[m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
@@ -170,6 +163,29 @@ sub show_hunk {
 	$line_cb->(@queue);
 }
 
+sub load_color_config {
+	# If the colors were NOT set from outside this module we load them on-demand
+	# from the git config. Note that only one of elements 0 and 2 in each
+	# array is used (depending on whether you are doing set/unset on an
+	# attribute, or specifying normal vs highlighted coloring). So we use
+	# element 1 as our check for whether colors were passed in; it should
+	# always be set if you want highlighting to do anything.
+	if (!defined $OLD_HIGHLIGHT[1]) {
+		@OLD_HIGHLIGHT = (
+			color_config('color.diff-highlight.oldnormal'),
+			color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
+			color_config('color.diff-highlight.oldreset', "\x1b[27m")
+		);
+	}
+	if (!defined $NEW_HIGHLIGHT[1]) {
+		@NEW_HIGHLIGHT = (
+			color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
+			color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
+			color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
+		);
+	};
+}
+
 sub highlight_pair {
 	my @a = split_line(shift);
 	my @b = split_line(shift);
@@ -218,6 +234,7 @@ sub highlight_pair {
 	}
 
 	if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) {
+		load_color_config();
 		return highlight_line(\@a, $pa, $sa, \@OLD_HIGHLIGHT),
 		       highlight_line(\@b, $pb, $sb, \@NEW_HIGHLIGHT);
 	}
diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 9c89146fb0..ed8d876a18 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -138,6 +138,12 @@ Your script may set up one or more of the following variables:
     processing a logical chunk of input). The default function flushes
     stdout.
 
+  - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these
+    arrays specify the normal, highlighted, and reset colors (in that order)
+    for old/new lines. If unset, values will be retrieved by calling `git
+    config` (see "Color Config" above). Note that these should be the literal
+    color bytes (starting with an ANSI escape code), not color names.
+
 The script may then feed lines, one at a time, to DiffHighlight::handle_line().
 When lines are done processing, they will be fed to $line_cb. Note that
 DiffHighlight may queue up many input lines (to analyze a whole hunk)

-- 
2.53.0.1051.ga14e96f895
Jeff KingMar 23, 2026, 06:02 UTC in reply to Jeff King on lore

[PATCH v2 8/8] diff-highlight: fetch all config with one process

When diff-highlight was written, there was no way to fetch multiple config keys _and_ have them interpreted as colors. So we were stuck with either invoking git-config once for each config key, or fetching them all and converting human-readable color names into ANSI codes ourselves.

I chose the former, but it means that diff-highlight kicks off 6 git-config processes (even if you haven't configured anything, it has to check each one).

But since Git 2.18.0, we can do:
   git config --type=color --get-regexp=^color\.diff-highlight\.
to get all of them in one shot.

Note that any callers which pass in colors directly to the module via @OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are unaffected; those colors suppress any config lookup we'd do ourselves.

You can see the effect like:
  # diff-highlight suppresses git-config's stderr, so dump
  # trace through descriptor 3
  git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null
which drops from 6 lines down to 1.
Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/diff-highlight/DiffHighlight.pm | 28 ++++++++++++++++++-------
 1 file changed, 20 insertions(+), 8 deletions(-)
Show changes to contrib/diff-highlight/DiffHighlight.pm +20 −8
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index 96369eadf9..abe457882e 100644
--- a/contrib/diff-highlight/DiffHighlight.pm
+++ b/contrib/diff-highlight/DiffHighlight.pm
@@ -131,9 +131,21 @@ sub highlight_stdin {
 # of it being used in other settings. Let's handle our own
 # fallback, which means we will work even if git can't be run.
 sub color_config {
+	our $cached_config;
 	my ($key, $default) = @_;
-	my $s = `git config --get-color $key 2>$NULL`;
-	return length($s) ? $s : $default;
+
+	if (!defined $cached_config) {
+		$cached_config = {};
+		my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`;
+		for my $line (split /\n/, $data) {
+			my ($key, $color) = split ' ', $line, 2;
+			$key =~ s/^color\.diff-highlight\.// or next;
+			$cached_config->{$key} = $color;
+		}
+	}
+
+	my $s = $cached_config->{$key};
+	return defined($s) ? $s : $default;
 }
 
 sub show_hunk {
@@ -172,16 +184,16 @@ sub load_color_config {
 	# always be set if you want highlighting to do anything.
 	if (!defined $OLD_HIGHLIGHT[1]) {
 		@OLD_HIGHLIGHT = (
-			color_config('color.diff-highlight.oldnormal'),
-			color_config('color.diff-highlight.oldhighlight', "\x1b[7m"),
-			color_config('color.diff-highlight.oldreset', "\x1b[27m")
+			color_config('oldnormal'),
+			color_config('oldhighlight', "\x1b[7m"),
+			color_config('oldreset', "\x1b[27m")
 		);
 	}
 	if (!defined $NEW_HIGHLIGHT[1]) {
 		@NEW_HIGHLIGHT = (
-			color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),
-			color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),
-			color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])
+			color_config('newnormal', $OLD_HIGHLIGHT[0]),
+			color_config('newhighlight', $OLD_HIGHLIGHT[1]),
+			color_config('newreset', $OLD_HIGHLIGHT[2])
 		);
 	};
 }

-- 
2.53.0.1051.ga14e96f895
Junio C HamanoMar 23, 2026, 16:38 UTC in reply to Jeff King on lore

Re: [PATCH v2 0/8] some diff-highlight tweaks

Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> Here's a re-roll based on the review from Yuchen. The two changes are:
>
>   1. Added a missing &&-chain in patch 3 (which cascades into patch 6).
>
>   2. Avoid length(undef), since old perl versions will warn about it.
>
> Patch list and range diff below.

Everything looks as expected from watching the discussion from the sideline. Looking good.

Will queue and mark the topic for 'next'.
Thanks.
Patrick SteinhardtMar 24, 2026, 06:50 UTC in reply to Junio C Hamano on lore

Re: [PATCH v2 0/8] some diff-highlight tweaks

On Mon, Mar 23, 2026 at 09:38:56AM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > Here's a re-roll based on the review from Yuchen. The two changes are:
> >
> >   1. Added a missing &&-chain in patch 3 (which cascades into patch 6).
> >
> >   2. Avoid length(undef), since old perl versions will warn about it.
> >
> > Patch list and range diff below.
> 
> Everything looks as expected from watching the discussion from
> the sideline.  Looking good.
> 
> Will queue and mark the topic for 'next'.

I just read through the series and couldn't find anything wrong. That being said, my Perl skills are severely lacking, so my assessment may not be worth much :)

Thanks!
Patrick

Back to recent threads