{"thread":{"id":"21675","subject":"[PATCH] No diff -b/-w output for all-whitespace changes","startedAt":"2009-11-19T21:12:24Z","lastAt":"2009-11-19T22:25:14Z","messageCount":4,"participants":["Greg Bacon","Junio C Hamano","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127929","messageId":"1258665144-26520-1-git-send-email-gbacon@dbresearch.net","threadId":"21675","inReplyTo":null,"subject":"[PATCH] No diff -b/-w output for all-whitespace changes","fromName":"Greg Bacon","fromEmail":"gbacon@dbresearch.net","sentAt":"2009-11-19T21:12:24Z","receivedAt":"2009-11-19T21:12:24Z","isPatch":true,"sender":{"key":"gbacon@dbresearch.net","avatar":"https://gravatar.com/avatar/5a2159cfb8b094827dd626b7d9b8d244c03ebdac365ff5c6721e8519a4760981?d=mp&s=160"},"body":"Change git-diff's whitespace-ignoring modes to generate\noutput only if a non-empty patch results, which git-apply\nrejects.\n\nUpdate the tests to look for the new behavior.\n\nSigned-off-by: Greg Bacon <gbacon@dbresearch.net>\n---\n diff.c                     |   35 +++++++++++++++++++++++++++--------\n t/t4015-diff-whitespace.sh |   14 ++++++++++++--\n 2 files changed, 39 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 0d7f5ea..108de23 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -189,6 +189,7 @@ struct emit_callback {\n \tstruct diff_words_data *diff_words;\n \tint *found_changesp;\n \tFILE *file;\n+\tstruct strbuf *header;\n };\n \n static int count_lines(const char *data, int size)\n@@ -755,6 +756,11 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n \tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n \n+\tif (ecbdata->header) {\n+\t\tfprintf(ecbdata->file, \"%s\", ecbdata->header->buf);\n+\t\tstrbuf_reset(ecbdata->header);\n+\t\tecbdata->header = NULL;\n+\t}\n \t*(ecbdata->found_changesp) = 1;\n \n \tif (ecbdata->label_path[0]) {\n@@ -1561,6 +1567,7 @@ static void builtin_diff(const char *name_a,\n \tconst char *reset = diff_get_color_opt(o, DIFF_RESET);\n \tconst char *a_prefix, *b_prefix;\n \tconst char *textconv_one = NULL, *textconv_two = NULL;\n+\tstruct strbuf header = STRBUF_INIT;\n \n \tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n \t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n@@ -1595,25 +1602,26 @@ static void builtin_diff(const char *name_a,\n \tb_two = quote_two(b_prefix, name_b + (*name_b == '/'));\n \tlbl[0] = DIFF_FILE_VALID(one) ? a_one : \"/dev/null\";\n \tlbl[1] = DIFF_FILE_VALID(two) ? b_two : \"/dev/null\";\n-\tfprintf(o->file, \"%sdiff --git %s %s%s\\n\", set, a_one, b_two, reset);\n+\tstrbuf_addf(&header, \"%sdiff --git %s %s%s\\n\", set, a_one, b_two, reset);\n \tif (lbl[0][0] == '/') {\n \t\t/* /dev/null */\n-\t\tfprintf(o->file, \"%snew file mode %06o%s\\n\", set, two->mode, reset);\n+\t\tstrbuf_addf(&header, \"%snew file mode %06o%s\\n\", set, two->mode, reset);\n \t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tfprintf(o->file, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n \t}\n \telse if (lbl[1][0] == '/') {\n-\t\tfprintf(o->file, \"%sdeleted file mode %06o%s\\n\", set, one->mode, reset);\n+\t\tstrbuf_addf(&header, \"%sdeleted file mode %06o%s\\n\", set, one->mode, reset);\n \t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tfprintf(o->file, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n \t}\n \telse {\n \t\tif (one->mode != two->mode) {\n-\t\t\tfprintf(o->file, \"%sold mode %06o%s\\n\", set, one->mode, reset);\n-\t\t\tfprintf(o->file, \"%snew mode %06o%s\\n\", set, two->mode, reset);\n+\t\t\tstrbuf_addf(&header, \"%sold mode %06o%s\\n\", set, one->mode, reset);\n+\t\t\tstrbuf_addf(&header, \"%snew mode %06o%s\\n\", set, two->mode, reset);\n \t\t}\n \t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tfprintf(o->file, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\n \t\t/*\n \t\t * we do not run diff between different kind\n \t\t * of objects.\n@@ -1623,6 +1631,8 @@ static void builtin_diff(const char *name_a,\n \t\tif (complete_rewrite &&\n \t\t    (textconv_one || !diff_filespec_is_binary(one)) &&\n \t\t    (textconv_two || !diff_filespec_is_binary(two))) {\n+\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\tstrbuf_reset(&header);\n \t\t\temit_rewrite_diff(name_a, name_b, one, two,\n \t\t\t\t\t\ttextconv_one, textconv_two, o);\n \t\t\to->found_changes = 1;\n@@ -1640,6 +1650,8 @@ static void builtin_diff(const char *name_a,\n \t\tif (mf1.size == mf2.size &&\n \t\t    !memcmp(mf1.ptr, mf2.ptr, mf1.size))\n \t\t\tgoto free_ab_and_return;\n+\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\tstrbuf_reset(&header);\n \t\tif (DIFF_OPT_TST(o, BINARY))\n \t\t\temit_binary_diff(o->file, &mf1, &mf2);\n \t\telse\n@@ -1656,6 +1668,11 @@ static void builtin_diff(const char *name_a,\n \t\tstruct emit_callback ecbdata;\n \t\tconst struct userdiff_funcname *pe;\n \n+\t\tif (!DIFF_XDL_TST(o, WHITESPACE_FLAGS)) {\n+\t\t\tfprintf(o->file, \"%s\", header.buf);\n+\t\t\tstrbuf_reset(&header);\n+\t\t}\n+\n \t\tif (textconv_one) {\n \t\t\tsize_t size;\n \t\t\tmf1.ptr = run_textconv(textconv_one, one, &size);\n@@ -1685,6 +1702,7 @@ static void builtin_diff(const char *name_a,\n \t\tif (ecbdata.ws_rule & WS_BLANK_AT_EOF)\n \t\t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n \t\tecbdata.file = o->file;\n+\t\tecbdata.header = header.len ? &header : NULL;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n \t\txecfg.interhunkctxlen = o->interhunkcontext;\n@@ -1729,6 +1747,7 @@ static void builtin_diff(const char *name_a,\n \t}\n \n  free_ab_and_return:\n+\tstrbuf_release(&header);\n \tdiff_free_filespec_data(one);\n \tdiff_free_filespec_data(two);\n \tfree(a_one);\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 8dd147d..90f3342 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -93,8 +93,6 @@ git diff > out\n test_expect_success 'another test, without options' 'test_cmp expect out'\n \n cat << EOF > expect\n-diff --git a/x b/x\n-index d99af23..8b32fb5 100644\n EOF\n git diff -w > out\n test_expect_success 'another test, with -w' 'test_cmp expect out'\n@@ -386,6 +384,18 @@ test_expect_success 'checkdiff allows new blank lines' '\n \tgit diff --check\n '\n \n+cat <<EOF >expect\n+EOF\n+test_expect_success 'whitespace-only changes not reported' '\n+\tgit reset --hard &&\n+\techo >x \"hello world\" &&\n+\tgit add x &&\n+\tgit commit -m \"hello 1\" &&\n+\techo >x \"hello  world\" &&\n+\tgit diff -b >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'combined diff with autocrlf conversion' '\n \n \tgit reset --hard &&\n-- \n1.6.5.3\n"},{"id":"127934","messageId":"7vbpiy9nl0.fsf@alter.siamese.dyndns.org","threadId":"21675","inReplyTo":"1258665144-26520-1-git-send-email-gbacon@dbresearch.net","subject":"Re: [PATCH] No diff -b/-w output for all-whitespace changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-19T22:06:19Z","receivedAt":"2009-11-19T22:06:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Bacon <gbacon@dbresearch.net> writes:\n\n> Change git-diff's whitespace-ignoring modes to generate\n> output only if a non-empty patch results, which git-apply\n> rejects.\n>\n> Update the tests to look for the new behavior.\n>\n> Signed-off-by: Greg Bacon <gbacon@dbresearch.net>\n\nThanks.\n\nI didn't read the patch carefully, but I think this is the right thing to\ndo and is in the same sprit as jc/1.7.0-diff-whitespace-only-status topic.\n"},{"id":"127935","messageId":"200911192314.41542.trast@student.ethz.ch","threadId":"21675","inReplyTo":"1258665144-26520-1-git-send-email-gbacon@dbresearch.net","subject":"Re: [PATCH] No diff -b/-w output for all-whitespace changes","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-11-19T22:14:35Z","receivedAt":"2009-11-19T22:14:35Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Greg Bacon wrote:\n> Change git-diff's whitespace-ignoring modes to generate\n> output only if a non-empty patch results, which git-apply\n> rejects.\n\nJudging from the test, this parses as\n\n  Change git-diff's whitespace-ignoring modes to generate\n  output only if a non-(empty patch, which git-apply\n  rejects) results.\n\nwhich is a bit weird, isn't it? :-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"127937","messageId":"4B05C5CA.6090408@dbresearch.net","threadId":"21675","inReplyTo":"200911192314.41542.trast@student.ethz.ch","subject":"Re: [PATCH] No diff -b/-w output for all-whitespace changes","fromName":"Greg Bacon","fromEmail":"gbacon@dbresearch.net","sentAt":"2009-11-19T22:25:14Z","receivedAt":"2009-11-19T22:25:14Z","isPatch":true,"sender":{"key":"gbacon@dbresearch.net","avatar":"https://gravatar.com/avatar/5a2159cfb8b094827dd626b7d9b8d244c03ebdac365ff5c6721e8519a4760981?d=mp&s=160"},"body":"Thomas Rast wrote\n\n> Judging from the test, this parses as\n> \n>   Change git-diff's whitespace-ignoring modes to generate\n>   output only if a non-(empty patch, which git-apply\n>   rejects) results.\n> \n> which is a bit weird, isn't it? :-)\n\nYes, rotten wording. I meant git-apply rejects empty patches,\nso let's not do that!\n\nI chose this route rather than making git-apply more\nforgiving because as currently implemented, git-diff is not\nentirely ignoring whitespace when commanded to do so.\n\nGreg\n"}]}