Re: [PATCH 1/1] diff-highlight: highlight range-diff
From: Jeff King <hidden>
Date: 2020-01-28 07:52:16
On Sun, Dec 29, 2019 at 03:49:50PM +0000, Jack Bates via GitGitGadget wrote:
From: Jack Bates <redacted> Piping `git range-diff` through diff-highlight currently has no effect, for two reasons:
Sorry for the slow review; this got overlooked over the holidays.
1. There are ANSI escapes before and after the `@@` hunk headers (when
color is enabled) which diff-highlight fails to match. One solution
is to match both escapes (`/^$COLOR*\@\@$COLOR* /`). This patch
drops the trailing space from the existing pattern instead.Hmm. Matching $COLOR on either side would be stricter. In particular, with your patch I think we'd match "@@@", undoing 3dbfe2b8ae (diff-highlight: avoid highlighting combined diffs, 2016-08-31).
2. Unlike `git log`, `git range-diff` diffs are indented, which
diff-highlight also fails to match. This patch allows hunk headers
preceded by any amount of whitespace, and then skips past that
indentation when parsing subsequent lines, by reusing the machinery
that handles the --graph output.This also means we'd start trying to highlight diffs that are inside commit messages. That might not be the end of the world. You can see some examples in git.git by doing: git log | /path/to/original/diff-highlight >old git log | /path/to/your/new/diff-highlight >new diff -u old new
quoted hunk ↗ jump to hunk
diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index e2589922a6..74f53e53c9 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm@@ -90,7 +90,8 @@ sub handle_line { if (!$in_hunk) { $line_cb->($orig); - $in_hunk = /^$COLOR*\@\@ /; + $in_hunk = /^( *)$COLOR*\@\@/;
There's a similar regex a few lines below to decide of we should remain in a hunk. Would you need to modify that, too?
+ $graph_indent = length($1);
This throws away the existing $graph_indent. I know we wouldn't have a range-diff mixed with a graph, but I think it breaks normal graph usage. At least "make test" in contrib/diff-highlight seems to complain, and adding "--graph -p" to the "git log" invocations above shows that we're not highlighting a bunch of cases (perhaps all?). -Peff