{"thread":{"id":"65313","subject":"[PATCH 0/8] some diff-highlight tweaks","startedAt":"2026-03-20T00:41:45Z","lastAt":"2026-03-24T06:50:45Z","messageCount":28,"participants":["Jeff King","Tian Yuchen","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"539449","messageId":"20260320004138.GA3653623@coredump.intra.peff.net","threadId":"65313","inReplyTo":null,"subject":"[PATCH 0/8] some diff-highlight tweaks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:41:38Z","receivedAt":"2026-03-20T00:41:45Z","isPatch":true,"body":"Here are a few small changes to diff-highlight. The main motivation is\nworking better with diff-so-fancy, which uses DiffHighlight.pm under the\nhood. But it was a good opportunity to polish up the tests and README,\nand the final patch implements a small optimization I'd been meaning to\ndo for a while.\n\nI based these on the bugfix patch I sent a few days ago in:\n\n https://lore.kernel.org/git/20260317230223.GA716496@coredump.intra.peff.net/\n\nThey don't _need_ to come after that, but there are otherwise textual\nconflicts as they both add new tests in the same spot.\n\n [1/8]: diff-highlight: mention build instructions\n [2/8]: diff-highlight: drop perl version dependency back to 5.8\n [3/8]: diff-highlight: check diff-highlight exit status in tests\n [4/8]: t: add matching negative attributes to test_decode_color\n [5/8]: diff-highlight: use test_decode_color in tests\n [6/8]: diff-highlight: test color config\n [7/8]: diff-highlight: allow module callers to pass in color config\n [8/8]: diff-highlight: fetch all config with one process\n\n contrib/diff-highlight/DiffHighlight.pm | 57 ++++++++++++----\n contrib/diff-highlight/README | 19 +++++-\n .../diff-highlight/t/t9400-diff-highlight.sh | 67 +++++++++++++------\n t/test-lib-functions.sh | 3 +\n 4 files changed, 111 insertions(+), 35 deletions(-)\n\n-Peff\n"},{"id":"539450","messageId":"20260320004208.GA3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 1/8] diff-highlight: mention build instructions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:42:08Z","receivedAt":"2026-03-20T00:42:09Z","isPatch":true,"body":"Once upon a time, this was just a script in a directory that could be\nrun directly. That changed in 0c977dbc81 (diff-highlight: split code\ninto module, 2017-06-15). Let's update the README to make it more clear\nthat you need to run make.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/README | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README\nindex 1db4440e68..9c89146fb0 100644\n--- a/contrib/diff-highlight/README\n+++ b/contrib/diff-highlight/README\n@@ -39,10 +39,21 @@ visually distracting. Non-diff lines and existing diff coloration is\n preserved; the intent is that the output should look exactly the same as\n the input, except for the occasional highlight.\n \n+Build/Install\n+-------------\n+\n+You can build the `diff-highlight` script by running `make` from within\n+the diff-highlight directory. There is no `make install` target; you can\n+copy the built script to your $PATH.\n+\n+You can run diff-highlight's internal tests by running `make test`. Note\n+that you must also build Git itself first (by running `make` from the\n+top-level of the project).\n+\n Use\n ---\n \n-You can try out the diff-highlight program with:\n+You can try out the built diff-highlight program with:\n \n ---------------------------------------------\n git log -p --color | /path/to/diff-highlight\n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539451","messageId":"20260320004242.GB3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 2/8] diff-highlight: drop perl version dependency back to 5.8","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:42:42Z","receivedAt":"2026-03-20T00:42:44Z","isPatch":true,"body":"From: Scott Baker \n\nThe diff-highlight code does not rely on any perl features beyond what\nperl 5.8 provides. We bumped it to v5.26 along with the rest of the\nproject's perl scripts in 702d8c1f3b (Require Perl 5.26.0, 2024-10-23).\n\nThere's some value in just having a uniform baseline for the project,\nbut I think diff-highlight is special here:\n\n - it's in a contrib/ directory that is not frequently touched, so\n there is little risk of Git developers getting annoyed that modern\n perl features are not available\n\n - it provides a module used by other projects. In particular,\n diff-so-fancy relies on DiffHighlight.pm but does not otherwise\n require a perl version more modern than 5.8.\n\nLet's drop back to the more conservative requirement.\n\nSigned-off-by: Scott Baker \nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex f0607a4b68..a5e5de3b18 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -1,6 +1,6 @@\n package DiffHighlight;\n \n-require v5.26;\n+require v5.008;\n use warnings FATAL => 'all';\n use strict;\n \n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539452","messageId":"20260320004303.GC3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 3/8] diff-highlight: check diff-highlight exit status in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:43:03Z","receivedAt":"2026-03-20T00:43:05Z","isPatch":true,"body":"When testing diff-highlight, we pipe the output through a sanitizing\nfunction. This loses the exit status of diff-highlight itself, which\ncould mean we are missing cases where it crashes or exits unexpectedly.\nUse an extra tempfile to avoid the pipe.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 2a9b68cf3b..42d331c6cd 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -41,8 +41,10 @@ dh_test () {\n \t\tgit show >commit.raw\n \t} >/dev/null &&\n \n-\t\"$DIFF_HIGHLIGHT\" diff.act &&\n-\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\t\"$DIFF_HIGHLIGHT\" diff.hi &&\n+\ttest_strip_patch_header diff.act\n+\t\"$DIFF_HIGHLIGHT\" commit.hi &&\n+\ttest_strip_patch_header commit.act &&\n \ttest_cmp patch.exp diff.act &&\n \ttest_cmp patch.exp commit.act\n }\n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539453","messageId":"20260320004336.GD3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 4/8] t: add matching negative attributes to test_decode_color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:43:36Z","receivedAt":"2026-03-20T00:43:38Z","isPatch":true,"body":"Most of the ANSI color attributes have an \"off\" variant. We don't use\nthese yet in our test suite, so we never bothered to decode them. Add\nthe ones that match the attributes we encode so we can make use of them.\n\nThere are even more attributes not covered on the positive side, so this\nis meant to be useful but not all-inclusive.\n\nNote that \"nobold\" and \"nodim\" are the same code, so I've decoded this\nas \"normal intensity\".\n\nSigned-off-by: Jeff King \n---\nThis is the only patch that touches anything outside of\ncontrib/diff-highlight, but hopefully it is uncontroversial. ;)\n\n t/test-lib-functions.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 14e238d24d..f3af10fb7e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -48,6 +48,9 @@ test_decode_color () {\n \t\t\tif (n == 2) return \"FAINT\";\n \t\t\tif (n == 3) return \"ITALIC\";\n \t\t\tif (n == 7) return \"REVERSE\";\n+\t\t\tif (n == 22) return \"NORMAL_INTENSITY\";\n+\t\t\tif (n == 23) return \"NOITALIC\";\n+\t\t\tif (n == 27) return \"NOREVERSE\";\n \t\t\tif (n == 30) return \"BLACK\";\n \t\t\tif (n == 31) return \"RED\";\n \t\t\tif (n == 32) return \"GREEN\";\n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539454","messageId":"20260320004436.GE3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 5/8] diff-highlight: use test_decode_color in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:44:36Z","receivedAt":"2026-03-20T00:44:38Z","isPatch":true,"body":"The diff-highlight tests use raw color bytes when comparing expected and\nactual output. Let's use test_decode_color, which is our usual technique\nin other tests. It makes reading test output diffs a bit easier, since\nyou're not relying on your terminal to interpret the result (or worse,\ninterpreting characters yourself via \"cat -A\").\n\nThis will also make it easier to add tests with new colors/attributes,\nwithout having to pre-define the byte sequences ourselves.\n\nSigned-off-by: Jeff King \n---\n .../diff-highlight/t/t9400-diff-highlight.sh | 37 +++++++++----------\n 1 file changed, 17 insertions(+), 20 deletions(-)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 42d331c6cd..ba80cda7c8 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)\n TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n \n-CW=\"$(printf \"\\033[7m\")\"\t# white\n-CR=\"$(printf \"\\033[27m\")\"\t# reset\n-\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n . \"$TEST_DIRECTORY\"/test-lib.sh\n@@ -42,9 +39,9 @@ dh_test () {\n \t} >/dev/null &&\n \n \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n-\ttest_strip_patch_header diff.act\n+\ttest_strip_patch_header diff.act\n \t\"$DIFF_HIGHLIGHT\" commit.hi &&\n-\ttest_strip_patch_header commit.act &&\n+\ttest_strip_patch_header commit.act &&\n \ttest_cmp patch.exp diff.act &&\n \ttest_cmp patch.exp commit.act\n }\n@@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-${CW}b${CR}bb\n-\t\t+${CW}0${CR}bb\n+\t\t-bbb\n+\t\t+0bb\n \t\t ccc\n \tEOF\n '\n@@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-bb${CW}b${CR}\n-\t\t+bb${CW}0${CR}\n+\t\t-bbb\n+\t\t+bb0\n \t\t ccc\n \tEOF\n '\n@@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-b${CW}b${CR}b\n-\t\t+b${CW}0${CR}b\n+\t\t-bbb\n+\t\t+b0b\n \t\t ccc\n \tEOF\n '\n@@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-b${CW}b${CR}b\n-\t\t+b${CW}0${CR}b\n+\t\t-bbb\n+\t\t+b0b\n \t\t+ccc\n \tEOF\n '\n@@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '\n \techo \"unic${o_stroke}de\" >b &&\n \tdh_test a b <<-EOF\n \t\t@@ -1 +1 @@\n-\t\t-unic${CW}${o_accent}${CR}de\n-\t\t+unic${CW}${o_stroke}${CR}de\n+\t\t-unic${o_accent}de\n+\t\t+unic${o_stroke}de\n \tEOF\n '\n \n@@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '\n \techo \"unico${combine_circum}de\" >b &&\n \tdh_test a b <<-EOF\n \t\t@@ -1 +1 @@\n-\t\t-unic${CW}o${combine_accent}${CR}de\n-\t\t+unic${CW}o${combine_circum}${CR}de\n+\t\t-unico${combine_accent}de\n+\t\t+unico${combine_circum}de\n \tEOF\n '\n \n@@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '\n \t+++ b/file\n \t@@ -1,3 +1,3 @@\n \t before\n-\t-the ${CW}old${CR} line\n-\t+the ${CW}new${CR} line\n+\t-the old line\n+\t+the new line\n \t -leading dash\n \tEOF\n \tgit log --graph -p -1 | \"$DIFF_HIGHLIGHT\" >actual.raw &&\n-\ttrim_graph actual &&\n+\ttrim_graph actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539455","messageId":"20260320004519.GF3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 6/8] diff-highlight: test color config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:45:19Z","receivedAt":"2026-03-20T00:45:21Z","isPatch":true,"body":"We added configurable colors long ago in bca45fbc1f (diff-highlight:\nallow configurable colors, 2014-11-20), but never actually tested it.\nSince we'll be touching the color code in a moment, this is a good time\nto beef up the tests.\n\nNote that we cover both the highlight/reset style used by the default\ncolors, as well as the normal/highlight style added by that commit\n(which was previously totally untested).\n\nSigned-off-by: Jeff King \n---\n .../diff-highlight/t/t9400-diff-highlight.sh | 28 +++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex ba80cda7c8..828d59e9c6 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -350,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' '\n \tEOF\n '\n \n+test_expect_success 'configure set/reset colors' '\n+\ttest_config color.diff-highlight.oldhighlight bold &&\n+\ttest_config color.diff-highlight.oldreset nobold &&\n+\ttest_config color.diff-highlight.newhighlight italic &&\n+\ttest_config color.diff-highlight.newreset noitalic &&\n+\techo \"prefix a suffix\" >a &&\n+\techo \"prefix b suffix\" >b &&\n+\tdh_test a b <<-\\EOF\n+\t@@ -1 +1 @@\n+\t-prefix a suffix\n+\t+prefix b suffix\n+\tEOF\n+'\n+\n+test_expect_success 'configure normal/highlight colors' '\n+\ttest_config color.diff-highlight.oldnormal red &&\n+\ttest_config color.diff-highlight.oldhighlight magenta &&\n+\ttest_config color.diff-highlight.newnormal green &&\n+\ttest_config color.diff-highlight.newhighlight yellow &&\n+\techo \"prefix a suffix\" >a &&\n+\techo \"prefix b suffix\" >b &&\n+\tdh_test a b <<-\\EOF\n+\t@@ -1 +1 @@\n+\t-prefix a suffix\n+\t+prefix b suffix\n+\tEOF\n+'\n+\n test_done\n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539459","messageId":"20260320004708.GG3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 7/8] diff-highlight: allow module callers to pass in color config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:47:08Z","receivedAt":"2026-03-20T00:47:10Z","isPatch":true,"body":"From: Scott Baker \n\nUsers of the module may want to pass in their own color config for a few\nobvious reasons:\n\n - they are pulling the config from different variables than\n diff-highlight itself uses\n\n - they are loading the config in a more efficient way (say, by parsing\n git-config --list) and don't want to incur the six (!) git-config\n calls that DiffHighlight.pm runs to check all config\n\nLet's allow users of the module to pass in the color config, and\nlazy-load it when needed if they haven't.\n\nSigned-off-by: Scott Baker \nSigned-off-by: Jeff King \n---\nThe next commit improves the six-process situation, but I think\ndiff-so-fancy will still want this, as it uses a single invocation\nwhich checks other non-color config (and already bit the bullet on\nimplementing its own color parsing).\n\n contrib/diff-highlight/DiffHighlight.pm | 41 +++++++++++++++++--------\n contrib/diff-highlight/README | 6 ++++\n 2 files changed, 35 insertions(+), 12 deletions(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex a5e5de3b18..96369eadf9 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -9,18 +9,11 @@ package DiffHighlight;\n \n my $NULL = File::Spec->devnull();\n \n-# Highlight by reversing foreground and background. You could do\n-# other things like bold or underline if you prefer.\n-my @OLD_HIGHLIGHT = (\n-\tcolor_config('color.diff-highlight.oldnormal'),\n-\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n-\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n-);\n-my @NEW_HIGHLIGHT = (\n-\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n-\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n-\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n-);\n+# The color theme is initially set to nothing here to allow outside callers\n+# to set the colors for their application. If nothing is sent in we use\n+# colors from git config in load_color_config().\n+our @OLD_HIGHLIGHT = ();\n+our @NEW_HIGHLIGHT = ();\n \n my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n@@ -170,6 +163,29 @@ sub show_hunk {\n \t$line_cb->(@queue);\n }\n \n+sub load_color_config {\n+\t# If the colors were NOT set from outside this module we load them on-demand\n+\t# from the git config. Note that only one of elements 0 and 2 in each\n+\t# array is used (depending on whether you are doing set/unset on an\n+\t# attribute, or specifying normal vs highlighted coloring). So we use\n+\t# element 1 as our check for whether colors were passed in; it should\n+\t# always be set if you want highlighting to do anything.\n+\tif (!defined $OLD_HIGHLIGHT[1]) {\n+\t\t@OLD_HIGHLIGHT = (\n+\t\t\tcolor_config('color.diff-highlight.oldnormal'),\n+\t\t\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n+\t\t\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n+\t\t);\n+\t}\n+\tif (!defined $NEW_HIGHLIGHT[1]) {\n+\t\t@NEW_HIGHLIGHT = (\n+\t\t\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n+\t\t\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n+\t\t\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n+\t\t);\n+\t};\n+}\n+\n sub highlight_pair {\n \tmy @a = split_line(shift);\n \tmy @b = split_line(shift);\n@@ -218,6 +234,7 @@ sub highlight_pair {\n \t}\n \n \tif (is_pair_interesting(\\@a, $pa, $sa, \\@b, $pb, $sb)) {\n+\t\tload_color_config();\n \t\treturn highlight_line(\\@a, $pa, $sa, \\@OLD_HIGHLIGHT),\n \t\t highlight_line(\\@b, $pb, $sb, \\@NEW_HIGHLIGHT);\n \t}\ndiff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README\nindex 9c89146fb0..ed8d876a18 100644\n--- a/contrib/diff-highlight/README\n+++ b/contrib/diff-highlight/README\n@@ -138,6 +138,12 @@ Your script may set up one or more of the following variables:\n processing a logical chunk of input). The default function flushes\n stdout.\n \n+ - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these\n+ arrays specify the normal, highlighted, and reset colors (in that order)\n+ for old/new lines. If unset, values will be retrieved by calling `git\n+ config` (see \"Color Config\" above). Note that these should be the literal\n+ color bytes (starting with an ANSI escape code), not color names.\n+\n The script may then feed lines, one at a time, to DiffHighlight::handle_line().\n When lines are done processing, they will be fed to $line_cb. Note that\n DiffHighlight may queue up many input lines (to analyze a whole hunk)\n-- \n2.53.0.945.ge67b727e8d\n\n"},{"id":"539460","messageId":"20260320004856.GH3654226@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH 8/8] diff-highlight: fetch all config with one process","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-20T00:48:56Z","receivedAt":"2026-03-20T00:48:57Z","isPatch":true,"body":"When diff-highlight was written, there was no way to fetch multiple\nconfig keys _and_ have them interpreted as colors. So we were stuck\nwith either invoking git-config once for each config key, or fetching\nthem all and converting human-readable color names into ANSI codes\nourselves.\n\nI chose the former, but it means that diff-highlight kicks off 6\ngit-config processes (even if you haven't configured anything, it has to\ncheck each one).\n\nBut since Git 2.18.0, we can do:\n\n git config --type=color --get-regexp=^color\\.diff-highlight\\.\n\nto get all of them in one shot.\n\nNote that any callers which pass in colors directly to the module via\n@OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are\nunaffected; those colors suppress any config lookup we'd do ourselves.\n\nYou can see the effect like:\n\n # diff-highlight suppresses git-config's stderr, so dump\n # trace through descriptor 3\n git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null\n\nwhich drops from 6 lines down to 1.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/DiffHighlight.pm | 26 ++++++++++++++++++-------\n 1 file changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex 96369eadf9..a22ba7a851 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -131,8 +131,20 @@ sub highlight_stdin {\n # of it being used in other settings. Let's handle our own\n # fallback, which means we will work even if git can't be run.\n sub color_config {\n+\tour $cached_config;\n \tmy ($key, $default) = @_;\n-\tmy $s = `git config --get-color $key 2>$NULL`;\n+\n+\tif (!defined $cached_config) {\n+\t\t$cached_config = {};\n+\t\tmy $data = `git config --type=color --get-regexp '^color\\.diff-highlight\\.' 2>$NULL`;\n+\t\tfor my $line (split /\\n/, $data) {\n+\t\t\tmy ($key, $color) = split ' ', $line, 2;\n+\t\t\t$key =~ s/^color\\.diff-highlight\\.// or next;\n+\t\t\t$cached_config->{$key} = $color;\n+\t\t}\n+\t}\n+\n+\tmy $s = $cached_config->{$key};\n \treturn length($s) ? $s : $default;\n }\n \n@@ -172,16 +184,16 @@ sub load_color_config {\n \t# always be set if you want highlighting to do anything.\n \tif (!defined $OLD_HIGHLIGHT[1]) {\n \t\t@OLD_HIGHLIGHT = (\n-\t\t\tcolor_config('color.diff-highlight.oldnormal'),\n-\t\t\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n-\t\t\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n+\t\t\tcolor_config('oldnormal'),\n+\t\t\tcolor_config('oldhighlight', \"\\x1b[7m\"),\n+\t\t\tcolor_config('oldreset', \"\\x1b[27m\")\n \t\t);\n \t}\n \tif (!defined $NEW_HIGHLIGHT[1]) {\n \t\t@NEW_HIGHLIGHT = (\n-\t\t\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n-\t\t\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n-\t\t\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n+\t\t\tcolor_config('newnormal', $OLD_HIGHLIGHT[0]),\n+\t\t\tcolor_config('newhighlight', $OLD_HIGHLIGHT[1]),\n+\t\t\tcolor_config('newreset', $OLD_HIGHLIGHT[2])\n \t\t);\n \t};\n }\n-- \n2.53.0.945.ge67b727e8d\n"},{"id":"539655","messageId":"9d3633e4-6413-4932-a29d-e0347546ede8@malon.dev","threadId":"65313","inReplyTo":"20260320004856.GH3654226@coredump.intra.peff.net","subject":"Re: [PATCH 8/8] diff-highlight: fetch all config with one process","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-03-22T17:18:30Z","receivedAt":"2026-03-22T17:18:37Z","isPatch":true,"body":"Hi Jeff,\n\nOn 3/20/26 08:48, Jeff King wrote:\n> When diff-highlight was written, there was no way to fetch multiple\n> config keys _and_ have them interpreted as colors. So we were stuck\n> with either invoking git-config once for each config key, or fetching\n> them all and converting human-readable color names into ANSI codes\n> ourselves.\n> \n> I chose the former, but it means that diff-highlight kicks off 6\n> git-config processes (even if you haven't configured anything, it has to\n> check each one).\n> \n> But since Git 2.18.0, we can do:\n> \n> git config --type=color --get-regexp=^color\\.diff-highlight\\.\n> \n> to get all of them in one shot.\n> \n> Note that any callers which pass in colors directly to the module via\n> @OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are\n> unaffected; those colors suppress any config lookup we'd do ourselves.\n> \n> You can see the effect like:\n> \n> # diff-highlight suppresses git-config's stderr, so dump\n> # trace through descriptor 3\n> git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null\n> \n> which drops from 6 lines down to 1.\n> \n> Signed-off-by: Jeff King \n> ---\n> contrib/diff-highlight/DiffHighlight.pm | 26 ++++++++++++++++++-------\n> 1 file changed, 19 insertions(+), 7 deletions(-)\n> \n> diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\n> index 96369eadf9..a22ba7a851 100644\n> --- a/contrib/diff-highlight/DiffHighlight.pm\n> +++ b/contrib/diff-highlight/DiffHighlight.pm\n> @@ -131,8 +131,20 @@ sub highlight_stdin {\n> # of it being used in other settings. Let's handle our own\n> # fallback, which means we will work even if git can't be run.\n> sub color_config {\n> +\tour $cached_config;\n> \tmy ($key, $default) = @_;\n\n$key...\n\n> -\tmy $s = `git config --get-color $key 2>$NULL`;\n> +\n> +\tif (!defined $cached_config) {\n> +\t\t$cached_config = {};\n> +\t\tmy $data = `git config --type=color --get-regexp '^color\\.diff-highlight\\.' 2>$NULL`;\n> +\t\tfor my $line (split /\\n/, $data) {\n> +\t\t\tmy ($key, $color) = split ' ', $line, 2;\n\n...another $key. I think it would be better to change the name here. \nWhat do you think?\n\n> +\t\t\t$key =~ s/^color\\.diff-highlight\\.// or next;\n> +\t\t\t$cached_config->{$key} = $color;\n> +\t\t}\n> +\t}\n> +\n\n...\n\n> +\tmy $s = $cached_config->{$key};\n> \treturn length($s) ? $s : $default;\n> }\n> \n\nSomething doesn't feel quite right here.\n\nIf the user has not configured color.diff-highlight.*, the expression \ngit config --type=color --get-regexp=^color\\.diff-highlight\\. will not \nfind a match and should not output anything. In this case, \n%cached_config->{$key} becomes undef, length() returns 0, and a warning \nis issued.\n\nBut we have \"use warnings FATAL => 'all'\". This situation will result in \na fatal error, which I don't think is what we want.\n\n\n> @@ -172,16 +184,16 @@ sub load_color_config {\n> \t# always be set if you want highlighting to do anything.\n> \tif (!defined $OLD_HIGHLIGHT[1]) {\n> \t\t@OLD_HIGHLIGHT = (\n> -\t\t\tcolor_config('color.diff-highlight.oldnormal'),\n> -\t\t\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n> -\t\t\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n> +\t\t\tcolor_config('oldnormal'),\n> +\t\t\tcolor_config('oldhighlight', \"\\x1b[7m\"),\n> +\t\t\tcolor_config('oldreset', \"\\x1b[27m\")\n> \t\t);\n> \t}\n> \tif (!defined $NEW_HIGHLIGHT[1]) {\n> \t\t@NEW_HIGHLIGHT = (\n> -\t\t\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n> -\t\t\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n> -\t\t\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n> +\t\t\tcolor_config('newnormal', $OLD_HIGHLIGHT[0]),\n> +\t\t\tcolor_config('newhighlight', $OLD_HIGHLIGHT[1]),\n> +\t\t\tcolor_config('newreset', $OLD_HIGHLIGHT[2])\n> \t\t);\n> \t};\n> }\n\nRegards,\n\nYuchen\n\n"},{"id":"539656","messageId":"b992e118-f948-4145-8d77-96f00b497f99@gmail.com","threadId":"65313","inReplyTo":"20260320004436.GE3654226@coredump.intra.peff.net","subject":"Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-22T17:24:00Z","receivedAt":"2026-03-22T17:24:04Z","isPatch":true,"body":"On 3/20/26 08:44, Jeff King wrote:\n> The diff-highlight tests use raw color bytes when comparing expected and\n> actual output. Let's use test_decode_color, which is our usual technique\n> in other tests. It makes reading test output diffs a bit easier, since\n> you're not relying on your terminal to interpret the result (or worse,\n> interpreting characters yourself via \"cat -A\").\n> \n> This will also make it easier to add tests with new colors/attributes,\n> without having to pre-define the byte sequences ourselves.\n> \n> Signed-off-by: Jeff King \n> ---\n> .../diff-highlight/t/t9400-diff-highlight.sh | 37 +++++++++----------\n> 1 file changed, 17 insertions(+), 20 deletions(-)\n> \n> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> index 42d331c6cd..ba80cda7c8 100755\n> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n> @@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)\n> TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n> DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n> \n> -CW=\"$(printf \"\\033[7m\")\"\t# white\n> -CR=\"$(printf \"\\033[27m\")\"\t# reset\n> -\n> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n> . \"$TEST_DIRECTORY\"/test-lib.sh\n> @@ -42,9 +39,9 @@ dh_test () {\n> \t} >/dev/null &&\n> \n> \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n> -\ttest_strip_patch_header diff.act\n> +\ttest_strip_patch_header diff.act\n\nAlthough this is just simple text filtering and leaving it as is \nwouldn’t cause any problems IMO, why not go ahead and add the && while \nyou’re at it?\n\nI've noticed that there are several missing &&.\n\n> \t\"$DIFF_HIGHLIGHT\" commit.hi &&\n> -\ttest_strip_patch_header commit.act &&\n> +\ttest_strip_patch_header commit.act &&\n> \ttest_cmp patch.exp diff.act &&\n> \ttest_cmp patch.exp commit.act\n> }\n> @@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '\n> \tdh_test a b <<-EOF\n> \t\t@@ -1,3 +1,3 @@\n> \t\t aaa\n> -\t\t-${CW}b${CR}bb\n> -\t\t+${CW}0${CR}bb\n> +\t\t-bbb\n> +\t\t+0bb\n> \t\t ccc\n> \tEOF\n> '\n> @@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '\n> \tdh_test a b <<-EOF\n> \t\t@@ -1,3 +1,3 @@\n> \t\t aaa\n> -\t\t-bb${CW}b${CR}\n> -\t\t+bb${CW}0${CR}\n> +\t\t-bbb\n> +\t\t+bb0\n> \t\t ccc\n> \tEOF\n> '\n> @@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '\n> \tdh_test a b <<-EOF\n> \t\t@@ -1,3 +1,3 @@\n> \t\t aaa\n> -\t\t-b${CW}b${CR}b\n> -\t\t+b${CW}0${CR}b\n> +\t\t-bbb\n> +\t\t+b0b\n> \t\t ccc\n> \tEOF\n> '\n> @@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n> \tdh_test a b <<-EOF\n> \t\t@@ -1,3 +1,3 @@\n> \t\t aaa\n> -\t\t-b${CW}b${CR}b\n> -\t\t+b${CW}0${CR}b\n> +\t\t-bbb\n> +\t\t+b0b\n> \t\t+ccc\n> \tEOF\n> '\n> @@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '\n> \techo \"unic${o_stroke}de\" >b &&\n> \tdh_test a b <<-EOF\n> \t\t@@ -1 +1 @@\n> -\t\t-unic${CW}${o_accent}${CR}de\n> -\t\t+unic${CW}${o_stroke}${CR}de\n> +\t\t-unic${o_accent}de\n> +\t\t+unic${o_stroke}de\n> \tEOF\n> '\n> \n> @@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '\n> \techo \"unico${combine_circum}de\" >b &&\n> \tdh_test a b <<-EOF\n> \t\t@@ -1 +1 @@\n> -\t\t-unic${CW}o${combine_accent}${CR}de\n> -\t\t+unic${CW}o${combine_circum}${CR}de\n> +\t\t-unico${combine_accent}de\n> +\t\t+unico${combine_circum}de\n> \tEOF\n> '\n> \n> @@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '\n> \t+++ b/file\n> \t@@ -1,3 +1,3 @@\n> \t before\n> -\t-the ${CW}old${CR} line\n> -\t+the ${CW}new${CR} line\n> +\t-the old line\n> +\t+the new line\n> \t -leading dash\n> \tEOF\n> \tgit log --graph -p -1 | \"$DIFF_HIGHLIGHT\" >actual.raw &&\n> -\ttrim_graph actual &&\n> +\ttrim_graph actual &&\n> \ttest_cmp expect actual\n> '\n> \n\nThanks,\n\nYuchen\n\n"},{"id":"539672","messageId":"20260322204509.GA2047044@coredump.intra.peff.net","threadId":"65313","inReplyTo":"9d3633e4-6413-4932-a29d-e0347546ede8@malon.dev","subject":"Re: [PATCH 8/8] diff-highlight: fetch all config with one process","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-22T20:45:09Z","receivedAt":"2026-03-22T20:45:17Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 01:18:30AM +0800, Tian Yuchen wrote:\n\n> > -\tmy $s = `git config --get-color $key 2>$NULL`;\n> > +\n> > +\tif (!defined $cached_config) {\n> > +\t\t$cached_config = {};\n> > +\t\tmy $data = `git config --type=color --get-regexp '^color\\.diff-highlight\\.' 2>$NULL`;\n> > +\t\tfor my $line (split /\\n/, $data) {\n> > +\t\t\tmy ($key, $color) = split ' ', $line, 2;\n> \n> ...another $key. I think it would be better to change the name here. What do\n> you think?\n\nI noticed it, too, but didn't have a better name (in fact they are of\nthe same type, just two different contexts). Shadowing seemed less bad\nto me than using a mis-matched name.\n\n> > +\tmy $s = $cached_config->{$key};\n> > \treturn length($s) ? $s : $default;\n> > }\n> \n> Something doesn't feel quite right here.\n> \n> If the user has not configured color.diff-highlight.*, the expression git\n> config --type=color --get-regexp=^color\\.diff-highlight\\. will not find a\n> match and should not output anything. In this case, %cached_config->{$key}\n> becomes undef, length() returns 0, and a warning is issued.\n\nThe length() of undef is also undef (and documented in \"perldoc -f\nlength\"). But either way, length($s) will be false, and we will return\n$default, not $s.\n\nI don't get any warning on perl 5.40.1. Are you seeing one on a\ndifferent version?\n\n-Peff\n"},{"id":"539673","messageId":"20260322204750.GB2047044@coredump.intra.peff.net","threadId":"65313","inReplyTo":"b992e118-f948-4145-8d77-96f00b497f99@gmail.com","subject":"Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-22T20:47:50Z","receivedAt":"2026-03-22T20:47:51Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 01:24:00AM +0800, Tian Yuchen wrote:\n\n> > @@ -42,9 +39,9 @@ dh_test () {\n> > \t} >/dev/null &&\n> > \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n> > -\ttest_strip_patch_header diff.act\n> > +\ttest_strip_patch_header diff.act\n> \n> Although this is just simple text filtering and leaving it as is wouldn’t\n> cause any problems IMO, why not go ahead and add the && while you’re at it?\n\nThe bug is in an earlier commit (patch 3), which breaks apart the pipe\nbut doesn't add the necessary &&. And it's more than just text\nfiltering; it breaks the &&-chain, so we miss the exit code of\n$DIFF_HIGHLIGHT (which was the whole point of patch 3).\n\nchainlint doesn't find it because we're in a helper function, not\ndirectinly in a test snippet.\n\nI'll send a revised series to fix it, but...\n\n> I've noticed that there are several missing &&.\n\nWhere else do you see?\n\nOr do you mean that we should not pipe text filtering commands? There\nI'd disagree. We are not likely to see a failure from 'sed', and if we\ndo, the fact that the output does not match would catch it. And the cost\nof breaking every command down without pipes means having to manage lots\nof intermediate files.\n\n-Peff\n"},{"id":"539686","messageId":"9818e3ec-838a-4eef-8436-a395f2970d42@malon.dev","threadId":"65313","inReplyTo":"20260322204509.GA2047044@coredump.intra.peff.net","subject":"Re: [PATCH 8/8] diff-highlight: fetch all config with one process","fromName":"Tian Yuchen","fromEmail":"cat@malon.dev","sentAt":"2026-03-23T05:39:44Z","receivedAt":"2026-03-23T05:39:55Z","isPatch":true,"body":"On 3/23/26 04:45, Jeff King wrote:\n\n> I noticed it, too, but didn't have a better name (in fact they are of\n> the same type, just two different contexts). Shadowing seemed less bad\n> to me than using a mis-matched name. \n\nFair enough! Let's leave it as it is ;)\n\n> The length() of undef is also undef (and documented in \"perldoc -f\n> length\"). But either way, length($s) will be false, and we will return\n> $default, not $s.\n> \n> I don't get any warning on perl 5.40.1. Are you seeing one on a\n> different version?\n\nIt's mainly because I saw that you changed the required version earlier：\n\n-require v5.26;\n+require v5.008;\n\nI clearly remember that in older versions of Perl, the length function \nbehaved differently than it does now.\n\n\tuse strict;\n\tuse warnings FATAL => 'uninitialized';\n\n\tmy $x;\n\tprint \"length = \", length($x), \"\\n\";\n\nIn 5.8.8 the output is:\n\n\tUse of uninitialized value in length at test.pl line 5.\n\nAnd in 5.38.2 the output is:\n\n\tUse of uninitialized value in print at test.pl line 5. length =\n\t\nIn the first case, the code will throw an error and exit. Although I \nhaven't compiled this patch under version 5.8.8 yet, I suspect there \nwill be issues.\n> \n> -Peff\n\nRegards, Yuchen\n\n"},{"id":"539687","messageId":"b1064c6b-21df-4ea1-b753-549e0ca1f346@gmail.com","threadId":"65313","inReplyTo":"20260322204750.GB2047044@coredump.intra.peff.net","subject":"Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-23T05:48:49Z","receivedAt":"2026-03-23T05:48:54Z","isPatch":true,"body":"On 3/23/26 04:47, Jeff King wrote:\n> On Mon, Mar 23, 2026 at 01:24:00AM +0800, Tian Yuchen wrote:\n> \n>>> @@ -42,9 +39,9 @@ dh_test () {\n>>> \t} >/dev/null &&\n>>> \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n>>> -\ttest_strip_patch_header diff.act\n>>> +\ttest_strip_patch_header diff.act\n>>\n>> Although this is just simple text filtering and leaving it as is wouldn’t\n>> cause any problems IMO, why not go ahead and add the && while you’re at it?\n> \n> The bug is in an earlier commit (patch 3), which breaks apart the pipe\n> but doesn't add the necessary &&. And it's more than just text\n> filtering; it breaks the &&-chain, so we miss the exit code of\n> $DIFF_HIGHLIGHT (which was the whole point of patch 3).\n> \n> chainlint doesn't find it because we're in a helper function, not\n> directinly in a test snippet.\n> \n> I'll send a revised series to fix it, but...\n> \n>> I've noticed that there are several missing &&.\n> \n> Where else do you see?\n> \n> Or do you mean that we should not pipe text filtering commands? There\n> I'd disagree. We are not likely to see a failure from 'sed', and if we\n> do, the fact that the output does not match would catch it. And the cost\n> of breaking every command down without pipes means having to manage lots\n> of intermediate files.\n> \n> -Peff\n\nOh, I see.\n\nTo be honest, I didn't think about it in that much detail. I just \nnoticed this tiny issue and wanted to take the opportunity to remind you \nto check other parts as well. I'm not saying we should remove those pipe \ncommands :P\n\nThanks for the explanation about chainlint.\n\nRegards, Yuchen\n"},{"id":"539688","messageId":"20260323055355.GA9976@coredump.intra.peff.net","threadId":"65313","inReplyTo":"b1064c6b-21df-4ea1-b753-549e0ca1f346@gmail.com","subject":"Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T05:53:55Z","receivedAt":"2026-03-23T05:53:57Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 01:48:49PM +0800, Tian Yuchen wrote:\n\n> To be honest, I didn't think about it in that much detail. I just noticed\n> this tiny issue and wanted to take the opportunity to remind you to check\n> other parts as well. I'm not saying we should remove those pipe commands :P\n\nAh, OK. I think that is the only spot, then. Thanks for pointing it out!\nWithout the fix it was missing the whole point of patch 3. ;)\n\n-Peff\n"},{"id":"539689","messageId":"20260323055732.GB9976@coredump.intra.peff.net","threadId":"65313","inReplyTo":"9818e3ec-838a-4eef-8436-a395f2970d42@malon.dev","subject":"Re: [PATCH 8/8] diff-highlight: fetch all config with one process","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T05:57:32Z","receivedAt":"2026-03-23T05:57:34Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 01:39:44PM +0800, Tian Yuchen wrote:\n\n> > I don't get any warning on perl 5.40.1. Are you seeing one on a\n> > different version?\n> \n> It's mainly because I saw that you changed the required version earlier：\n> \n> -require v5.26;\n> +require v5.008;\n> \n> I clearly remember that in older versions of Perl, the length function\n> behaved differently than it does now.\n\nHeh, it figures that dropping back the version requirement would bite me\nimmediately. ;)\n\n> In 5.8.8 the output is:\n> \n> \tUse of uninitialized value in length at test.pl line 5.\n\nYeah, looks like it changed in 5.12:\n\n https://www.effectiveperlprogramming.com/2010/09/in-perl-v5-12-lengthundef-returns-undef/\n\nI had actually written it using exists() originally, but then dropped it\nto keep the diff smaller. Which is a silly reason. I've switched it to\nuse:\n\n return defined($s) ? $s : $default;\n\nwhich I think captures the intent pretty clearly. You could also use\n\"$s || $default\", but I try to avoid that because of surprise-false\nvalues. In modern perl you could just use \"//\", but that wasn't added\nuntil v5.10.\n\nThanks for pointing it out!\n\n-Peff\n"},{"id":"539690","messageId":"20260323060139.GA10215@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260320004138.GA3653623@coredump.intra.peff.net","subject":"[PATCH v2 0/8] some diff-highlight tweaks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:01:39Z","receivedAt":"2026-03-23T06:01:40Z","isPatch":true,"body":"Here's a re-roll based on the review from Yuchen. The two changes are:\n\n 1. Added a missing &&-chain in patch 3 (which cascades into patch 6).\n\n 2. Avoid length(undef), since old perl versions will warn about it.\n\nPatch list and range diff below.\n\n 1: c59dd0aac9 = 1: c59dd0aac9 contrib/diff-highlight: do not highlight identical pairs\n 2: 16aa04fc6d = 2: 16aa04fc6d diff-highlight: mention build instructions\n 3: 55788fac3a = 3: 55788fac3a diff-highlight: drop perl version dependency back to 5.8\n 4: 7c2af2348b ! 4: 1101c94f65 diff-highlight: check diff-highlight exit status in tests\n @@ contrib/diff-highlight/t/t9400-diff-highlight.sh: dh_test () {\n -\t\"$DIFF_HIGHLIGHT\" diff.act &&\n -\t\"$DIFF_HIGHLIGHT\" commit.act &&\n +\t\"$DIFF_HIGHLIGHT\" diff.hi &&\n -+\ttest_strip_patch_header diff.act\n ++\ttest_strip_patch_header diff.act &&\n +\t\"$DIFF_HIGHLIGHT\" commit.hi &&\n +\ttest_strip_patch_header commit.act &&\n \ttest_cmp patch.exp diff.act &&\n 5: 52f0358329 = 5: 65420b8b79 t: add matching negative attributes to test_decode_color\n 6: 0f1aacf264 ! 6: 60977c32f6 diff-highlight: use test_decode_color in tests\n @@ contrib/diff-highlight/t/t9400-diff-highlight.sh: dh_test () {\n \t} >/dev/null &&\n \n \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n --\ttest_strip_patch_header diff.act\n -+\ttest_strip_patch_header diff.act\n +-\ttest_strip_patch_header diff.act &&\n ++\ttest_strip_patch_header diff.act &&\n \t\"$DIFF_HIGHLIGHT\" commit.hi &&\n -\ttest_strip_patch_header commit.act &&\n +\ttest_strip_patch_header commit.act &&\n 7: 8bad893f09 = 7: bf33329640 diff-highlight: test color config\n 8: 179f83d791 = 8: ea71a8b648 diff-highlight: allow module callers to pass in color config\n 9: b8ff37b193 ! 9: aab7912ca2 diff-highlight: fetch all config with one process\n @@ contrib/diff-highlight/DiffHighlight.pm: sub highlight_stdin {\n +\tour $cached_config;\n \tmy ($key, $default) = @_;\n -\tmy $s = `git config --get-color $key 2>$NULL`;\n +-\treturn length($s) ? $s : $default;\n +\n +\tif (!defined $cached_config) {\n +\t\t$cached_config = {};\n @@ contrib/diff-highlight/DiffHighlight.pm: sub highlight_stdin {\n +\t}\n +\n +\tmy $s = $cached_config->{$key};\n - \treturn length($s) ? $s : $default;\n ++\treturn defined($s) ? $s : $default;\n }\n \n + sub show_hunk {\n @@ contrib/diff-highlight/DiffHighlight.pm: sub load_color_config {\n \t# always be set if you want highlighting to do anything.\n \tif (!defined $OLD_HIGHLIGHT[1]) {\n\n [1/8]: diff-highlight: mention build instructions\n [2/8]: diff-highlight: drop perl version dependency back to 5.8\n [3/8]: diff-highlight: check diff-highlight exit status in tests\n [4/8]: t: add matching negative attributes to test_decode_color\n [5/8]: diff-highlight: use test_decode_color in tests\n [6/8]: diff-highlight: test color config\n [7/8]: diff-highlight: allow module callers to pass in color config\n [8/8]: diff-highlight: fetch all config with one process\n\n contrib/diff-highlight/DiffHighlight.pm | 59 +++++++++++-----\n contrib/diff-highlight/README | 19 +++++-\n .../diff-highlight/t/t9400-diff-highlight.sh | 67 +++++++++++++------\n t/test-lib-functions.sh | 3 +\n 4 files changed, 112 insertions(+), 36 deletions(-)\n\n"},{"id":"539691","messageId":"20260323060158.GA10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 1/8] diff-highlight: mention build instructions","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:01:58Z","receivedAt":"2026-03-23T06:01:59Z","isPatch":true,"body":"Once upon a time, this was just a script in a directory that could be\nrun directly. That changed in 0c977dbc81 (diff-highlight: split code\ninto module, 2017-06-15). Let's update the README to make it more clear\nthat you need to run make.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/README | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README\nindex 1db4440e68..9c89146fb0 100644\n--- a/contrib/diff-highlight/README\n+++ b/contrib/diff-highlight/README\n@@ -39,10 +39,21 @@ visually distracting. Non-diff lines and existing diff coloration is\n preserved; the intent is that the output should look exactly the same as\n the input, except for the occasional highlight.\n \n+Build/Install\n+-------------\n+\n+You can build the `diff-highlight` script by running `make` from within\n+the diff-highlight directory. There is no `make install` target; you can\n+copy the built script to your $PATH.\n+\n+You can run diff-highlight's internal tests by running `make test`. Note\n+that you must also build Git itself first (by running `make` from the\n+top-level of the project).\n+\n Use\n ---\n \n-You can try out the diff-highlight program with:\n+You can try out the built diff-highlight program with:\n \n ---------------------------------------------\n git log -p --color | /path/to/diff-highlight\n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539692","messageId":"20260323060202.GB10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 2/8] diff-highlight: drop perl version dependency back to 5.8","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:02Z","receivedAt":"2026-03-23T06:02:04Z","isPatch":true,"body":"From: Scott Baker \n\nThe diff-highlight code does not rely on any perl features beyond what\nperl 5.8 provides. We bumped it to v5.26 along with the rest of the\nproject's perl scripts in 702d8c1f3b (Require Perl 5.26.0, 2024-10-23).\n\nThere's some value in just having a uniform baseline for the project,\nbut I think diff-highlight is special here:\n\n - it's in a contrib/ directory that is not frequently touched, so\n there is little risk of Git developers getting annoyed that modern\n perl features are not available\n\n - it provides a module used by other projects. In particular,\n diff-so-fancy relies on DiffHighlight.pm but does not otherwise\n require a perl version more modern than 5.8.\n\nLet's drop back to the more conservative requirement.\n\nSigned-off-by: Scott Baker \nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/DiffHighlight.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex f0607a4b68..a5e5de3b18 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -1,6 +1,6 @@\n package DiffHighlight;\n \n-require v5.26;\n+require v5.008;\n use warnings FATAL => 'all';\n use strict;\n \n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539693","messageId":"20260323060205.GC10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 3/8] diff-highlight: check diff-highlight exit status in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:05Z","receivedAt":"2026-03-23T06:02:07Z","isPatch":true,"body":"When testing diff-highlight, we pipe the output through a sanitizing\nfunction. This loses the exit status of diff-highlight itself, which\ncould mean we are missing cases where it crashes or exits unexpectedly.\nUse an extra tempfile to avoid the pipe.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/t/t9400-diff-highlight.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 2a9b68cf3b..7ebff8b18f 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -41,8 +41,10 @@ dh_test () {\n \t\tgit show >commit.raw\n \t} >/dev/null &&\n \n-\t\"$DIFF_HIGHLIGHT\" diff.act &&\n-\t\"$DIFF_HIGHLIGHT\" commit.act &&\n+\t\"$DIFF_HIGHLIGHT\" diff.hi &&\n+\ttest_strip_patch_header diff.act &&\n+\t\"$DIFF_HIGHLIGHT\" commit.hi &&\n+\ttest_strip_patch_header commit.act &&\n \ttest_cmp patch.exp diff.act &&\n \ttest_cmp patch.exp commit.act\n }\n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539694","messageId":"20260323060207.GD10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 4/8] t: add matching negative attributes to test_decode_color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:07Z","receivedAt":"2026-03-23T06:02:09Z","isPatch":true,"body":"Most of the ANSI color attributes have an \"off\" variant. We don't use\nthese yet in our test suite, so we never bothered to decode them. Add\nthe ones that match the attributes we encode so we can make use of them.\n\nThere are even more attributes not covered on the positive side, so this\nis meant to be useful but not all-inclusive.\n\nNote that \"nobold\" and \"nodim\" are the same code, so I've decoded this\nas \"normal intensity\".\n\nSigned-off-by: Jeff King \n---\n t/test-lib-functions.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 14e238d24d..f3af10fb7e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -48,6 +48,9 @@ test_decode_color () {\n \t\t\tif (n == 2) return \"FAINT\";\n \t\t\tif (n == 3) return \"ITALIC\";\n \t\t\tif (n == 7) return \"REVERSE\";\n+\t\t\tif (n == 22) return \"NORMAL_INTENSITY\";\n+\t\t\tif (n == 23) return \"NOITALIC\";\n+\t\t\tif (n == 27) return \"NOREVERSE\";\n \t\t\tif (n == 30) return \"BLACK\";\n \t\t\tif (n == 31) return \"RED\";\n \t\t\tif (n == 32) return \"GREEN\";\n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539695","messageId":"20260323060210.GE10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 5/8] diff-highlight: use test_decode_color in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:10Z","receivedAt":"2026-03-23T06:02:11Z","isPatch":true,"body":"The diff-highlight tests use raw color bytes when comparing expected and\nactual output. Let's use test_decode_color, which is our usual technique\nin other tests. It makes reading test output diffs a bit easier, since\nyou're not relying on your terminal to interpret the result (or worse,\ninterpreting characters yourself via \"cat -A\").\n\nThis will also make it easier to add tests with new colors/attributes,\nwithout having to pre-define the byte sequences ourselves.\n\nSigned-off-by: Jeff King \n---\n .../diff-highlight/t/t9400-diff-highlight.sh | 37 +++++++++----------\n 1 file changed, 17 insertions(+), 20 deletions(-)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 7ebff8b18f..4f3d55a26e 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)\n TEST_DIRECTORY=\"$CURR_DIR\"/../../../t\n DIFF_HIGHLIGHT=\"$CURR_DIR\"/../diff-highlight\n \n-CW=\"$(printf \"\\033[7m\")\"\t# white\n-CR=\"$(printf \"\\033[27m\")\"\t# reset\n-\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n . \"$TEST_DIRECTORY\"/test-lib.sh\n@@ -42,9 +39,9 @@ dh_test () {\n \t} >/dev/null &&\n \n \t\"$DIFF_HIGHLIGHT\" diff.hi &&\n-\ttest_strip_patch_header diff.act &&\n+\ttest_strip_patch_header diff.act &&\n \t\"$DIFF_HIGHLIGHT\" commit.hi &&\n-\ttest_strip_patch_header commit.act &&\n+\ttest_strip_patch_header commit.act &&\n \ttest_cmp patch.exp diff.act &&\n \ttest_cmp patch.exp commit.act\n }\n@@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-${CW}b${CR}bb\n-\t\t+${CW}0${CR}bb\n+\t\t-bbb\n+\t\t+0bb\n \t\t ccc\n \tEOF\n '\n@@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-bb${CW}b${CR}\n-\t\t+bb${CW}0${CR}\n+\t\t-bbb\n+\t\t+bb0\n \t\t ccc\n \tEOF\n '\n@@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-b${CW}b${CR}b\n-\t\t+b${CW}0${CR}b\n+\t\t-bbb\n+\t\t+b0b\n \t\t ccc\n \tEOF\n '\n@@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '\n \tdh_test a b <<-EOF\n \t\t@@ -1,3 +1,3 @@\n \t\t aaa\n-\t\t-b${CW}b${CR}b\n-\t\t+b${CW}0${CR}b\n+\t\t-bbb\n+\t\t+b0b\n \t\t+ccc\n \tEOF\n '\n@@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '\n \techo \"unic${o_stroke}de\" >b &&\n \tdh_test a b <<-EOF\n \t\t@@ -1 +1 @@\n-\t\t-unic${CW}${o_accent}${CR}de\n-\t\t+unic${CW}${o_stroke}${CR}de\n+\t\t-unic${o_accent}de\n+\t\t+unic${o_stroke}de\n \tEOF\n '\n \n@@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '\n \techo \"unico${combine_circum}de\" >b &&\n \tdh_test a b <<-EOF\n \t\t@@ -1 +1 @@\n-\t\t-unic${CW}o${combine_accent}${CR}de\n-\t\t+unic${CW}o${combine_circum}${CR}de\n+\t\t-unico${combine_accent}de\n+\t\t+unico${combine_circum}de\n \tEOF\n '\n \n@@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '\n \t+++ b/file\n \t@@ -1,3 +1,3 @@\n \t before\n-\t-the ${CW}old${CR} line\n-\t+the ${CW}new${CR} line\n+\t-the old line\n+\t+the new line\n \t -leading dash\n \tEOF\n \tgit log --graph -p -1 | \"$DIFF_HIGHLIGHT\" >actual.raw &&\n-\ttrim_graph actual &&\n+\ttrim_graph actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539696","messageId":"20260323060213.GF10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 6/8] diff-highlight: test color config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:13Z","receivedAt":"2026-03-23T06:02:14Z","isPatch":true,"body":"We added configurable colors long ago in bca45fbc1f (diff-highlight:\nallow configurable colors, 2014-11-20), but never actually tested it.\nSince we'll be touching the color code in a moment, this is a good time\nto beef up the tests.\n\nNote that we cover both the highlight/reset style used by the default\ncolors, as well as the normal/highlight style added by that commit\n(which was previously totally untested).\n\nSigned-off-by: Jeff King \n---\n .../diff-highlight/t/t9400-diff-highlight.sh | 28 +++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh\nindex 4f3d55a26e..b38fe2196a 100755\n--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh\n+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh\n@@ -350,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' '\n \tEOF\n '\n \n+test_expect_success 'configure set/reset colors' '\n+\ttest_config color.diff-highlight.oldhighlight bold &&\n+\ttest_config color.diff-highlight.oldreset nobold &&\n+\ttest_config color.diff-highlight.newhighlight italic &&\n+\ttest_config color.diff-highlight.newreset noitalic &&\n+\techo \"prefix a suffix\" >a &&\n+\techo \"prefix b suffix\" >b &&\n+\tdh_test a b <<-\\EOF\n+\t@@ -1 +1 @@\n+\t-prefix a suffix\n+\t+prefix b suffix\n+\tEOF\n+'\n+\n+test_expect_success 'configure normal/highlight colors' '\n+\ttest_config color.diff-highlight.oldnormal red &&\n+\ttest_config color.diff-highlight.oldhighlight magenta &&\n+\ttest_config color.diff-highlight.newnormal green &&\n+\ttest_config color.diff-highlight.newhighlight yellow &&\n+\techo \"prefix a suffix\" >a &&\n+\techo \"prefix b suffix\" >b &&\n+\tdh_test a b <<-\\EOF\n+\t@@ -1 +1 @@\n+\t-prefix a suffix\n+\t+prefix b suffix\n+\tEOF\n+'\n+\n test_done\n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539697","messageId":"20260323060215.GG10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 7/8] diff-highlight: allow module callers to pass in color config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:15Z","receivedAt":"2026-03-23T06:02:16Z","isPatch":true,"body":"From: Scott Baker \n\nUsers of the module may want to pass in their own color config for a few\nobvious reasons:\n\n - they are pulling the config from different variables than\n diff-highlight itself uses\n\n - they are loading the config in a more efficient way (say, by parsing\n git-config --list) and don't want to incur the six (!) git-config\n calls that DiffHighlight.pm runs to check all config\n\nLet's allow users of the module to pass in the color config, and\nlazy-load it when needed if they haven't.\n\nSigned-off-by: Scott Baker \nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/DiffHighlight.pm | 41 +++++++++++++++++--------\n contrib/diff-highlight/README | 6 ++++\n 2 files changed, 35 insertions(+), 12 deletions(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex a5e5de3b18..96369eadf9 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -9,18 +9,11 @@ package DiffHighlight;\n \n my $NULL = File::Spec->devnull();\n \n-# Highlight by reversing foreground and background. You could do\n-# other things like bold or underline if you prefer.\n-my @OLD_HIGHLIGHT = (\n-\tcolor_config('color.diff-highlight.oldnormal'),\n-\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n-\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n-);\n-my @NEW_HIGHLIGHT = (\n-\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n-\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n-\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n-);\n+# The color theme is initially set to nothing here to allow outside callers\n+# to set the colors for their application. If nothing is sent in we use\n+# colors from git config in load_color_config().\n+our @OLD_HIGHLIGHT = ();\n+our @NEW_HIGHLIGHT = ();\n \n my $RESET = \"\\x1b[m\";\n my $COLOR = qr/\\x1b\\[[0-9;]*m/;\n@@ -170,6 +163,29 @@ sub show_hunk {\n \t$line_cb->(@queue);\n }\n \n+sub load_color_config {\n+\t# If the colors were NOT set from outside this module we load them on-demand\n+\t# from the git config. Note that only one of elements 0 and 2 in each\n+\t# array is used (depending on whether you are doing set/unset on an\n+\t# attribute, or specifying normal vs highlighted coloring). So we use\n+\t# element 1 as our check for whether colors were passed in; it should\n+\t# always be set if you want highlighting to do anything.\n+\tif (!defined $OLD_HIGHLIGHT[1]) {\n+\t\t@OLD_HIGHLIGHT = (\n+\t\t\tcolor_config('color.diff-highlight.oldnormal'),\n+\t\t\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n+\t\t\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n+\t\t);\n+\t}\n+\tif (!defined $NEW_HIGHLIGHT[1]) {\n+\t\t@NEW_HIGHLIGHT = (\n+\t\t\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n+\t\t\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n+\t\t\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n+\t\t);\n+\t};\n+}\n+\n sub highlight_pair {\n \tmy @a = split_line(shift);\n \tmy @b = split_line(shift);\n@@ -218,6 +234,7 @@ sub highlight_pair {\n \t}\n \n \tif (is_pair_interesting(\\@a, $pa, $sa, \\@b, $pb, $sb)) {\n+\t\tload_color_config();\n \t\treturn highlight_line(\\@a, $pa, $sa, \\@OLD_HIGHLIGHT),\n \t\t highlight_line(\\@b, $pb, $sb, \\@NEW_HIGHLIGHT);\n \t}\ndiff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README\nindex 9c89146fb0..ed8d876a18 100644\n--- a/contrib/diff-highlight/README\n+++ b/contrib/diff-highlight/README\n@@ -138,6 +138,12 @@ Your script may set up one or more of the following variables:\n processing a logical chunk of input). The default function flushes\n stdout.\n \n+ - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these\n+ arrays specify the normal, highlighted, and reset colors (in that order)\n+ for old/new lines. If unset, values will be retrieved by calling `git\n+ config` (see \"Color Config\" above). Note that these should be the literal\n+ color bytes (starting with an ANSI escape code), not color names.\n+\n The script may then feed lines, one at a time, to DiffHighlight::handle_line().\n When lines are done processing, they will be fed to $line_cb. Note that\n DiffHighlight may queue up many input lines (to analyze a whole hunk)\n-- \n2.53.0.1051.ga14e96f895\n\n"},{"id":"539698","messageId":"20260323060218.GH10482@coredump.intra.peff.net","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"[PATCH v2 8/8] diff-highlight: fetch all config with one process","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-23T06:02:18Z","receivedAt":"2026-03-23T06:02:19Z","isPatch":true,"body":"When diff-highlight was written, there was no way to fetch multiple\nconfig keys _and_ have them interpreted as colors. So we were stuck\nwith either invoking git-config once for each config key, or fetching\nthem all and converting human-readable color names into ANSI codes\nourselves.\n\nI chose the former, but it means that diff-highlight kicks off 6\ngit-config processes (even if you haven't configured anything, it has to\ncheck each one).\n\nBut since Git 2.18.0, we can do:\n\n git config --type=color --get-regexp=^color\\.diff-highlight\\.\n\nto get all of them in one shot.\n\nNote that any callers which pass in colors directly to the module via\n@OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are\nunaffected; those colors suppress any config lookup we'd do ourselves.\n\nYou can see the effect like:\n\n # diff-highlight suppresses git-config's stderr, so dump\n # trace through descriptor 3\n git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null\n\nwhich drops from 6 lines down to 1.\n\nSigned-off-by: Jeff King \n---\n contrib/diff-highlight/DiffHighlight.pm | 28 ++++++++++++++++++-------\n 1 file changed, 20 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm\nindex 96369eadf9..abe457882e 100644\n--- a/contrib/diff-highlight/DiffHighlight.pm\n+++ b/contrib/diff-highlight/DiffHighlight.pm\n@@ -131,9 +131,21 @@ sub highlight_stdin {\n # of it being used in other settings. Let's handle our own\n # fallback, which means we will work even if git can't be run.\n sub color_config {\n+\tour $cached_config;\n \tmy ($key, $default) = @_;\n-\tmy $s = `git config --get-color $key 2>$NULL`;\n-\treturn length($s) ? $s : $default;\n+\n+\tif (!defined $cached_config) {\n+\t\t$cached_config = {};\n+\t\tmy $data = `git config --type=color --get-regexp '^color\\.diff-highlight\\.' 2>$NULL`;\n+\t\tfor my $line (split /\\n/, $data) {\n+\t\t\tmy ($key, $color) = split ' ', $line, 2;\n+\t\t\t$key =~ s/^color\\.diff-highlight\\.// or next;\n+\t\t\t$cached_config->{$key} = $color;\n+\t\t}\n+\t}\n+\n+\tmy $s = $cached_config->{$key};\n+\treturn defined($s) ? $s : $default;\n }\n \n sub show_hunk {\n@@ -172,16 +184,16 @@ sub load_color_config {\n \t# always be set if you want highlighting to do anything.\n \tif (!defined $OLD_HIGHLIGHT[1]) {\n \t\t@OLD_HIGHLIGHT = (\n-\t\t\tcolor_config('color.diff-highlight.oldnormal'),\n-\t\t\tcolor_config('color.diff-highlight.oldhighlight', \"\\x1b[7m\"),\n-\t\t\tcolor_config('color.diff-highlight.oldreset', \"\\x1b[27m\")\n+\t\t\tcolor_config('oldnormal'),\n+\t\t\tcolor_config('oldhighlight', \"\\x1b[7m\"),\n+\t\t\tcolor_config('oldreset', \"\\x1b[27m\")\n \t\t);\n \t}\n \tif (!defined $NEW_HIGHLIGHT[1]) {\n \t\t@NEW_HIGHLIGHT = (\n-\t\t\tcolor_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]),\n-\t\t\tcolor_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]),\n-\t\t\tcolor_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2])\n+\t\t\tcolor_config('newnormal', $OLD_HIGHLIGHT[0]),\n+\t\t\tcolor_config('newhighlight', $OLD_HIGHLIGHT[1]),\n+\t\t\tcolor_config('newreset', $OLD_HIGHLIGHT[2])\n \t\t);\n \t};\n }\n-- \n2.53.0.1051.ga14e96f895\n"},{"id":"539759","messageId":"xmqqfr5q5wm7.fsf@gitster.g","threadId":"65313","inReplyTo":"20260323060139.GA10215@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/8] some diff-highlight tweaks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-23T16:38:56Z","receivedAt":"2026-03-23T16:38:59Z","isPatch":true,"body":"Jeff King writes:\n\n> Here's a re-roll based on the review from Yuchen. The two changes are:\n>\n> 1. Added a missing &&-chain in patch 3 (which cascades into patch 6).\n>\n> 2. Avoid length(undef), since old perl versions will warn about it.\n>\n> Patch list and range diff below.\n\nEverything looks as expected from watching the discussion from\nthe sideline. Looking good.\n\nWill queue and mark the topic for 'next'.\n\nThanks.\n"},{"id":"539808","messageId":"acI0PcVF2wbjvGva@pks.im","threadId":"65313","inReplyTo":"xmqqfr5q5wm7.fsf@gitster.g","subject":"Re: [PATCH v2 0/8] some diff-highlight tweaks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-24T06:50:37Z","receivedAt":"2026-03-24T06:50:45Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:38:56AM -0700, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > Here's a re-roll based on the review from Yuchen. The two changes are:\n> >\n> > 1. Added a missing &&-chain in patch 3 (which cascades into patch 6).\n> >\n> > 2. Avoid length(undef), since old perl versions will warn about it.\n> >\n> > Patch list and range diff below.\n> \n> Everything looks as expected from watching the discussion from\n> the sideline. Looking good.\n> \n> Will queue and mark the topic for 'next'.\n\nI just read through the series and couldn't find anything wrong. That\nbeing said, my Perl skills are severely lacking, so my assessment may\nnot be worth much :)\n\nThanks!\n\nPatrick\n"}]}