Merge branch 'jk/diff-highlight-more'

Various updates to contrib/diff-highlight, including documentation
updates, test improvements, and color configuration handling.

* jk/diff-highlight-more:
  diff-highlight: fetch all config with one process
  diff-highlight: allow module callers to pass in color config
  diff-highlight: test color config
  diff-highlight: use test_decode_color in tests
  t: add matching negative attributes to test_decode_color
  diff-highlight: check diff-highlight exit status in tests
  diff-highlight: drop perl version dependency back to 5.8
  diff-highlight: mention build instructions

4 files changed, 112 insertions, 36 deletions

diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm
index f0607a4b68..abe457882e 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;
 
@@ -9,18 +9,11 @@ use File::Spec;
 
 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/;
@@ -138,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 {
@@ -170,6 +175,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('oldnormal'),
+			color_config('oldhighlight', "\x1b[7m"),
+			color_config('oldreset', "\x1b[27m")
+		);
+	}
+	if (!defined $NEW_HIGHLIGHT[1]) {
+		@NEW_HIGHLIGHT = (
+			color_config('newnormal', $OLD_HIGHLIGHT[0]),
+			color_config('newhighlight', $OLD_HIGHLIGHT[1]),
+			color_config('newreset', $OLD_HIGHLIGHT[2])
+		);
+	};
+}
+
 sub highlight_pair {
 	my @a = split_line(shift);
 	my @b = split_line(shift);
@@ -218,6 +246,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 1db4440e68..ed8d876a18 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
@@ -127,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)
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 2a9b68cf3b..b38fe2196a 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
@@ -41,8 +38,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 | test_decode_color >diff.act &&
+	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
+	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
 	test_cmp patch.exp diff.act &&
 	test_cmp patch.exp commit.act
 }
@@ -124,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
 '
@@ -146,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
 '
@@ -168,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
 '
@@ -211,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
 '
@@ -230,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
 '
 
@@ -248,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
 '
 
@@ -331,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
 '
 
@@ -351,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
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";