On Mon, Feb 13, 2012 at 04:05:05PM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> 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'.
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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html