{"thread":{"id":"20849","subject":"[PATCH 0/9] War on blank-at-eof","startedAt":"2009-09-04T10:55:09Z","lastAt":"2009-09-06T06:13:51Z","messageCount":14,"participants":["Junio C Hamano","Johannes Sixt","Thell Fowler"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"122413","messageId":"1252061718-11579-1-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":null,"subject":"[PATCH 0/9] War on blank-at-eof","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:09Z","receivedAt":"2009-09-04T10:55:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We had quite inconsistent handing of patches that add new blank lines at\nthe end of file, and this miniseries is about fixing it.\n\nPatch 1 is a fix to an ancient bug introduced by v1.5.5-rc0~156^2~11.\n\nPatch 2 fixes a bug that is even older---I suspect it dates back to the\nvery first change that introduced the feature, but I did not bother to\ndig.\n\nPatch 4 (Patch 3 is a preliminary refactoring used by it) is about the\ndiscrepancy between \"--whitespace=fix\" and \"--whitespace=warn\".  The\nblank-at-eof error was silently fixed but never diagnosed, which has\nbeen one of the long-standing itch of mine to fix.\n\nPatch 5 corrects the definition of blank-at-eof.  If a patch adds an\nnon-empty line that consists solely of whitespaces at the end of file, we\nshould diagnose and strip it just line a new empty line.  After all, both\nare blank lines.\n\nPatch 6 is a simple code reduction I noticed while preparing this series;\nit can be a standalone patch, but it is obvious enough to be here.\n\nPatches 7 and 8 address \"git diff --check\", which had roughly the same\nlogic as the --whitespace=fix.  It shared the same problems the earlier\nparts of the series fixed for \"git apply\".\n\nPatch 9 is about \"diff --color\" to paint blank-at-eof as error, which we\ndid not do so far because it was too cumbersome.  This has been another\none of the long-standing itch of mine to fix.\n\nThe series applies to v1.6.0.6-87-g82d97da; merging the result to 'master'\nneeds some conflict resolution.\n\n 1 apply --whitespace=fix: fix handling of blank lines at the eof\n 2 apply --whitespace=fix: detect new blank lines at eof correctly\n 3 apply.c: split check_whitespace() into two\n 4 apply --whitespace=warn/error: diagnose blank at EOF\n 5 apply --whitespace: warn blank but not necessarily empty lines at EOF\n 6 diff.c: the builtin_diff() deals with only two-file comparison\n 7 diff --whitespace=warn/error: obey blank-at-eof\n 8 diff --whitespace=warn/error: fix blank-at-eof check\n 9 diff --color: color blank-at-eof\n\n Documentation/config.txt   |    2 +\n builtin-apply.c            |   61 +++++++++++++++-------\n cache.h                    |    3 +-\n diff.c                     |  119 +++++++++++++++++++++++++++++---------------\n t/t4015-diff-whitespace.sh |   11 +++-\n t/t4019-diff-wserror.sh    |   11 ++++-\n t/t4124-apply-ws-rule.sh   |   80 +++++++++++++++++++++++++++++\n ws.c                       |    6 ++\n 8 files changed, 230 insertions(+), 63 deletions(-)\n"},{"id":"122416","messageId":"1252061718-11579-2-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/9] apply --whitespace=fix: fix handling of blank lines at the eof","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:10Z","receivedAt":"2009-09-04T10:55:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"b94f2ed (builtin-apply.c: make it more line oriented, 2008-01-26) broke\nthe logic used to detect if a hunk adds blank lines at the end of the\nfile.  With the new code after that commit:\n\n - img holds the contents of the file that the hunk is being applied to;\n\n - preimage has the lines the hunk expects to be in img; and\n\n - postimage has the lines the hunk wants to update the part in img that\n   corresponds to preimage with.\n\nand we need to compare if the last line of preimage (not postimage)\nmatches the last line of img to see if the hunk applies at the end of the\nfile.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-apply.c          |    2 +-\n t/t4124-apply-ws-rule.sh |   29 +++++++++++++++++++++++++++++\n 2 files changed, 30 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 7a1ff04..5b5bde4 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -2069,7 +2069,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \tif (applied_pos >= 0) {\n \t\tif (ws_error_action == correct_ws_error &&\n \t\t    new_blank_lines_at_end &&\n-\t\t    postimage.nr + applied_pos == img->nr) {\n+\t\t    preimage.nr + applied_pos == img->nr) {\n \t\t\t/*\n \t\t\t * If the patch application adds blank lines\n \t\t\t * at the end, and if the patch applies at the\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex f83322e..6898722 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -148,4 +148,33 @@ do\n \tdone\n done\n \n+\n+test_expect_success 'blank at EOF with --whitespace=fix (1)' '\n+\t: these can fail depending on what we did before\n+\tgit config --unset core.whitespace\n+\trm -f .gitattributes\n+\n+\t{ echo a; echo b; echo c; } >one &&\n+\tgit add one &&\n+\t{ echo a; echo b; echo c; } >expect &&\n+\t{ cat expect; echo; } >one &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=fix patch &&\n+\ttest_cmp expect one\n+'\n+\n+test_expect_success 'blank at EOF with --whitespace=fix (2)' '\n+\t{ echo a; echo b; echo c; } >one &&\n+\tgit add one &&\n+\t{ echo a; echo c; } >expect &&\n+\t{ cat expect; echo; echo; } >one &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=fix patch &&\n+\ttest_cmp expect one\n+'\n+\n test_done\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122414","messageId":"1252061718-11579-3-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:11Z","receivedAt":"2009-09-04T10:55:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The command tries to strip blank lines at the end of the file added by a\npatch.  However, if the original ends with blank lines, often the patch\nhunk ends like this:\n\n    @@ -l,5 +m,7 @@$\n    _context$\n    _context$\n    -deleted$\n    +$\n    +$\n    +$\n    _$\n    _$\n\nwhere _ stands for SP and $ shows a end-of-line.  This example patch adds\nthree trailing blank lines, but the code fails to notice it, because it\nonly pays attention to added blank lines at the very end of the hunk.  In\nthis example, the three added blank lines do not appear textually at the\nend in the patch, even though you can see that they are indeed added at\nthe end, if you rearrange the diff like this:\n\n    @@ -l,5 +m,7 @@$\n    _context$\n    _context$\n    -deleted$\n    _$\n    _$\n    +$\n    +$\n    +$\n\nFix this by not resetting the number of (candidate) added blank lines at\nthe end when the loop sees a context line that is empty.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-apply.c          |    6 ++++++\n t/t4124-apply-ws-rule.sh |   12 ++++++++++++\n 2 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 5b5bde4..c5e4048 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1913,6 +1913,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t\tint len = linelen(patch, size);\n \t\tint plen, added;\n \t\tint added_blank_line = 0;\n+\t\tint is_blank_context = 0;\n \n \t\tif (!len)\n \t\t\tbreak;\n@@ -1945,8 +1946,11 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t\t\t*new++ = '\\n';\n \t\t\tadd_line_info(&preimage, \"\\n\", 1, LINE_COMMON);\n \t\t\tadd_line_info(&postimage, \"\\n\", 1, LINE_COMMON);\n+\t\t\tis_blank_context = 1;\n \t\t\tbreak;\n \t\tcase ' ':\n+\t\t\tif (plen && patch[1] == '\\n')\n+\t\t\t\tis_blank_context = 1;\n \t\tcase '-':\n \t\t\tmemcpy(old, patch + 1, plen);\n \t\t\tadd_line_info(&preimage, old, plen,\n@@ -1986,6 +1990,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t\t}\n \t\tif (added_blank_line)\n \t\t\tnew_blank_lines_at_end++;\n+\t\telse if (is_blank_context)\n+\t\t\t;\n \t\telse\n \t\t\tnew_blank_lines_at_end = 0;\n \t\tpatch += len;\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex 6898722..ba2b7f9 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -177,4 +177,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (2)' '\n \ttest_cmp expect one\n '\n \n+test_expect_success 'blank at EOF with --whitespace=fix (3)' '\n+\t{ echo a; echo b; echo; } >one &&\n+\tgit add one &&\n+\t{ echo a; echo c; echo; } >expect &&\n+\t{ cat expect; echo; echo; } >one &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=fix patch &&\n+\ttest_cmp expect one\n+'\n+\n test_done\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122417","messageId":"1252061718-11579-4-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/9] apply.c: split check_whitespace() into two","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:12Z","receivedAt":"2009-09-04T10:55:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This splits the logic to record the presence of whitespace errors out of\nthe check_whitespace() function, which checks and then records.  The new\nfunction, record_ws_error(), can be used by the blank-at-eof check that\ndoes not use ws_check() logic to report its findings in the same output\nformat.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-apply.c |   24 +++++++++++++++---------\n 1 files changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex c5e4048..80ddf55 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1055,23 +1055,29 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc\n \treturn -1;\n }\n \n-static void check_whitespace(const char *line, int len, unsigned ws_rule)\n+static void record_ws_error(unsigned result, const char *line, int len, int linenr)\n {\n \tchar *err;\n-\tunsigned result = ws_check(line + 1, len - 1, ws_rule);\n+\n \tif (!result)\n \t\treturn;\n \n \twhitespace_error++;\n \tif (squelch_whitespace_errors &&\n \t    squelch_whitespace_errors < whitespace_error)\n-\t\t;\n-\telse {\n-\t\terr = whitespace_error_string(result);\n-\t\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n-\t\t\tpatch_input_file, linenr, err, len - 2, line + 1);\n-\t\tfree(err);\n-\t}\n+\t\treturn;\n+\n+\terr = whitespace_error_string(result);\n+\tfprintf(stderr, \"%s:%d: %s.\\n%.*s\\n\",\n+\t\tpatch_input_file, linenr, err, len, line);\n+\tfree(err);\n+}\n+\n+static void check_whitespace(const char *line, int len, unsigned ws_rule)\n+{\n+\tunsigned result = ws_check(line + 1, len - 1, ws_rule);\n+\n+\trecord_ws_error(result, line + 1, len - 2, linenr);\n }\n \n /*\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122415","messageId":"1252061718-11579-5-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/9] apply --whitespace=warn/error: diagnose blank at EOF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:13Z","receivedAt":"2009-09-04T10:55:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git apply\" strips new blank lines at EOF under --whitespace=fix option,\nbut neigher --whitespace=warn nor --whitespace=error paid any attention to\nthese errors.\n\nIntroduce a new whitespace error class, blank-at-eof, to make the\nwhitespace error handling more consistent.\n\nThe patch adds a new \"linenr\" field to the struct fragment in order to\nrecord which line the hunk started in the input file, but this is needed\nsolely for reporting purposes.  The detection of this class of whitespace\nerrors cannot be done while parsing a patch like we do for all the other\nclasses of whitespace errors.  It instead has to wait until we find where\nto apply the hunk, but at that point, we do not have an access to the\noriginal line number in the input file anymore, hence the new field.\n\nDepending on your point of view, this may be a bugfix that makes warn and\nerror in line with fix.  Or you could call it a new feature.  The line\nbetween them is somewhat fuzzy in this case.\n\nStrictly speaking, triggering more errors than before is a change in\nbehaviour that is not backward compatible, even though the reason for the\nchange is because the code was not checking for an error that it should\nhave.  People who do not want added blank lines at EOF to trigger an error\ncan disable the new error class.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |    2 ++\n builtin-apply.c          |   27 ++++++++++++++++++---------\n cache.h                  |    3 ++-\n t/t4124-apply-ws-rule.sh |   26 ++++++++++++++++++++++++++\n ws.c                     |    6 ++++++\n 5 files changed, 54 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 113d9d1..871384e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -389,6 +389,8 @@ core.whitespace::\n   error (enabled by default).\n * `indent-with-non-tab` treats a line that is indented with 8 or more\n   space characters as an error (not enabled by default).\n+* `blank-at-eof` treats blank lines added at the end of file as an error\n+  (enabled by default).\n * `cr-at-eol` treats a carriage-return at the end of line as\n   part of the line terminator, i.e. with it, `trailing-space`\n   does not trigger if the character before such a carriage-return\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 80ddf55..37d3bc0 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -126,6 +126,7 @@ struct fragment {\n \tconst char *patch;\n \tint size;\n \tint rejected;\n+\tint linenr;\n \tstruct fragment *next;\n };\n \n@@ -1193,6 +1194,7 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc\n \t\tint len;\n \n \t\tfragment = xcalloc(1, sizeof(*fragment));\n+\t\tfragment->linenr = linenr;\n \t\tlen = parse_fragment(line, size, patch, fragment);\n \t\tif (len <= 0)\n \t\t\tdie(\"corrupt patch at line %d\", linenr);\n@@ -2079,17 +2081,24 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t}\n \n \tif (applied_pos >= 0) {\n-\t\tif (ws_error_action == correct_ws_error &&\n-\t\t    new_blank_lines_at_end &&\n-\t\t    preimage.nr + applied_pos == img->nr) {\n+\t\tif (new_blank_lines_at_end &&\n+\t\t    preimage.nr + applied_pos == img->nr &&\n+\t\t    (ws_rule & WS_BLANK_AT_EOF) &&\n+\t\t    ws_error_action != nowarn_ws_error) {\n+\t\t\trecord_ws_error(WS_BLANK_AT_EOF, \"+\", 1, frag->linenr);\n+\t\t\tif (ws_error_action == correct_ws_error) {\n+\t\t\t\twhile (new_blank_lines_at_end--)\n+\t\t\t\t\tremove_last_line(&postimage);\n+\t\t\t}\n \t\t\t/*\n-\t\t\t * If the patch application adds blank lines\n-\t\t\t * at the end, and if the patch applies at the\n-\t\t\t * end of the image, remove those added blank\n-\t\t\t * lines.\n+\t\t\t * We would want to prevent write_out_results()\n+\t\t\t * from taking place in apply_patch() that follows\n+\t\t\t * the callchain led us here, which is:\n+\t\t\t * apply_patch->check_patch_list->check_patch->\n+\t\t\t * apply_data->apply_fragments->apply_one_fragment\n \t\t\t */\n-\t\t\twhile (new_blank_lines_at_end--)\n-\t\t\t\tremove_last_line(&postimage);\n+\t\t\tif (ws_error_action == die_on_ws_error)\n+\t\t\t\tapply = 0;\n \t\t}\n \n \t\t/*\ndiff --git a/cache.h b/cache.h\nindex 099a32e..7152fea 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -845,7 +845,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i\n #define WS_SPACE_BEFORE_TAB\t02\n #define WS_INDENT_WITH_NON_TAB\t04\n #define WS_CR_AT_EOL           010\n-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)\n+#define WS_BLANK_AT_EOF        020\n+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)\n extern unsigned whitespace_rule_cfg;\n extern unsigned whitespace_rule(const char *);\n extern unsigned parse_whitespace_rule(const char *);\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex ba2b7f9..89b71e1 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -189,4 +189,30 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '\n \ttest_cmp expect one\n '\n \n+test_expect_success 'blank at EOF with --whitespace=warn' '\n+\t{ echo a; echo b; echo c; } >one &&\n+\tgit add one &&\n+\techo >>one &&\n+\tcat one >expect &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=warn patch 2>error &&\n+\ttest_cmp expect one &&\n+\tgrep \"new blank line at EOF\" error\n+'\n+\n+test_expect_success 'blank at EOF with --whitespace=error' '\n+\t{ echo a; echo b; echo c; } >one &&\n+\tgit add one &&\n+\tcat one >expect &&\n+\techo >>one &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\ttest_must_fail git apply --whitespace=error patch 2>error &&\n+\ttest_cmp expect one &&\n+\tgrep \"new blank line at EOF\" error\n+'\n+\n test_done\ndiff --git a/ws.c b/ws.c\nindex 7a7ff13..d56636b 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -15,6 +15,7 @@ static struct whitespace_rule {\n \t{ \"space-before-tab\", WS_SPACE_BEFORE_TAB },\n \t{ \"indent-with-non-tab\", WS_INDENT_WITH_NON_TAB },\n \t{ \"cr-at-eol\", WS_CR_AT_EOL },\n+\t{ \"blank-at-eof\", WS_BLANK_AT_EOF },\n };\n \n unsigned parse_whitespace_rule(const char *string)\n@@ -113,6 +114,11 @@ char *whitespace_error_string(unsigned ws)\n \t\t\tstrbuf_addstr(&err, \", \");\n \t\tstrbuf_addstr(&err, \"indent with spaces\");\n \t}\n+\tif (ws & WS_BLANK_AT_EOF) {\n+\t\tif (err.len)\n+\t\t\tstrbuf_addstr(&err, \", \");\n+\t\tstrbuf_addstr(&err, \"new blank line at EOF\");\n+\t}\n \treturn strbuf_detach(&err, NULL);\n }\n \n-- \n1.6.4.2.313.g0425f\n"},{"id":"122418","messageId":"1252061718-11579-6-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/9] apply --whitespace: warn blank but not necessarily empty lines at EOF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:14Z","receivedAt":"2009-09-04T10:55:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The whitespace error of adding blank lines at the end of file should\ntrigger if you added a non-empty line at the end, if the contents of the\nline is full of whitespaces.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-apply.c          |    6 ++++--\n t/t4124-apply-ws-rule.sh |   13 +++++++++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 37d3bc0..6662cc4 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1957,7 +1957,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t\t\tis_blank_context = 1;\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tif (plen && patch[1] == '\\n')\n+\t\t\tif (plen && (ws_rule & WS_BLANK_AT_EOF) &&\n+\t\t\t    ws_blank_line(patch + 1, plen, ws_rule))\n \t\t\t\tis_blank_context = 1;\n \t\tcase '-':\n \t\t\tmemcpy(old, patch + 1, plen);\n@@ -1985,7 +1986,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,\n \t\t\t\t      (first == '+' ? 0 : LINE_COMMON));\n \t\t\tnew += added;\n \t\t\tif (first == '+' &&\n-\t\t\t    added == 1 && new[-1] == '\\n')\n+\t\t\t    (ws_rule & WS_BLANK_AT_EOF) &&\n+\t\t\t    ws_blank_line(patch + 1, plen, ws_rule))\n \t\t\t\tadded_blank_line = 1;\n \t\t\tbreak;\n \t\tcase '@': case '\\\\':\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex 89b71e1..b3c3b2c 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -215,4 +215,17 @@ test_expect_success 'blank at EOF with --whitespace=error' '\n \tgrep \"new blank line at EOF\" error\n '\n \n+test_expect_success 'blank but not empty at EOF' '\n+\t{ echo a; echo b; echo c; } >one &&\n+\tgit add one &&\n+\techo \"   \" >>one &&\n+\tcat one >expect &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=warn patch 2>error &&\n+\ttest_cmp expect one &&\n+\tgrep \"new blank line at EOF\" error\n+'\n+\n test_done\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122421","messageId":"1252061718-11579-7-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/9] diff.c: the builtin_diff() deals with only two-file comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:15Z","receivedAt":"2009-09-04T10:55:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The combined diff is implemented in combine_diff() and fn_out_consume()\ncodepath never has to deal with anything but two-file comparison.\n\nDrop nparents from the emit_callback structure and simplify the code.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c |   32 +++++++++-----------------------\n 1 files changed, 9 insertions(+), 23 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 6fea3c0..1eddd59 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -489,7 +489,7 @@ typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);\n \n struct emit_callback {\n \tstruct xdiff_emit_state xm;\n-\tint nparents, color_diff;\n+\tint color_diff;\n \tunsigned ws_rule;\n \tsane_truncate_fn truncate;\n \tconst char **label_path;\n@@ -549,9 +549,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons\n \t\temit_line(ecbdata->file, set, reset, line, len);\n \telse {\n \t\t/* Emit just the prefix, then the rest. */\n-\t\temit_line(ecbdata->file, set, reset, line, ecbdata->nparents);\n-\t\tws_check_emit(line + ecbdata->nparents,\n-\t\t\t      len - ecbdata->nparents, ecbdata->ws_rule,\n+\t\temit_line(ecbdata->file, set, reset, line, 1);\n+\t\tws_check_emit(line + 1, len - 1, ecbdata->ws_rule,\n \t\t\t      ecbdata->file, set, reset, ws);\n \t}\n }\n@@ -576,7 +575,6 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u\n \n static void fn_out_consume(void *priv, char *line, unsigned long len)\n {\n-\tint i;\n \tint color;\n \tstruct emit_callback *ecbdata = priv;\n \tconst char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);\n@@ -598,13 +596,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\tecbdata->label_path[0] = ecbdata->label_path[1] = NULL;\n \t}\n \n-\t/* This is not really necessary for now because\n-\t * this codepath only deals with two-way diffs.\n-\t */\n-\tfor (i = 0; i < len && line[i] == '@'; i++)\n-\t\t;\n-\tif (2 <= i && i < len && line[i] == ' ') {\n-\t\tecbdata->nparents = i - 1;\n+\tif (line[0] == '@') {\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n \t\temit_line(ecbdata->file,\n \t\t\t  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n@@ -614,15 +606,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\treturn;\n \t}\n \n-\tif (len < ecbdata->nparents) {\n+\tif (len < 1) {\n \t\temit_line(ecbdata->file, reset, reset, line, len);\n \t\treturn;\n \t}\n \n \tcolor = DIFF_PLAIN;\n-\tif (ecbdata->diff_words && ecbdata->nparents != 1)\n-\t\t/* fall back to normal diff */\n-\t\tfree_diff_words_data(ecbdata);\n \tif (ecbdata->diff_words) {\n \t\tif (line[0] == '-') {\n \t\t\tdiff_words_append(line, len,\n@@ -641,13 +630,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\temit_line(ecbdata->file, plain, reset, line, len);\n \t\treturn;\n \t}\n-\tfor (i = 0; i < ecbdata->nparents && len; i++) {\n-\t\tif (line[i] == '-')\n-\t\t\tcolor = DIFF_FILE_OLD;\n-\t\telse if (line[i] == '+')\n-\t\t\tcolor = DIFF_FILE_NEW;\n-\t}\n-\n+\tif (line[0] == '-')\n+\t\tcolor = DIFF_FILE_OLD;\n+\telse if (line[0] == '+')\n+\t\tcolor = DIFF_FILE_NEW;\n \tif (color != DIFF_FILE_NEW) {\n \t\temit_line(ecbdata->file,\n \t\t\t  diff_get_color(ecbdata->color_diff, color),\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122419","messageId":"1252061718-11579-8-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 7/9] diff --whitespace=warn/error: obey blank-at-eof","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:16Z","receivedAt":"2009-09-04T10:55:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"diff --check\" code used to conflate trailing-space whitespace error\nclass with this, but now we have a proper separate error class, we should\ncheck it under blank-at-eof, not trailing-space.\n\nThe whitespace error is not about _having_ blank lines at end, but about\nadding _new_ blank lines.  To keep the message consistent with what is\ngiven by \"git apply\", call whitespace_error_string() to generate it,\ninstead of using a hardcoded custom message.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                     |   10 +++++++---\n t/t4015-diff-whitespace.sh |    4 ++--\n t/t4019-diff-wserror.sh    |    2 +-\n 3 files changed, 10 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 1eddd59..a693d18 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1650,10 +1650,14 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\tecb.priv = &data;\n \t\txdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n \n-\t\tif ((data.ws_rule & WS_TRAILING_SPACE) &&\n+\t\tif ((data.ws_rule & WS_BLANK_AT_EOF) &&\n \t\t    data.trailing_blanks_start) {\n-\t\t\tfprintf(o->file, \"%s:%d: ends with blank lines.\\n\",\n-\t\t\t\tdata.filename, data.trailing_blanks_start);\n+\t\t\tstatic char *err;\n+\n+\t\t\tif (!err)\n+\t\t\t\terr = whitespace_error_string(WS_BLANK_AT_EOF);\n+\t\t\tfprintf(o->file, \"%s:%d: %s\\n\",\n+\t\t\t\tdata.filename, data.trailing_blanks_start, err);\n \t\t\tdata.status = 1; /* report errors */\n \t\t}\n \t}\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex b1cbd36..a5d4461 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -335,10 +335,10 @@ test_expect_success 'line numbers in --check output are correct' '\n \n '\n \n-test_expect_success 'checkdiff detects trailing blank lines' '\n+test_expect_success 'checkdiff detects new trailing blank lines (1)' '\n \techo \"foo();\" >x &&\n \techo \"\" >>x &&\n-\tgit diff --check | grep \"ends with blank\"\n+\tgit diff --check | grep \"new blank line\"\n '\n \n test_expect_success 'checkdiff allows new blank lines' '\ndiff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh\nindex 84a1fe3..1517fff 100755\n--- a/t/t4019-diff-wserror.sh\n+++ b/t/t4019-diff-wserror.sh\n@@ -165,7 +165,7 @@ test_expect_success 'trailing empty lines (1)' '\n \n \trm -f .gitattributes &&\n \ttest_must_fail git diff --check >output &&\n-\tgrep \"ends with blank lines.\" output &&\n+\tgrep \"new blank line at\" output &&\n \tgrep \"trailing whitespace\" output\n \n '\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122422","messageId":"1252061718-11579-9-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 8/9] diff --whitespace=warn/error: fix blank-at-eof check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:17Z","receivedAt":"2009-09-04T10:55:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"diff --check\" logic used to share the same issue as the one fixed for\n\"git apply\" earlier in this series, in that a patch that adds new blank\nlines at end could appear as\n\n    @@ -l,5 +m,7 @@$\n    _context$\n    _context$\n    -deleted$\n    +$\n    +$\n    +$\n    _$\n    _$\n\nwhere _ stands for SP and $ shows a end-of-line.  Instead of looking at\neach line in the patch in the callback, simply count the blank lines from\nthe end in two versions, and notice the presence of new ones.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                     |   64 +++++++++++++++++++++++++++++++++-----------\n t/t4015-diff-whitespace.sh |    7 +++++\n 2 files changed, 55 insertions(+), 16 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a693d18..c19c476 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1149,7 +1149,6 @@ struct checkdiff_t {\n \tstruct diff_options *o;\n \tunsigned ws_rule;\n \tunsigned status;\n-\tint trailing_blanks_start;\n };\n \n static int is_conflict_marker(const char *line, unsigned long len)\n@@ -1193,10 +1192,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)\n \tif (line[0] == '+') {\n \t\tunsigned bad;\n \t\tdata->lineno++;\n-\t\tif (!ws_blank_line(line + 1, len - 1, data->ws_rule))\n-\t\t\tdata->trailing_blanks_start = 0;\n-\t\telse if (!data->trailing_blanks_start)\n-\t\t\tdata->trailing_blanks_start = data->lineno;\n \t\tif (is_conflict_marker(line + 1, len - 1)) {\n \t\t\tdata->status |= 1;\n \t\t\tfprintf(data->o->file,\n@@ -1216,14 +1211,12 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)\n \t\t\t      data->o->file, set, reset, ws);\n \t} else if (line[0] == ' ') {\n \t\tdata->lineno++;\n-\t\tdata->trailing_blanks_start = 0;\n \t} else if (line[0] == '@') {\n \t\tchar *plus = strchr(line, '+');\n \t\tif (plus)\n \t\t\tdata->lineno = strtol(plus, NULL, 10) - 1;\n \t\telse\n \t\t\tdie(\"invalid diff\");\n-\t\tdata->trailing_blanks_start = 0;\n \t}\n }\n \n@@ -1437,6 +1430,44 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi\n \treturn NULL;\n }\n \n+static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)\n+{\n+\tchar *ptr = mf->ptr;\n+\tlong size = mf->size;\n+\tint cnt = 0;\n+\n+\tif (!size)\n+\t\treturn cnt;\n+\tptr += size - 1; /* pointing at the very end */\n+\tif (*ptr != '\\n')\n+\t\t; /* incomplete line */\n+\telse\n+\t\tptr--; /* skip the last LF */\n+\twhile (mf->ptr < ptr) {\n+\t\tchar *prev_eol;\n+\t\tfor (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)\n+\t\t\tif (*prev_eol == '\\n')\n+\t\t\t\tbreak;\n+\t\tif (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))\n+\t\t\tbreak;\n+\t\tcnt++;\n+\t\tptr = prev_eol - 1;\n+\t}\n+\treturn cnt;\n+}\n+\n+static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)\n+{\n+\tint l1, l2, at;\n+\tl1 = count_trailing_blank(mf1, ws_rule);\n+\tl2 = count_trailing_blank(mf2, ws_rule);\n+\tif (l2 <= l1)\n+\t\treturn 0;\n+\t/* starting where? */\n+\tat = count_lines(mf1->ptr, mf1->size);\n+\treturn (at - l1) + 1; /* the line number counts from 1 */\n+}\n+\n static void builtin_diff(const char *name_a,\n \t\t\t const char *name_b,\n \t\t\t struct diff_filespec *one,\n@@ -1650,15 +1681,16 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\tecb.priv = &data;\n \t\txdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n \n-\t\tif ((data.ws_rule & WS_BLANK_AT_EOF) &&\n-\t\t    data.trailing_blanks_start) {\n-\t\t\tstatic char *err;\n-\n-\t\t\tif (!err)\n-\t\t\t\terr = whitespace_error_string(WS_BLANK_AT_EOF);\n-\t\t\tfprintf(o->file, \"%s:%d: %s\\n\",\n-\t\t\t\tdata.filename, data.trailing_blanks_start, err);\n-\t\t\tdata.status = 1; /* report errors */\n+\t\tif (data.ws_rule & WS_BLANK_AT_EOF) {\n+\t\t\tint blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);\n+\t\t\tif (blank_at_eof) {\n+\t\t\t\tstatic char *err;\n+\t\t\t\tif (!err)\n+\t\t\t\t\terr = whitespace_error_string(WS_BLANK_AT_EOF);\n+\t\t\t\tfprintf(o->file, \"%s:%d: %s.\\n\",\n+\t\t\t\t\tdata.filename, blank_at_eof, err);\n+\t\t\t\tdata.status = 1; /* report errors */\n+\t\t\t}\n \t\t}\n \t}\n  free_and_return:\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex a5d4461..e0b481d 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -341,6 +341,13 @@ test_expect_success 'checkdiff detects new trailing blank lines (1)' '\n \tgit diff --check | grep \"new blank line\"\n '\n \n+test_expect_success 'checkdiff detects new trailing blank lines (2)' '\n+\t{ echo a; echo b; echo; echo; } >x &&\n+\tgit add x &&\n+\t{ echo a; echo; echo; echo; echo; } >x &&\n+\tgit diff --check | grep \"new blank line\"\n+'\n+\n test_expect_success 'checkdiff allows new blank lines' '\n \tgit checkout x &&\n \tmv x y &&\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122420","messageId":"1252061718-11579-10-git-send-email-gitster@pobox.com","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"[PATCH 9/9] diff --color: color blank-at-eof","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T10:55:18Z","receivedAt":"2009-09-04T10:55:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Since the coloring logic processed the patch output one line at a time, we\ncouldn't easily color code the new blank lines at the end of file.\n\nReuse the adds_blank_at_eof() function to find where the runs of such\nblank lines start, keep track of the line number in the preimage while\nprocessing the patch output one line at a time, and paint the new blank\nlines that appear after that line to implement this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                  |   37 +++++++++++++++++++++++++++----------\n t/t4019-diff-wserror.sh |    9 +++++++++\n 2 files changed, 36 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex c19c476..2b285b8 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -491,6 +491,8 @@ struct emit_callback {\n \tstruct xdiff_emit_state xm;\n \tint color_diff;\n \tunsigned ws_rule;\n+\tint blank_at_eof;\n+\tint lno_in_preimage;\n \tsane_truncate_fn truncate;\n \tconst char **label_path;\n \tstruct diff_words_data *diff_words;\n@@ -547,6 +549,12 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons\n \n \tif (!*ws)\n \t\temit_line(ecbdata->file, set, reset, line, len);\n+\telse if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n+\t\t ecbdata->blank_at_eof &&\n+\t\t (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&\n+\t\t ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))\n+\t\t/* Blank line at EOF */\n+\t\temit_line(ecbdata->file, ws, reset, line, len);\n \telse {\n \t\t/* Emit just the prefix, then the rest. */\n \t\temit_line(ecbdata->file, set, reset, line, 1);\n@@ -573,9 +581,16 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u\n \treturn allot - l;\n }\n \n+static int find_preimage_lno(const char *line)\n+{\n+\tchar *p = strchr(line, '-');\n+\tif (!p)\n+\t\treturn 0; /* should not happen */\n+\treturn strtol(p+1, NULL, 10);\n+}\n+\n static void fn_out_consume(void *priv, char *line, unsigned long len)\n {\n-\tint color;\n \tstruct emit_callback *ecbdata = priv;\n \tconst char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);\n \tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n@@ -598,6 +613,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \n \tif (line[0] == '@') {\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n+\t\tecbdata->lno_in_preimage = find_preimage_lno(line);\n \t\temit_line(ecbdata->file,\n \t\t\t  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n \t\t\t  reset, line, len);\n@@ -611,7 +627,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\treturn;\n \t}\n \n-\tcolor = DIFF_PLAIN;\n \tif (ecbdata->diff_words) {\n \t\tif (line[0] == '-') {\n \t\t\tdiff_words_append(line, len,\n@@ -630,14 +645,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\temit_line(ecbdata->file, plain, reset, line, len);\n \t\treturn;\n \t}\n-\tif (line[0] == '-')\n-\t\tcolor = DIFF_FILE_OLD;\n-\telse if (line[0] == '+')\n-\t\tcolor = DIFF_FILE_NEW;\n-\tif (color != DIFF_FILE_NEW) {\n-\t\temit_line(ecbdata->file,\n-\t\t\t  diff_get_color(ecbdata->color_diff, color),\n-\t\t\t  reset, line, len);\n+\n+\tif (line[0] != '+') {\n+\t\tconst char *color =\n+\t\t\tdiff_get_color(ecbdata->color_diff,\n+\t\t\t\t       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);\n+\t\tecbdata->lno_in_preimage++;\n+\t\temit_line(ecbdata->file, color, reset, line, len);\n \t\treturn;\n \t}\n \temit_add_line(reset, ecbdata, line, len);\n@@ -1557,6 +1571,9 @@ static void builtin_diff(const char *name_a,\n \t\tecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);\n \t\tecbdata.found_changesp = &o->found_changes;\n \t\tecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);\n+\t\tif (ecbdata.ws_rule & WS_BLANK_AT_EOF)\n+\t\t\tecbdata.blank_at_eof =\n+\t\t\t\tadds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);\n \t\tecbdata.file = o->file;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\ndiff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh\nindex 1517fff..1e75f1a 100755\n--- a/t/t4019-diff-wserror.sh\n+++ b/t/t4019-diff-wserror.sh\n@@ -190,4 +190,13 @@ test_expect_success 'do not color trailing cr in context' '\n \n '\n \n+test_expect_success 'color new trailing blank lines' '\n+\t{ echo a; echo b; echo; echo; } >x &&\n+\tgit add x &&\n+\t{ echo a; echo; echo; echo; echo; } >x &&\n+\tgit diff --color x >output &&\n+\tcnt=$(grep \"${blue_grep}\" output | wc -l) &&\n+\ttest $cnt = 2\n+'\n+\n test_done\n-- \n1.6.4.2.313.g0425f\n"},{"id":"122425","messageId":"4AA101BB.7010206@viscovery.net","threadId":"20849","inReplyTo":"1252061718-11579-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-04T12:02:03Z","receivedAt":"2009-09-04T12:02:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> The command tries to strip blank lines at the end of the file added by a\n> patch.  However, if the original ends with blank lines, often the patch\n> hunk ends like this:\n> \n>     @@ -l,5 +m,7 @@$\n>     _context$\n>     _context$\n>     -deleted$\n>     +$\n>     +$\n>     +$\n>     _$\n>     _$\n> \n> where _ stands for SP and $ shows a end-of-line.  This example patch adds\n> three trailing blank lines, but the code fails to notice it, because it\n> only pays attention to added blank lines at the very end of the hunk.  In\n> this example, the three added blank lines do not appear textually at the\n> end in the patch, even though you can see that they are indeed added at\n> the end, if you rearrange the diff like this:\n> \n>     @@ -l,5 +m,7 @@$\n>     _context$\n>     _context$\n>     -deleted$\n>     _$\n>     _$\n>     +$\n>     +$\n>     +$\n> \n> Fix this by not resetting the number of (candidate) added blank lines at\n> the end when the loop sees a context line that is empty.\n\nAfter reading this explanation, I was worried that added blank lines that\nare at the end of a patch but apply in the middle of a file would be\nmis-attributed as blank lines at EOF. But appearently, they are not, i.e.\nsuch added blank lines are not removed. Could you squash in this test case\nthat checks for this condition.\n\n-- Hannes\n\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex ba2b7f9..fedc8b9 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -189,4 +189,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '\n \ttest_cmp expect one\n '\n\n+test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '\n+\t{ echo a; echo b; echo; echo; echo; echo; echo; echo d; } >one &&\n+\tgit add one &&\n+\t{ echo a; echo c; echo; echo; echo; echo; echo; echo; echo d; } >expect &&\n+\tcp expect one &&\n+\tgit diff -- one >patch &&\n+\n+\tgit checkout one &&\n+\tgit apply --whitespace=fix patch &&\n+\ttest_cmp expect one\n+'\n+\n test_done\n"},{"id":"122447","messageId":"7vpra6n0b7.fsf@alter.siamese.dyndns.org","threadId":"20849","inReplyTo":"4AA101BB.7010206@viscovery.net","subject":"Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-04T16:26:20Z","receivedAt":"2009-09-04T16:26:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> After reading this explanation, I was worried that added blank lines that\n> are at the end of a patch but apply in the middle of a file would be\n> mis-attributed as blank lines at EOF.\n\nThe codepath this patch is about checks \"does the hunk result in more\nblank at the end of the place it applies to?\".  There is a separate logic\nthat checks \"does the hunk applies at the end of the file\", which is the\ntopic of the codepath that is fixed by Patch #1.\n"},{"id":"122529","messageId":"alpine.WNT.2.00.0909051534380.7040@GWNotebook","threadId":"20849","inReplyTo":"1252061718-11579-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/9] War on blank-at-eof","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-09-05T21:28:22Z","receivedAt":"2009-09-05T21:28:22Z","isPatch":true,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"On Fri, 4 Sep 2009, Junio C Hamano wrote:\n\n> Patch 5 corrects the definition of blank-at-eof.  If a patch adds an\n> non-empty line that consists solely of whitespaces at the end of file, we\n> should diagnose and strip it just line a new empty line.  After all, both\n> are blank lines.\n>\n\nThank you. Thank you, thank you. Thank you!  And did I mention thank you?\n\nTested this out after cherry-picking:\n3b5ef0e xutils: Fix xdl_recmatch() on incomplete lines\n78ed710 xutils: Fix hashing an incomplete line with whitespaces at the end\n\nIt worked as nicely!  I'm throwing away the --allow-whitelines-at-eof \npatch! :D  Converting a _real_ dirty whitespace branch into an 'almost' \nwhitespace policy compliant branch with validation of the diffs was \nable to be done like so:\n\tgit diff -b DIRTY CLEAN\n\tgit diff DIRTY^ CLEAN > diff1\n\tgit diff CLEAN^ DIRTY > diff2\n\tgit diff -b diff1 diff2\n\nI mention 'almost' above because unfortunately this type of conversion \nleaves extra line-spaces at the end of some files that you might not want \nto have in a whitespace policy.\n\nWhile thinking about what appeared in:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/124138\nJunio C Hamano <gitster <at> pobox.com> writes:\n>Bruno Haible <bruno <at> clisp.org> writes:\n>> In some GNU projects, there are file types for which trailing spaces in a line\n>> ...\n>> Currently the user has to turn off the 'trailing-space' whitespace attribute\n>> in order for 'git diff --check' to not complain about such files. This has\n>> the drawback that trailing spaces are not detected.\n\t\n>Very good problem description.  Thanks.\n\nI thought it might be interesting to throw this out there...  What do you \nthink of an additional attribute value like\n\tcore.whitespace blank-at-eof-min-<some 0 to N #>\n\tcore.whitespace blank-at-eof-max-<some 0 to N #>\nthat could be read in when core.whitespace blank-at-eof is set.\n\nIf neither are present then use current. (No new eof blanks).\nIf min but not max is set then allow new blanks and ensure at least min.\nIf max but not min is set then only allow max blanks at eof.\nIf both then treat it as a boundary.\n\nThis could ensure a whitespace policy without the repository maintainer \nhaving to correct this type of minutia and without having to nit-pick \ncontributors into submission.\n\nThen perhaps diff could also recognize an in range blank-at-eof so a diff \nusing one of the ignore whitespace options would ignore eof whitelines \nthat are in range?\n\n\n> The series applies to v1.6.0.6-87-g82d97da; merging the result to 'master'\n> needs some conflict resolution.\n>\n\n\n-- \nThell\n"},{"id":"122535","messageId":"7v63bwob1c.fsf@alter.siamese.dyndns.org","threadId":"20849","inReplyTo":"alpine.WNT.2.00.0909051534380.7040@GWNotebook","subject":"Re: [PATCH 0/9] War on blank-at-eof","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-06T06:13:51Z","receivedAt":"2009-09-06T06:13:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thell Fowler <git@tbfowler.name> writes:\n\n> While thinking about what appeared in:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/124138\n\nOh, I forgot all about that one.  The suggestion does include two very\ngood points, one being \"git apply\" which I did, and the other being what I\ncompletely forgot.  Introduction of blank-at-eol and blank-at-eof, and\nmake trailing-space a convenience synonym that triggers both.\n\nThanks for a reminder.  The following patch can come on top of the\nseries.\n\n-- >8 --\nSubject: core.whitespace: split trailing-space into blank-at-{eol,eof}\n\nPeople who configured trailing-space depended on it to catch both extra\nwhite space at the end of line, and extra blank lines at the end of file.\nEarlier attempt to introduce only blank-at-eof gave them an escape hatch\nto keep the old behaviour, but it is a regression until they explicitly\nspecify the new error class.\n\nThis introduces a blank-at-eol that only catches extra white space at the\nend of line, and makes the traditional trailing-space a convenient synonym\nto catch both blank-at-eol and blank-at-eof.  This way, people who used\ntrailing-space continue to catch both classes of errors.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |    5 ++++-\n cache.h                  |    5 +++--\n ws.c                     |   24 +++++++++++++++---------\n 3 files changed, 22 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 871384e..0e245a7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -382,7 +382,7 @@ core.whitespace::\n \tconsider them as errors.  You can prefix `-` to disable\n \tany of them (e.g. `-trailing-space`):\n +\n-* `trailing-space` treats trailing whitespaces at the end of the line\n+* `blank-at-eol` treats trailing whitespaces at the end of the line\n   as an error (enabled by default).\n * `space-before-tab` treats a space character that appears immediately\n   before a tab character in the initial indent part of the line as an\n@@ -391,11 +391,14 @@ core.whitespace::\n   space characters as an error (not enabled by default).\n * `blank-at-eof` treats blank lines added at the end of file as an error\n   (enabled by default).\n+* `trailing-space` is a short-hand to cover both `blank-at-eol` and\n+  `blank-at-eof`.\n * `cr-at-eol` treats a carriage-return at the end of line as\n   part of the line terminator, i.e. with it, `trailing-space`\n   does not trigger if the character before such a carriage-return\n   is not a whitespace (not enabled by default).\n \n+\n core.fsyncobjectfiles::\n \tThis boolean will enable 'fsync()' when writing object files.\n +\ndiff --git a/cache.h b/cache.h\nindex 7152fea..ee12e74 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -841,12 +841,13 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i\n  * whitespace rules.\n  * used by both diff and apply\n  */\n-#define WS_TRAILING_SPACE\t01\n+#define WS_BLANK_AT_EOL         01\n #define WS_SPACE_BEFORE_TAB\t02\n #define WS_INDENT_WITH_NON_TAB\t04\n #define WS_CR_AT_EOL           010\n #define WS_BLANK_AT_EOF        020\n-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)\n+#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)\n extern unsigned whitespace_rule_cfg;\n extern unsigned whitespace_rule(const char *);\n extern unsigned parse_whitespace_rule(const char *);\ndiff --git a/ws.c b/ws.c\nindex d56636b..cd03bc0 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -15,6 +15,7 @@ static struct whitespace_rule {\n \t{ \"space-before-tab\", WS_SPACE_BEFORE_TAB },\n \t{ \"indent-with-non-tab\", WS_INDENT_WITH_NON_TAB },\n \t{ \"cr-at-eol\", WS_CR_AT_EOL },\n+\t{ \"blank-at-eol\", WS_BLANK_AT_EOL },\n \t{ \"blank-at-eof\", WS_BLANK_AT_EOF },\n };\n \n@@ -101,9 +102,19 @@ unsigned whitespace_rule(const char *pathname)\n char *whitespace_error_string(unsigned ws)\n {\n \tstruct strbuf err;\n+\n \tstrbuf_init(&err, 0);\n-\tif (ws & WS_TRAILING_SPACE)\n+\tif ((ws & WS_TRAILING_SPACE) == WS_TRAILING_SPACE)\n \t\tstrbuf_addstr(&err, \"trailing whitespace\");\n+\telse {\n+\t\tif (ws & WS_BLANK_AT_EOL)\n+\t\t\tstrbuf_addstr(&err, \"trailing whitespace\");\n+\t\tif (ws & WS_BLANK_AT_EOF) {\n+\t\t\tif (err.len)\n+\t\t\t\tstrbuf_addstr(&err, \", \");\n+\t\t\tstrbuf_addstr(&err, \"new blank line at EOF\");\n+\t\t}\n+\t}\n \tif (ws & WS_SPACE_BEFORE_TAB) {\n \t\tif (err.len)\n \t\t\tstrbuf_addstr(&err, \", \");\n@@ -114,11 +125,6 @@ char *whitespace_error_string(unsigned ws)\n \t\t\tstrbuf_addstr(&err, \", \");\n \t\tstrbuf_addstr(&err, \"indent with spaces\");\n \t}\n-\tif (ws & WS_BLANK_AT_EOF) {\n-\t\tif (err.len)\n-\t\t\tstrbuf_addstr(&err, \", \");\n-\t\tstrbuf_addstr(&err, \"new blank line at EOF\");\n-\t}\n \treturn strbuf_detach(&err, NULL);\n }\n \n@@ -146,11 +152,11 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,\n \t}\n \n \t/* Check for trailing whitespace. */\n-\tif (ws_rule & WS_TRAILING_SPACE) {\n+\tif (ws_rule & WS_BLANK_AT_EOL) {\n \t\tfor (i = len - 1; i >= 0; i--) {\n \t\t\tif (isspace(line[i])) {\n \t\t\t\ttrailing_whitespace = i;\n-\t\t\t\tresult |= WS_TRAILING_SPACE;\n+\t\t\t\tresult |= WS_BLANK_AT_EOL;\n \t\t\t}\n \t\t\telse\n \t\t\t\tbreak;\n@@ -266,7 +272,7 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro\n \t/*\n \t * Strip trailing whitespace\n \t */\n-\tif ((ws_rule & WS_TRAILING_SPACE) &&\n+\tif ((ws_rule & WS_BLANK_AT_EOL) &&\n \t    (2 <= len && isspace(src[len-2]))) {\n \t\tif (src[len - 1] == '\\n') {\n \t\t\tadd_nl_to_tail = 1;\n-- \n1.6.4.2.313.g0425f\n"}]}