[PATCH 0/3] fix "git diff --color-words -U0"

[PATCH 0/3] fix "git diff --color-words -U0"

[Markus Heidelberg]

The wrong output roughly is as follows:

@@ -a,b +c,d @@
@@ -e,f +g,h @@
some red, green and black text
more red, green and black text

When it should be:

@@ -a,b +c,d @@
some red, green and black text
@@ -e,f +g,h @@
more red, green and black text


Markus Heidelberg (3):
  t4034-diff-words: add a test for word diff without context
  diff: move the handling of the hunk header after the changed lines
  diff: fix the location of hunk headers for "git diff --color-words
    -U0"

 diff.c                |   40 +++++++++++++++++++++++-----------------
 t/t4034-diff-words.sh |   20 ++++++++++++++++++++
 2 files changed, 43 insertions(+), 17 deletions(-)

[PATCH 1/3] t4034-diff-words: add a test for word diff without context

[Markus Heidelberg]

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 t/t4034-diff-words.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 4508eff..82240cf 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -68,6 +68,26 @@ cat > expect <<\EOF
 <WHITE>index 330b04f..5ed8eff 100644<RESET>
 <WHITE>--- a/pre<RESET>
 <WHITE>+++ b/post<RESET>
+<BROWN>@@ -1 +1 @@<RESET>
+<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
+<BROWN>@@ -3,0 +4,4 @@ a = b + c<RESET>
+
+<GREEN>aa = a<RESET>
+
+<GREEN>aeff = aeff * ( aaa )<RESET>
+EOF
+
+test_expect_failure 'word diff without context' '
+
+	word_diff --color-words --unified=0
+
+'
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
 <BROWN>@@ -1,3 +1,7 @@<RESET>
 h(4),<GREEN>hh<RESET>[44]
 <RESET>
-- 
1.6.5.2.86.g61663

[PATCH 2/3] diff: move the handling of the hunk header after the changed lines

[Markus Heidelberg]

In preparation for a special case handling of colored word diff without
context.

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 diff.c |   41 +++++++++++++++++++++--------------------
 1 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/diff.c b/diff.c
index b0c7e61..067e5a0 100644
--- a/diff.c
+++ b/diff.c
@@ -771,17 +771,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		len = 1;
 	}
 
-	if (line[0] == '@') {
-		len = sane_truncate_line(ecbdata, line, len);
-		find_lno(line, ecbdata);
-		emit_line(ecbdata->file,
-			  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
-			  reset, line, len);
-		if (line[len-1] != '\n')
-			putc('\n', ecbdata->file);
-		return;
-	}
-
 	if (len < 1) {
 		emit_line(ecbdata->file, reset, reset, line, len);
 		return;
@@ -796,17 +785,18 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->plus);
 			return;
+		} else if (line[0] == ' ') {
+			if (ecbdata->diff_words->minus.text.size ||
+			    ecbdata->diff_words->plus.text.size)
+				diff_words_show(ecbdata->diff_words);
+			line++;
+			len--;
+			emit_line(ecbdata->file, plain, reset, line, len);
+			return;
 		}
-		if (ecbdata->diff_words->minus.text.size ||
-		    ecbdata->diff_words->plus.text.size)
-			diff_words_show(ecbdata->diff_words);
-		line++;
-		len--;
-		emit_line(ecbdata->file, plain, reset, line, len);
-		return;
 	}
 
-	if (line[0] != '+') {
+	if (line[0] == ' ' || line[0] == '-') {
 		const char *color =
 			diff_get_color(ecbdata->color_diff,
 				       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
@@ -814,10 +804,21 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		if (line[0] == ' ')
 			ecbdata->lno_in_postimage++;
 		emit_line(ecbdata->file, color, reset, line, len);
-	} else {
+		return;
+	} else if (line[0] == '+') {
 		ecbdata->lno_in_postimage++;
 		emit_add_line(reset, ecbdata, line + 1, len - 1);
+		return;
 	}
+
+	/* line[0] == '@' */
+	len = sane_truncate_line(ecbdata, line, len);
+	find_lno(line, ecbdata);
+	emit_line(ecbdata->file,
+		  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
+		  reset, line, len);
+	if (line[len-1] != '\n')
+		putc('\n', ecbdata->file);
 }
 
 static char *pprint_rename(const char *a, const char *b)
-- 
1.6.5.2.86.g61663

[PATCH 3/3] diff: fix the location of hunk headers for "git diff --color-words -U0"

[Markus Heidelberg]

Colored word diff without context lines firstly printed all the hunk
headers among each other and then printed the diff.

Because the word diff cannot be calculated before the end of the diff
(added/removed lines) hunk, it was calculated directly before first line
of context after the diff. But this didn't work if there was no context.
In this case the diff wasn't printed in fn_out_consume(), but entirely
in free_diff_words_data(). This also led to calculate the colored diff
from the whole diff in one swoop instead of calculating it in several
independent steps (one step per hunk).

We now calculate and print the word diff directly before the next hunk
header. The word diff of the last hunk is still printed in
free_diff_words_data().

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 diff.c                |   13 +++++++++----
 t/t4034-diff-words.sh |    2 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 067e5a0..e95fe9b 100644
--- a/diff.c
+++ b/diff.c
@@ -785,10 +785,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->plus);
 			return;
-		} else if (line[0] == ' ') {
-			if (ecbdata->diff_words->minus.text.size ||
-			    ecbdata->diff_words->plus.text.size)
-				diff_words_show(ecbdata->diff_words);
+		}
+		/*
+		 * If line[0] == '@' then this prints the content of the
+		 * previous hunk, necessary for 0-context.
+		 */
+		if (ecbdata->diff_words->minus.text.size ||
+		    ecbdata->diff_words->plus.text.size)
+			diff_words_show(ecbdata->diff_words);
+		if (line[0] == ' ') {
 			line++;
 			len--;
 			emit_line(ecbdata->file, plain, reset, line, len);
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 82240cf..21db6e9 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -77,7 +77,7 @@ cat > expect <<\EOF
 <GREEN>aeff = aeff * ( aaa )<RESET>
 EOF
 
-test_expect_failure 'word diff without context' '
+test_expect_success 'word diff without context' '
 
 	word_diff --color-words --unified=0
 
-- 
1.6.5.2.86.g61663

[PATCH] diff --color-words -U0: fix the location of hunk headers

[Johannes Schindelin]

Colored word diff without context lines firstly printed all the hunk
headers among each other and then printed the diff.

This was due to the code relying on getting at least one context line at
the end of each hunk, where the colored words would be flushed (it is
done that way to be able to ignore rewrapped lines).

Noticed by Markus Heidelberg.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I would strongly prefer this fix instead of your 2/3 and 3/3.

 diff.c                |    6 ++++++
 t/t4034-diff-words.sh |    2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 51b5dbb..4eafaf5 100644
--- a/diff.c
+++ b/diff.c
@@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	for (i = 0; i < len && line[i] == '@'; i++)
 		;
 	if (2 <= i && i < len && line[i] == ' ') {
+		/* flush --color-words even for --unified=0 */
+		if (ecbdata->diff_words &&
+		    (ecbdata->diff_words->minus.text.size ||
+		     ecbdata->diff_words->plus.text.size))
+			diff_words_show(ecbdata->diff_words);
+
 		ecbdata->nparents = i - 1;
 		len = sane_truncate_line(ecbdata, line, len);
 		emit_line(ecbdata->file,
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 82240cf..21db6e9 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -77,7 +77,7 @@ cat > expect <<\EOF
 <GREEN>aeff = aeff * ( aaa )<RESET>
 EOF
 
-test_expect_failure 'word diff without context' '
+test_expect_success 'word diff without context' '
 
 	word_diff --color-words --unified=0
 
-- 
1.6.4.GIT

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Markus Heidelberg]

Johannes Schindelin, 29.10.2009:
> 
> 	I would strongly prefer this fix instead of your 2/3 and 3/3.
> 
>  diff.c                |    6 ++++++
>  t/t4034-diff-words.sh |    2 +-
>  2 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index 51b5dbb..4eafaf5 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>  	for (i = 0; i < len && line[i] == '@'; i++)
>  		;
>  	if (2 <= i && i < len && line[i] == ' ') {
> +		/* flush --color-words even for --unified=0 */
> +		if (ecbdata->diff_words &&
> +		    (ecbdata->diff_words->minus.text.size ||
> +		     ecbdata->diff_words->plus.text.size))
> +			diff_words_show(ecbdata->diff_words);
> +
>  		ecbdata->nparents = i - 1;
>  		len = sane_truncate_line(ecbdata, line, len);
>  		emit_line(ecbdata->file,

This seems to apply before commit b8d9c1a (diff.c: the builtin_diff()
deals with only two-file comparison, 2009-09-03).

Indeed my initial fix was in the same fashion:

@@ -772,6 +772,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
        }

        if (line[0] == '@') {
+               if (ecbdata->diff_words) {
+                       /*
+                        * The content of the previous hunk, necessary for
+                        * 0-context.
+                        */
+                       if (ecbdata->diff_words->minus.text.size ||
+                           ecbdata->diff_words->plus.text.size)
+                               diff_words_show(ecbdata->diff_words);
+               }
                len = sane_truncate_line(ecbdata, line, len);
                find_lno(line, ecbdata);
                emit_line(ecbdata->file,

But then I thought I should not put the diff output from --color-words
into the block that deals with the hunk header, but save another place
where diff_words_show() is called.

Markus

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Johannes Schindelin]

Hi,

On Thu, 29 Oct 2009, Markus Heidelberg wrote:

> Johannes Schindelin, 29.10.2009:
> > 
> > 	I would strongly prefer this fix instead of your 2/3 and 3/3.
> > 
> >  diff.c                |    6 ++++++
> >  t/t4034-diff-words.sh |    2 +-
> >  2 files changed, 7 insertions(+), 1 deletions(-)
> > 
> > diff --git a/diff.c b/diff.c
> > index 51b5dbb..4eafaf5 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -656,6 +656,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> >  	for (i = 0; i < len && line[i] == '@'; i++)
> >  		;
> >  	if (2 <= i && i < len && line[i] == ' ') {
> > +		/* flush --color-words even for --unified=0 */
> > +		if (ecbdata->diff_words &&
> > +		    (ecbdata->diff_words->minus.text.size ||
> > +		     ecbdata->diff_words->plus.text.size))
> > +			diff_words_show(ecbdata->diff_words);
> > +
> >  		ecbdata->nparents = i - 1;
> >  		len = sane_truncate_line(ecbdata, line, len);
> >  		emit_line(ecbdata->file,
> 
> This seems to apply before commit b8d9c1a (diff.c: the builtin_diff()
> deals with only two-file comparison, 2009-09-03).

Yes, sorry, for some reason I worked on a machine where I do not work from 
junio's next, but my own fork (which is outdated due to lack of time).

> Indeed my initial fix was in the same fashion:
> 
> @@ -772,6 +772,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>         }
> 
>         if (line[0] == '@') {
> +               if (ecbdata->diff_words) {
> +                       /*
> +                        * The content of the previous hunk, necessary for
> +                        * 0-context.
> +                        */
> +                       if (ecbdata->diff_words->minus.text.size ||
> +                           ecbdata->diff_words->plus.text.size)
> +                               diff_words_show(ecbdata->diff_words);
> +               }
>                 len = sane_truncate_line(ecbdata, line, len);
>                 find_lno(line, ecbdata);
>                 emit_line(ecbdata->file,
> 
> But then I thought I should not put the diff output from --color-words
> into the block that deals with the hunk header, but save another place
> where diff_words_show() is called.

I found this paragraph, as well as the patches 2/3 and 3/3, hard to 
follow.

And besides, flushing in that block is the correct thing to do.  The 
function diff_words_show() is a function for that exact purpose.

Ciao,
Dscho

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Markus Heidelberg]

Johannes Schindelin, 29.10.2009:
> Hi,
> 
> On Thu, 29 Oct 2009, Markus Heidelberg wrote:
> 
> > Indeed my initial fix was in the same fashion:
> > 
> > @@ -772,6 +772,15 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> >         }
> > 
> >         if (line[0] == '@') {
> > +               if (ecbdata->diff_words) {
> > +                       /*
> > +                        * The content of the previous hunk, necessary for
> > +                        * 0-context.
> > +                        */
> > +                       if (ecbdata->diff_words->minus.text.size ||
> > +                           ecbdata->diff_words->plus.text.size)
> > +                               diff_words_show(ecbdata->diff_words);
> > +               }
> >                 len = sane_truncate_line(ecbdata, line, len);
> >                 find_lno(line, ecbdata);
> >                 emit_line(ecbdata->file,
> > 
> > But then I thought I should not put the diff output from --color-words
> > into the block that deals with the hunk header, but save another place
> > where diff_words_show() is called.
> 
> I found this paragraph, as well as the patches 2/3 and 3/3, hard to 
> follow.

I try to reword:
With 2/3 and 3/3 I wanted to keep --color-words specific code in the
block starting with

	if (ecbdata->diff_words) {

and didn't want to contaminate the block starting with

	if (line[0] == '@') {

with non-hunk-header content.

But I'm not sure what's the better way and am content with either.

> And besides, flushing in that block is the correct thing to do.  The 
> function diff_words_show() is a function for that exact purpose.

Yes, 2/3 and 3/3 just don't introduce a new invocation of this function
at another place in the code.

Markus

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Junio C Hamano]

Markus Heidelberg <markus.heidelberg@web.de> writes:

> I try to reword:
> With 2/3 and 3/3 I wanted to keep --color-words specific code in the
> block starting with
>
> 	if (ecbdata->diff_words) {
>
> and didn't want to contaminate the block starting with
>
> 	if (line[0] == '@') {
>
> with non-hunk-header content.

The contamination was already done long time ago.  The way "color words"
mode piggybacks into the rest of the code is to let the line oriented diff
codepath run, capture the lines to its buffer so that it can split them
into words and compare, and hijack the control flow not to let the line
oriented diff to be output, and in the context of the original design,
Dscho's patch makes sense.

I do think that the way the "color words" logic has to touch line-oriented
codepaths unnecessarily makes it look intrusive; one reason is because it
exposes the state that is only interesting to the "color words" mode to
these places too much.

With a small helper function on top of Dscho's patch, I think the code can
be made a lot more readable.  This way, "consume" codepath is made more
about "here is what we do when we get a line from the line-oriented diff
engine", and we can keep the knowledge of "color words" mode to the
minimum (no more than "here is where we may need to flush the buffer").
The helper hides the implementation details such as how we decide if we
have accumulated words or what we do when we do need to flush.

There is another large-ish "if (ecbdata->diff_words)" codeblock in
fn_out_consume() that peeks into *(ecbdata->diff_words); I think it should
be ejected from the main codepath for the same reason (i.e. readability).
.
Probably we can make a helper function that has only a single caller, like
this:

        static void diff_words_use_line(char *line, unsigned long len,
                                        struct emit_callback *ecbdata,
                                        const char *plain, const char *reset)
        {
                if (line[0] == '-') {
                        diff_words_append(line, len,
                                          &ecbdata->diff_words->minus);
                        return;
                } else if (line[0] == '+') {
                        diff_words_append(line, len,
                                          &ecbdata->diff_words->plus);
                        return;
                }
                diff_words_flush(ecbdata);
                line++;
                len--;
                emit_line(ecbdata->file, plain, reset, line, len);
        }

It would even be cleaner to change "diff_words_append()" function to do
all of the above.

But that is outside the scope of this comment.


 diff.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/diff.c b/diff.c
index b7ecfe3..8c66e4a 100644
--- a/diff.c
+++ b/diff.c
@@ -541,14 +541,18 @@ struct emit_callback {
 	FILE *file;
 };
 
+/* In "color-words" mode, show word-diff of words accumulated in the buffer */
+static void diff_words_flush(struct emit_callback *ecbdata)
+{
+	if (ecbdata->diff_words->minus.text.size ||
+	    ecbdata->diff_words->plus.text.size)
+		diff_words_show(ecbdata->diff_words);
+}
+
 static void free_diff_words_data(struct emit_callback *ecbdata)
 {
 	if (ecbdata->diff_words) {
-		/* flush buffers */
-		if (ecbdata->diff_words->minus.text.size ||
-				ecbdata->diff_words->plus.text.size)
-			diff_words_show(ecbdata->diff_words);
-
+		diff_words_flush(ecbdata);
 		free (ecbdata->diff_words->minus.text.ptr);
 		free (ecbdata->diff_words->minus.orig);
 		free (ecbdata->diff_words->plus.text.ptr);
@@ -656,12 +660,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 	for (i = 0; i < len && line[i] == '@'; i++)
 		;
 	if (2 <= i && i < len && line[i] == ' ') {
-		/* flush --color-words even for --unified=0 */
-		if (ecbdata->diff_words &&
-		    (ecbdata->diff_words->minus.text.size ||
-		     ecbdata->diff_words->plus.text.size))
-			diff_words_show(ecbdata->diff_words);
-
+		if (ecbdata->diff_words)
+			diff_words_flush(ecbdata);
 		ecbdata->nparents = i - 1;
 		len = sane_truncate_line(ecbdata, line, len);
 		emit_line(ecbdata->file,
@@ -691,9 +691,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 					  &ecbdata->diff_words->plus);
 			return;
 		}
-		if (ecbdata->diff_words->minus.text.size ||
-		    ecbdata->diff_words->plus.text.size)
-			diff_words_show(ecbdata->diff_words);
+		diff_words_flush(ecbdata);
 		line++;
 		len--;
 		emit_line(ecbdata->file, plain, reset, line, len);

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Johannes Schindelin]

Hi,

On Fri, 30 Oct 2009, Junio C Hamano wrote:

> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > I try to reword:
> > With 2/3 and 3/3 I wanted to keep --color-words specific code in the
> > block starting with
> >
> > 	if (ecbdata->diff_words) {
> >
> > and didn't want to contaminate the block starting with
> >
> > 	if (line[0] == '@') {
> >
> > with non-hunk-header content.
> 
> The contamination was already done long time ago.

Actually, it was don on purpose.

> diff --git a/diff.c b/diff.c
> index b7ecfe3..8c66e4a 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -541,14 +541,18 @@ struct emit_callback {
>  	FILE *file;
>  };
>  
> +/* In "color-words" mode, show word-diff of words accumulated in the buffer */
> +static void diff_words_flush(struct emit_callback *ecbdata)
> +{
> +	if (ecbdata->diff_words->minus.text.size ||
> +	    ecbdata->diff_words->plus.text.size)
> +		diff_words_show(ecbdata->diff_words);
> +}

Instead of this function, you can check the same at the beginning of 
diff_words_show().

The reason I did not do that was to avoid a full subroutine call, as I 
expected this code path to be very expensive.

Ciao,
Dscho

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The reason I did not do that was to avoid a full subroutine call, as I 
> expected this code path to be very expensive.

This is only done for the "word diff" mode, and my gut feeling is that it
is not such a big issue.  But you can always inline it if it is
performance critical.

The current structure shows how this code evolved.  fn_out_consume() used
to be "this is called every time a line worth of diff becomes ready from
the lower-level diff routine, and here is what we do to output line
oriented diff using that line."

When we introduced the "word diff" mode, we could have done three things.

 * change fn_out_consume() to "this is called every time a line worth of
   diff becomes ready from the lower-level diff routine.  This function
   knows two sets of helpers (one for line-oriented diff, another for word
   diff), and each set has various functions to be called at certain
   places (e.g. hunk header, context, ...).  The function's role is to
   inspect the incoming line, and dispatch appropriate helpers to produce
   either line- or word- oriented diff output."

 * introduce fn_out_consume_word_diff() that is "this is called every time
   a line worth of diff becomes ready from the lower-level diff routine,
   and here is what we do to prepare word oriented diff using that line."
   without touching fn_out_consume() at all.

 * Do neither of the above, and keep fn_out_consume() to "this is called
   every time a line worth of diff becomes ready from the lower-level diff
   routine, and here is what we do to output line oriented diff using that
   line."  but sprinkle a handful of 'are we in word-diff mode?  if so do
   this totally different thing' at random places.

My clean-up "something like this" patch was to at least abstract the
details of "this totally different thing" out from the main codepath, in
order to improve readability.

We can of course refactor this by introducing fn_out_consume_word_diff(),
i.e. the second route above.  Then we do not have to check "are we in word
diff mode" for every line of diff and that would give you even better
performance.

Re: [PATCH] diff --color-words -U0: fix the location of hunk headers

[Johannes Schindelin]

Hi,

On Fri, 30 Oct 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The reason I did not do that was to avoid a full subroutine call, as I 
> > expected this code path to be very expensive.
> 
> This is only done for the "word diff" mode, and my gut feeling is that it
> is not such a big issue.

Yeah, sorry, I should have stated explicitely that I no longer think that 
there is a performance issue.

Ciao,
Dscho

Re: [PATCH 0/3] fix "git diff --color-words -U0"

[Junio C Hamano]

Is this a serious enough breakage that deserves to be fixed in the
maintenance track (1.6.5.X)?

Re: [PATCH 0/3] fix "git diff --color-words -U0"

[Markus Heidelberg]

Junio C Hamano, 28.10.2009:
> Is this a serious enough breakage that deserves to be fixed in the
> maintenance track (1.6.5.X)?

This problem exists since the introduction of this feature over three
years ago and apparently nobody complained so far. So I don't think it's
overly serious.

OTOH, depending on the change the produced diff can be totally wrong.
Found a good example: c5022f576aa583429c245054d8600564b788ff33
Compare "--color-words -U0" with "--color-words -U1".