Re: [PATCH] Add color to git-add--interactive diffs (Take 2: now without spurious line break!)
- From
Wincent Colaiuta <win@wincent.com>
- Date
- Oct 14, 2007, 11:36 UTC
- Message-ID
- <EFADE863-FC59-4A50-B165-9D30D9648B97@wincent.com>
- In-Reply-To
- <1192351494.7226.18.camel@athena>
El 14/10/2007, a las 10:44, Tom Tobin escribió:
> After banging my head against parsing colorized output of git-add- > files, > I gave up and implemented internal colorization keying off of the > color.diff configuration.
Great!
> +sub parse_color {You could simplify the manual escape sequence construction that you're doing here by using Term::ANSIColor like the other patches did. I see that git-send-email.perl uses that module too, so I guess depending on that module is ok.
I also wonder whether the config code should be using the git.pm module like git-send-email.perl and a couple others do (although it would be slower than slurping in all the config in one shot like you do; perhaps there's justification for a new function in git.pm that wraps git-config --get-regexp...).
Show 32 quoted lines
> +sub colorize_head_line {
> + my $line = shift @_;
> + if ($use_color) {
> + # git doesn't colorize these by default, soooo
> + # if ($line =~ /^\+/) {
> + # return parse_color($colorconfig{'color.diff.new'}) . "$line\e
> [m";
> + # }
> + # if ($line =~ /^-/) {
> + # return parse_color($colorconfig{'color.diff.old'}) . "$line\e
> [m";
> + # }
> + return parse_color($colorconfig{'color.diff.meta'}) . "$line\e[m";
> + }
> + return $line;
> +}
> +
> +sub colorize_hunk_line {
> + my $line = shift @_;
> + if ($use_color) {
> + if ($line =~ /^\+/) {
> + return parse_color($colorconfig{'color.diff.new'}) . "$line\e[m";
> + }
> + if ($line =~ /^-/) {
> + return parse_color($colorconfig{'color.diff.old'}) . "$line\e[m";
> + }
> + if ($line =~ /^@@ /) {
> + return parse_color($colorconfig{'color.diff.frag'}) . "$line\e[m";
> + }
> + }
> + return $line;
> +}This is a good start but to completely match the colorized output produced by diff it will need some additional logic; for example, highlighting spurious whitespace. Search for need_highlight_leading_space in diff.c and you'll see that the test is basically for any space which precedes a tab in the leading whitespace on newly inserted lines. In this case the spaces are highlighted using the whitespace color (normally red background).
I don't know when color.diff.commit is ever used in diff output, but perhaps that would need to be handled as well.
Cheers, Wincent