{"thread":{"id":"29351","subject":"git diff --word-diff problem","startedAt":"2012-01-12T09:05:01Z","lastAt":"2012-01-12T19:23:59Z","messageCount":3,"participants":["Ivan Shirokoff","Thomas Rast","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"182421","messageId":"4F0EA23D.3010603@yandex-team.ru","threadId":"29351","inReplyTo":null,"subject":"git diff --word-diff problem","fromName":"Ivan Shirokoff","fromEmail":"shirokoff@yandex-team.ru","sentAt":"2012-01-12T09:05:01Z","receivedAt":"2012-01-12T09:05:01Z","isPatch":false,"sender":{"key":"shirokoff@yandex-team.ru","avatar":null},"body":"Hello.\nI've got a couple of generated files with obfuscated code.\nI want to word-diff them just to make sure that everything is right.\n\nThe thing is when word-diff gets oneline file without newline at the end \nit compares it with regular line by line diff.\n\nHere is an example. Two one line files generated with perl -e \"print 'a \n'x10\" > file\n\ngit diff --word-diff=plain file1 file2\ndiff --git a/file1 b/file2\nindex 3526254..0515a63 100644\n--- a/file1\n+++ b/file2\n@@ -1 +1 @@\n[- a a a a a a a a a a-]\n  No newline at end of file\n  {+a a a a a ab a a a a+}\n\nGit shows that the whole line is different.\nAnd if I add newlines to that files everything works just as expected\n\ngit diff --word-diff=plain file1 file2\ndiff --git a/file1 b/file2\nindex 1756d83..1ec45b9 100644\n--- a/file1\n+++ b/file2\n@@ -1,2 +1,2 @@\n  a a a a a [-a -]{+ab +}a a a a\n\nIs that a bug or I've missed explanation in docs?\n\n-- \nIvan Shirokoff\n"},{"id":"182427","messageId":"902665ee053876c2684f5b935ee4f81e77122802.1326366909.git.trast@student.ethz.ch","threadId":"29351","inReplyTo":"4F0EA23D.3010603@yandex-team.ru","subject":"[PATCH] word-diff: ignore '\\ No newline at eof' marker","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-01-12T11:15:33Z","receivedAt":"2012-01-12T11:15:33Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The word-diff logic accumulates + and - lines until another line type\nappears (normally [ @\\]), at which point it generates the word diff.\nThis is usually correct, but it breaks when the preimage does not have\na newline at EOF:\n\n  $ printf \"%s\" \"a a a\" >a\n  $ printf \"%s\\n\" \"a ab a\" >b\n  $ git diff --no-index --word-diff a b\n  diff --git 1/a 2/b\n  index 9f68e94..6a7c02f 100644\n  --- 1/a\n  +++ 2/b\n  @@ -1 +1 @@\n  [-a a a-]\n   No newline at end of file\n  {+a ab a+}\n\nBecause of the order of the lines in a unified diff\n\n  @@ -1 +1 @@\n  -a a a\n  \\ No newline at end of file\n  +a ab a\n\nthe '\\' line flushed the buffers, and the - and + lines were never\nmatched with each other.\n\nA proper fix would defer such markers until the end of the hunk.\nHowever, word-diff is inherently whitespace-ignoring, so as a cheap\nfix simply ignore the marker (and hide it from the output).\n\nWe use a prefix match for '\\ ' to parallel the logic in\napply.c:parse_fragment().  We currently do not localize this string\n(just accept other variants of it in git-apply), but this should be\nfuture-proof.\n\nNoticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n diff.c                |    8 ++++++++\n t/t4034-diff-words.sh |   14 ++++++++++++++\n 2 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a65223a..996cc60 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\t\tdiff_words_append(line, len,\n \t\t\t\t\t  &ecbdata->diff_words->plus);\n \t\t\treturn;\n+\t\t} else if (!prefixcmp(line, \"\\\\ \")) {\n+\t\t\t/*\n+\t\t\t * Silently eat the \"no newline at eof\" marker\n+\t\t\t * (we are diffing without regard to\n+\t\t\t * whitespace anyway), and defer processing:\n+\t\t\t * more '+' lines could be after it.\n+\t\t\t */\n+\t\t\treturn;\n \t\t}\n \t\tdiff_words_flush(ecbdata);\n \t\tif (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 6f1e5a2..5c20121 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '\n \tword_diff --word-diff=plain\n '\n \n+test_expect_success 'word-diff with no newline at EOF' '\n+\tcat >expect <<-\\EOF &&\n+\tdiff --git a/pre b/post\n+\tindex 7bf316e..3dd0303 100644\n+\t--- a/pre\n+\t+++ b/post\n+\t@@ -1 +1 @@\n+\ta a [-a-]{+ab+} a a\n+\tEOF\n+\tprintf \"%s\" \"a a a a a\" >pre &&\n+\tprintf \"%s\" \"a a ab a a\" >post &&\n+\tword_diff --word-diff=plain\n+'\n+\n test_done\n-- \n1.7.9.rc0.168.g3847c\n"},{"id":"182458","messageId":"7vty40208g.fsf@alter.siamese.dyndns.org","threadId":"29351","inReplyTo":"902665ee053876c2684f5b935ee4f81e77122802.1326366909.git.trast@student.ethz.ch","subject":"Re: [PATCH] word-diff: ignore '\\ No newline at eof' marker","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-12T19:23:59Z","receivedAt":"2012-01-12T19:23:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast <trast@student.ethz.ch> writes:\n\n> A proper fix would defer such markers until the end of the hunk.\n> However, word-diff is inherently whitespace-ignoring, so as a cheap\n> fix simply ignore the marker (and hide it from the output).\n\nSounds like a very sensible simplification of the issue.\n\n> We use a prefix match for '\\ ' to parallel the logic in\n> apply.c:parse_fragment().  We currently do not localize this string\n> (just accept other variants of it in git-apply), but this should be\n> future-proof.\n>\n> Noticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>\n> Signed-off-by: Thomas Rast <trast@student.ethz.ch>\n> ---\n>\n>  diff.c                |    8 ++++++++\n>  t/t4034-diff-words.sh |   14 ++++++++++++++\n>  2 files changed, 22 insertions(+), 0 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index a65223a..996cc60 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n>  \t\t\tdiff_words_append(line, len,\n>  \t\t\t\t\t  &ecbdata->diff_words->plus);\n>  \t\t\treturn;\n> +\t\t} else if (!prefixcmp(line, \"\\\\ \")) {\n> +\t\t\t/*\n> +\t\t\t * Silently eat the \"no newline at eof\" marker\n> +\t\t\t * (we are diffing without regard to\n> +\t\t\t * whitespace anyway), and defer processing:\n> +\t\t\t * more '+' lines could be after it.\n> +\t\t\t */\n> +\t\t\treturn;\n>  \t\t}\n>  \t\tdiff_words_flush(ecbdata);\n\nIt took me a while to realize \"defer processing\" in the comment was meant\nto justify the placement of the new block _before_ this flush. Perhaps\nrephrasing it to \"return without calling diff_words_flush()\" would make it\nmore readable?\n\nOtherwise the patch looks good.\n\nThanks.\n\n>  \t\tif (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {\n> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\n> index 6f1e5a2..5c20121 100755\n> --- a/t/t4034-diff-words.sh\n> +++ b/t/t4034-diff-words.sh\n> @@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '\n>  \tword_diff --word-diff=plain\n>  '\n>  \n> +test_expect_success 'word-diff with no newline at EOF' '\n> +\tcat >expect <<-\\EOF &&\n> +\tdiff --git a/pre b/post\n> +\tindex 7bf316e..3dd0303 100644\n> +\t--- a/pre\n> +\t+++ b/post\n> +\t@@ -1 +1 @@\n> +\ta a [-a-]{+ab+} a a\n> +\tEOF\n> +\tprintf \"%s\" \"a a a a a\" >pre &&\n> +\tprintf \"%s\" \"a a ab a a\" >post &&\n> +\tword_diff --word-diff=plain\n> +'\n> +\n>  test_done\n"}]}