diff-highlight: highlight range-diff
cover diff-highlight: highlight range-diff
Piping git range-diff through diff-highlight currently has no effect, for two reasons:
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.
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.
Jack Bates (1):
diff-highlight: highlight range-diff
contrib/diff-highlight/DiffHighlight.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-684%2Fjablko%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-684/jablko/patch-1-v1
Pull-Request: https://github.com/git/git/pull/684
Signature · 1 line
gitgitgadget
PATCH 1/1 diff-highlight: highlight range-diff
From: Jack Bates <[email protected]>
Piping git range-diff through diff-highlight currently has no effect, for two reasons:
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.
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.
--- contrib/diff-highlight/DiffHighlight.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
contrib/diff-highlight/DiffHighlight.pm +2 -1
@@ -90,7 +90,8 @@ sub handle_line { if (!$in_hunk) { $line_cb->($orig); - $in_hunk = /^$COLOR*\@\@ /; + $in_hunk = /^( *)$COLOR*\@\@/; + $graph_indent = length($1); } elsif (/^$COLOR*-/) { push @removed, $orig;
Signature · 1 line
gitgitgadget
Quoted 4 lines · Jack Bates via GitGitGadget
From: Jack Bates <[email protected]>
Piping
git range-diffthrough diff-highlight currently has no effect, for two reasons:
Sorry for the slow review; this got overlooked over the holidays.
Quoted 4 lines
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).
Quoted 5 lines
2. Unlike
git log,git range-diffdiffs 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
contrib/diff-highlight/DiffHighlight.pm sub handle_line { · line 93
Quoted 10 lines
contrib/diff-highlight/DiffHighlight.pm +1
-1@@ -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?
contrib/diff-highlight/DiffHighlight.pm sub handle_line { · line 94
Quoted 1 line
contrib/diff-highlight/DiffHighlight.pm +1
-0+ $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