On Mon, Feb 13, 2012 at 05:19:33PM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > 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).
>
> I also find it ugly, but I am on black-letters on white background window,
> and I do not see my terminal's red very well, so it is hard to tell the
> old from the context if I use the "diff.color.old=red" default; that is
> the primary reason for my setting.
I find black-on-white ugly, too, but I have heard some people find it
more readable. You might try setting color.diff.old to "bold red" to
make it more readable. Depending on your terminal, that may end up as a
brighter shade of red.
Configurable colors for diff-highlight would look like the patch below:
diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
index c4404d4..f43832b 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -2,11 +2,13 @@
use warnings FATAL => 'all';
use strict;
+use Git;
+
+my $repo = Git->repository;
+my $color_old = $repo->get_color('color.diff.highlightold', 'reverse');
+my $color_new = $repo->get_color('color.diff.highlightnew', 'reverse');
+my $color_end = $repo->get_color('color.diff.highlightend', 'noreverse');
-# Highlight by reversing foreground and background. You could do
-# other things like bold or underline if you prefer.
-my $HIGHLIGHT = "\x1b[7m";
-my $UNHIGHLIGHT = "\x1b[27m";
my $COLOR = qr/\x1b\[[0-9;]*m/;
my $BORING = qr/$COLOR|\s/;
@@ -128,8 +130,8 @@ sub highlight_pair {
}
if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) {
- return highlight_line(\@a, $pa, $sa),
- highlight_line(\@b, $pb, $sb);
+ return highlight_line(\@a, $pa, $sa, $color_old, $color_end),
+ highlight_line(\@b, $pb, $sb, $color_new, $color_end);
}
else {
return join('', @a),
@@ -144,13 +146,13 @@ sub split_line {
}
sub highlight_line {
- my ($line, $prefix, $suffix) = @_;
+ my ($line, $prefix, $suffix, $highlight, $unhighlight) = @_;
return join('',
@{$line}[0..($prefix-1)],
- $HIGHLIGHT,
+ $highlight,
@{$line}[$prefix..$suffix],
- $UNHIGHLIGHT,
+ $unhighlight,
@{$line}[($suffix+1)..$#$line]
);
}
However, there are two problems:
1. Git's color-parsing support does not understand the "noreverse"
attribute. There is no way to turn off attributes short of doing a
whole "reset". But we don't want to do that here, because we want
whatever other colors were in effect to continue after the
highlight ends. The patch to teach "noreverse" is below (though it
should probably also teach "normal" and "noblink", too).
2. The Git.pm:get_color code requires that we have a repository
object, which means we will die() if we are not in a git
repository. Yet "git config" will do the right thing whether we are
in a repository or not. I would have thought all of the _maybe_self
stuff in Git.pm would handle "Git->get_color" properly, but it
doesn't. That's probably a bug that should be fixed.
I don't especially care about this feature, as I won't use it. But if
you are interested in using diff-highlight and the lack of configurable
colors is blocking you, then I at least know there will be one user and
I don't mind putting a little bit of time into it.
Here's the patch for (1) above.
diff --git a/color.c b/color.c
index e8e2681..b0f53a7 100644
--- a/color.c
+++ b/color.c
@@ -47,9 +47,9 @@ static int parse_color(const char *name, int len)
static int parse_attr(const char *name, int len)
{
- static const int attr_values[] = { 1, 2, 4, 5, 7 };
+ static const int attr_values[] = { 1, 2, 4, 5, 7, 27 };
static const char * const attr_names[] = {
- "bold", "dim", "ul", "blink", "reverse"
+ "bold", "dim", "ul", "blink", "reverse", "noreverse"
};
int i;
for (i = 0; i < ARRAY_SIZE(attr_names); i++) {
@@ -128,7 +128,7 @@ void color_parse_mem(const char *value, int value_len, const char *var,
attr &= ~bit;
if (sep++)
*dst++ = ';';
- *dst++ = '0' + i;
+ dst += sprintf(dst, "%d", i);
}
if (fg >= 0) {
if (sep++)
--
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