RFC: git diff colorization idea

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

Wincent Colaiuta <win@wincent.com> writes:

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.

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

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

Wincent Colaiuta <win@wincent.com> writes:

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

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

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.

Wincent Colaiuta <win@wincent.com> writes:

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

[...]

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

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

+1 from me