git diff --word-diff problem

git diff --word-diff problem

[Ivan Shirokoff]

Hello.
I've got a couple of generated files with obfuscated code.
I want to word-diff them just to make sure that everything is right.

The thing is when word-diff gets oneline file without newline at the end 
it compares it with regular line by line diff.

Here is an example. Two one line files generated with perl -e "print 'a 
'x10" > file

git diff --word-diff=plain file1 file2
diff --git a/file1 b/file2
index 3526254..0515a63 100644
--- a/file1
+++ b/file2
@@ -1 +1 @@
[- a a a a a a a a a a-]
  No newline at end of file
  {+a a a a a ab a a a a+}

Git shows that the whole line is different.
And if I add newlines to that files everything works just as expected

git diff --word-diff=plain file1 file2
diff --git a/file1 b/file2
index 1756d83..1ec45b9 100644
--- a/file1
+++ b/file2
@@ -1,2 +1,2 @@
  a a a a a [-a -]{+ab +}a a a a

Is that a bug or I've missed explanation in docs?

-- 
Ivan Shirokoff

[PATCH] word-diff: ignore '\ No newline at eof' marker

[Thomas Rast]

The word-diff logic accumulates + and - lines until another line type
appears (normally [ @\]), at which point it generates the word diff.
This is usually correct, but it breaks when the preimage does not have
a newline at EOF:

  $ printf "%s" "a a a" >a
  $ printf "%s\n" "a ab a" >b
  $ git diff --no-index --word-diff a b
  diff --git 1/a 2/b
  index 9f68e94..6a7c02f 100644
  --- 1/a
  +++ 2/b
  @@ -1 +1 @@
  [-a a a-]
   No newline at end of file
  {+a ab a+}

Because of the order of the lines in a unified diff

  @@ -1 +1 @@
  -a a a
  \ No newline at end of file
  +a ab a

the '\' line flushed the buffers, and the - and + lines were never
matched with each other.

A proper fix would defer such markers until the end of the hunk.
However, word-diff is inherently whitespace-ignoring, so as a cheap
fix simply ignore the marker (and hide it from the output).

We use a prefix match for '\ ' to parallel the logic in
apply.c:parse_fragment().  We currently do not localize this string
(just accept other variants of it in git-apply), but this should be
future-proof.

Noticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 diff.c                |    8 ++++++++
 t/t4034-diff-words.sh |   14 ++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index a65223a..996cc60 100644
--- a/diff.c
+++ b/diff.c
@@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->plus);
 			return;
+		} else if (!prefixcmp(line, "\\ ")) {
+			/*
+			 * Silently eat the "no newline at eof" marker
+			 * (we are diffing without regard to
+			 * whitespace anyway), and defer processing:
+			 * more '+' lines could be after it.
+			 */
+			return;
 		}
 		diff_words_flush(ecbdata);
 		if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 6f1e5a2..5c20121 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '
 	word_diff --word-diff=plain
 '
 
+test_expect_success 'word-diff with no newline at EOF' '
+	cat >expect <<-\EOF &&
+	diff --git a/pre b/post
+	index 7bf316e..3dd0303 100644
+	--- a/pre
+	+++ b/post
+	@@ -1 +1 @@
+	a a [-a-]{+ab+} a a
+	EOF
+	printf "%s" "a a a a a" >pre &&
+	printf "%s" "a a ab a a" >post &&
+	word_diff --word-diff=plain
+'
+
 test_done
-- 
1.7.9.rc0.168.g3847c

Re: [PATCH] word-diff: ignore '\ No newline at eof' marker

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> A proper fix would defer such markers until the end of the hunk.
> However, word-diff is inherently whitespace-ignoring, so as a cheap
> fix simply ignore the marker (and hide it from the output).

Sounds like a very sensible simplification of the issue.

> We use a prefix match for '\ ' to parallel the logic in
> apply.c:parse_fragment().  We currently do not localize this string
> (just accept other variants of it in git-apply), but this should be
> future-proof.
>
> Noticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
>  diff.c                |    8 ++++++++
>  t/t4034-diff-words.sh |   14 ++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index a65223a..996cc60 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>  			diff_words_append(line, len,
>  					  &ecbdata->diff_words->plus);
>  			return;
> +		} else if (!prefixcmp(line, "\\ ")) {
> +			/*
> +			 * Silently eat the "no newline at eof" marker
> +			 * (we are diffing without regard to
> +			 * whitespace anyway), and defer processing:
> +			 * more '+' lines could be after it.
> +			 */
> +			return;
>  		}
>  		diff_words_flush(ecbdata);

It took me a while to realize "defer processing" in the comment was meant
to justify the placement of the new block _before_ this flush. Perhaps
rephrasing it to "return without calling diff_words_flush()" would make it
more readable?

Otherwise the patch looks good.

Thanks.

>  		if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
> index 6f1e5a2..5c20121 100755
> --- a/t/t4034-diff-words.sh
> +++ b/t/t4034-diff-words.sh
> @@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '
>  	word_diff --word-diff=plain
>  '
>  
> +test_expect_success 'word-diff with no newline at EOF' '
> +	cat >expect <<-\EOF &&
> +	diff --git a/pre b/post
> +	index 7bf316e..3dd0303 100644
> +	--- a/pre
> +	+++ b/post
> +	@@ -1 +1 @@
> +	a a [-a-]{+ab+} a a
> +	EOF
> +	printf "%s" "a a a a a" >pre &&
> +	printf "%s" "a a ab a a" >post &&
> +	word_diff --word-diff=plain
> +'
> +
>  test_done