Re: [PATCH] diff-highlight: Work for multiline changes too
From: Jeff King <hidden>
Date: 2016-06-15 22:53:04
On Mon, Feb 13, 2012 at 04:05:05PM -0800, Junio C Hamano wrote:
Jeff King [off-list ref] writes:quoted
I ended up pulling your changes out into a few distinct commits. That made it easier for me to review and understand what was going on (and hopefully ditto for other reviewers, or people who end up bisecting or reading the log later). I'll post that series in a moment.This is all nice. As I am lazy and this is a long neglected contrib/ material I didn't even know it existed ;-), I am tempted to apply them directly on top of 'master'.
Yeah, given the contrib nature, I'm comfortable just applying. I wanted to wait on the attribution question from Michał before sending a final version, though.
quoted hunk ↗ jump to hunk
This shows the first hunk of your "diff-highlight: refactor to prepare for multi-line hunks" like this to me, by the way.@@ -23,7 +23,7 @@ while (<>) { $window[2] =~ /^$COLOR*\+/ && $window[3] !~ /^$COLOR*\+/) { print shift @window;{- show_pair}(shift @window, shift @window); + show_{hunk}(shift @window, shift @window); } else { print shift @window; Is this intended, or is setting "diff.color.old = red reverse" not supported (without the custom configuration, the leading blank on the old line is not highlighted)?
No, it's not intended, and should be:
- show-{pair}...
+ show+{hunk}...
(and appears that way with my config). I suspect it is because the
default highlight color is a subset of your "old" configured color. It
is ANSI "reverse video", but sadly that does not work as a toggle (i.e.,
it does not reverse your reverse, but simply is a no-op).
I chose reverse because I like the way it looks, and because it should
Just Work if people have selected alternate colors (I never dreamed
somebody would use "reverse" all the time, as I find it horribly ugly.
But to each his own). We could read the highlight color from
diff.highlight{Old,New}. That would also let people highlight in purple
or something if they care.
-Peff