RFC: git diff colorization idea

RFC: git diff colorization idea

[Wincent Colaiuta]

Hi,

Lately I've been wishing that Git's diff output were colorized in a  
way that combines the standard line-by-line colorizing with the word- 
by-word colorizing you get with --color-words.

Pictures speak louder than words, so here are some to show what I mean:

http://www.flickr.com/photos/wincent-colaiuta/sets/72157612877491482/

There you'll find:

1. A couple of sample hunks colorized in the current, standard way

2. The same hunks colorized with "--color-words"

3. The same hunks as they would be colorized if you could take the  
standard colorization (1) and augment it with per-word highlighting  
from (2)

(that last hunk would probably look better with a different regex  
defining what a "word" is; in reality all that happened was that  
"rename|" got added to the line, so there's no need to highlight more  
than that).

This is not a new idea; it's something that I find myself wanting due  
to experience with colorization available in a number of different  
diff viewers. Here are some more sample shots showing how different  
viewers do it, with varying degrees of prettiness/ugliness:

- Meld: http://meld.sourceforge.net/meld_file1.png

- kdiff: http://kdiff3.sourceforge.net/doc/letter_by_letter.png

- Apple FileMerge: http://homepage.mac.com/kelleherk/iblog/C711669388/E464913847/Media/compare_db_filemerge.jpg

No doubt there are others, but you get the idea.

Would people be interested in seeing this feature go in? I've already  
started snooping around diff.c seeing what would need to be done. From  
what I can tell it would require a new command-line switch (seeing as  
"--color" plus "--color-words" already means something), and probably  
two new customizable color slots (such as color.diff.new.word,  
color.diff.old.word).

The approach I was thinking of taking was just grabbing the diff_words  
info produced when --color-words is passed and using it to augment  
specialized versions of emit_line() and emit_add_line().

I'm also thinking that perhaps a per-character approach might be  
useful here instead of a per-word one (it would make that last hunk  
look better in the mock-up screenshot that I posted); if I go the per- 
character route then that suggests that "--color-chars" might be the  
right option name, and the color slots would then be  
color.diff.new.char and color.diff.old.char.

Any feedback or suggestions before I get in too deep?

Cheers,
Wincent

Re: RFC: git diff colorization idea

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> I'm also thinking that perhaps a per-character approach might be
> useful here instead of a per-word one (it would make that last hunk
> look better in the mock-up screenshot that I posted); if I go the per-
> character route then that suggests that "--color-chars" might be the
> right option name, and the color slots would then be
> color.diff.new.char and color.diff.old.char.
>
> Any feedback or suggestions before I get in too deep?

I personally find your "prposal" picture too loud to my eye.

I would have expected you to propose something like this:

  | _git_remote ()
  | {
  |-    local subcommands="add rm show prune <red>update</red>"
  |+    local subcommands="add <green>rename</green> rm show prune"
  |     local subcommand=$(__git_find_subcommand "$subcommands")"
  |     if ...

if the patch were to remove update and add rename, that is.  If there is
no deletion but only insertion, you would only see green.

And that way you do not need new color slots.  You can use new color and
old color as before, and the coloring will be done to highlight only the
parts that really matter.

If you were to go this route, I suspect that showing the unchanged part on
the preimage line in light gray might make sense, like:

  | _git_remote ()
  | {
  |-    <gray>local subcommands="add rm show prune<gray> <red>update</red>"
  |+    local subcommands="add <green>rename</green> rm show prune"
  |     local subcommand=$(__git_find_subcommand "$subcommands")"
  |     if ...

because there will be the same chars/words on the postimage line anyway.

Just 2c from somebody who does not like colors very much.

Re: RFC: git diff colorization idea

[Wincent Colaiuta]

El 23/1/2009, a las 1:32, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> I'm also thinking that perhaps a per-character approach might be
>> useful here instead of a per-word one (it would make that last hunk
>> look better in the mock-up screenshot that I posted); if I go the  
>> per-
>> character route then that suggests that "--color-chars" might be the
>> right option name, and the color slots would then be
>> color.diff.new.char and color.diff.old.char.
>>
>> Any feedback or suggestions before I get in too deep?
>
> I personally find your "prposal" picture too loud to my eye.

Yes, mine too. I wouldn't actually use those colors in practice.  
(Doubly so because the "removed" color looks like the "whitespace  
error" color.)

I'll whip something up with non-garish defaults.

Cheers,
Wincent

Re: RFC: git diff colorization idea

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

>>> Any feedback or suggestions before I get in too deep?
>>
>> I personally find your "prposal" picture too loud to my eye.
>
> Yes, mine too. I wouldn't actually use those colors in practice.  
> (Doubly so because the "removed" color looks like the "whitespace  
> error" color.)

I did not mean that your choice of colors is loud.  Not at all.  What I
meant was the use of color _everywhere_ makes it too loud, and also the
use of _larger number of_ colors makes it too loud..  In other words, you
are painting the output more than what the current output does, and that
made me find it too loud, no matter what the choice of colors are.

You might have misunderstood my illustration, but what I meant to suggest
was to leave most of the characters on both - and + lines in monochrome
(or whatever the "normal" is), and paint _only_ the words that are
different.  The outcome would become _less_ colorful than the current
"whole -/+ lines are painted" output, but "only different words are
coloured; the words that are unmodified are left uncoloured, without
distracting the eyes."  Much less louder than even the current one, yet
giving more information.

As -/+ has a marker value, I think colouring them may be Ok, too.  That
would make something like this:

  | {
  |<red>-</red>    <gray>local subcommands="add rm show prune<gray> <red>update</red>"
  |<green>+</green>    local subcommands="add <green>rename</green> rm show prune"
  |     if ...

Re: RFC: git diff colorization idea

[Nanako Shiraishi]

Quoting Junio C Hamano <gitster@pobox.com>:

> If you were to go this route, I suspect that showing the unchanged part on
> the preimage line in light gray might make sense, like:
>
>   | _git_remote ()
>   | {
>   |-    <gray>local subcommands="add rm show prune<gray> <red>update</red>"
>   |+    local subcommands="add <green>rename</green> rm show prune"
>   |     local subcommand=$(__git_find_subcommand "$subcommands")"
>   |     if ...
>
> because there will be the same chars/words on the postimage line anyway.

I think this makes a lot more sense than any of the screenshot
WIncent prepared on his web pages, and it is a much easier output
for users to spot which word is different by not coloring the
unchanged word at all.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: RFC: git diff colorization idea

[Jakub Narebski]

Wincent Colaiuta <win@wincent.com> writes:

> Lately I've been wishing that Git's diff output were colorized in a
> way that combines the standard line-by-line colorizing with the word-
> by-word colorizing you get with --color-words.

I like this idea, also because Emacs ediff / ediff3 / emerge uses it,
from what I understand under the name of 'refinement'.

> 
> Pictures speak louder than words, so here are some to show what I mean:
> 
> http://www.flickr.com/photos/wincent-colaiuta/sets/72157612877491482/
>
[...]

There was some discussion in this thread on how to do this, whether
with --refine / --color-chars we shoud ony highlight differences, or
whether for example use reverse (i.e. background green or background
red), or other red / other green, or perhaps bold, or perhaps
underline to highlight regions in line which differ

> - Meld: http://meld.sourceforge.net/meld_file1.png

Not extremly good example, as it uses equivalent of context diff
format (-,+,!) with added, removed and _changed_ lines, and not
unified diff format (only added / removed lines).

> Would people be interested in seeing this feature go in? [...]

+1 from me
-- 
Jakub Narebski
Poland
ShadeHawk on #git