Re: [PATCH] diff-highlight: Work for multiline changes too

Jeff King [off-list ref] writes:

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'.

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)?

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.

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