apply --whitespace=warn/error: diagnose blank at EOF

"git apply" strips new blank lines at EOF under --whitespace=fix option,
but neigher --whitespace=warn nor --whitespace=error paid any attention to
these errors.

Introduce a new whitespace error class, blank-at-eof, to make the
whitespace error handling more consistent.

The patch adds a new "linenr" field to the struct fragment in order to
record which line the hunk started in the input file, but this is needed
solely for reporting purposes.  The detection of this class of whitespace
errors cannot be done while parsing a patch like we do for all the other
classes of whitespace errors.  It instead has to wait until we find where
to apply the hunk, but at that point, we do not have an access to the
original line number in the input file anymore, hence the new field.

Depending on your point of view, this may be a bugfix that makes warn and
error in line with fix.  Or you could call it a new feature.  The line
between them is somewhat fuzzy in this case.

Strictly speaking, triggering more errors than before is a change in
behaviour that is not backward compatible, even though the reason for the
change is because the code was not checking for an error that it should
have.  People who do not want added blank lines at EOF to trigger an error
can disable the new error class.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 113d9d1438891b51d11f2fee2764c88e69024921..871384eb73e9df782553049100fd8ee2891daa12 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -389,6 +389,8 @@ core.whitespace::
   error (enabled by default).
 * `indent-with-non-tab` treats a line that is indented with 8 or more
   space characters as an error (not enabled by default).
+* `blank-at-eof` treats blank lines added at the end of file as an error
+  (enabled by default).
 * `cr-at-eol` treats a carriage-return at the end of line as
   part of the line terminator, i.e. with it, `trailing-space`
   does not trigger if the character before such a carriage-return

diff --git a/builtin-apply.c b/builtin-apply.c
index 80ddf55ba598935636e3bccc8d5097209096bd89..37d3bc069b1618f81c14cf0c87d31a0f1a3b081c 100644 (file)
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -126,6 +126,7 @@ struct fragment {
        const char *patch;
        int size;
        int rejected;
+       int linenr;
        struct fragment *next;
 };
 
@@ -1193,6 +1194,7 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc
                int len;
 
                fragment = xcalloc(1, sizeof(*fragment));
+               fragment->linenr = linenr;
                len = parse_fragment(line, size, patch, fragment);
                if (len <= 0)
                        die("corrupt patch at line %d", linenr);
@@ -2079,17 +2081,24 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
        }
 
        if (applied_pos >= 0) {
-               if (ws_error_action == correct_ws_error &&
-                   new_blank_lines_at_end &&
-                   preimage.nr + applied_pos == img->nr) {
+               if (new_blank_lines_at_end &&
+                   preimage.nr + applied_pos == img->nr &&
+                   (ws_rule & WS_BLANK_AT_EOF) &&
+                   ws_error_action != nowarn_ws_error) {
+                       record_ws_error(WS_BLANK_AT_EOF, "+", 1, frag->linenr);
+                       if (ws_error_action == correct_ws_error) {
+                               while (new_blank_lines_at_end--)
+                                       remove_last_line(&postimage);
+                       }
                        /*
-                        * If the patch application adds blank lines
-                        * at the end, and if the patch applies at the
-                        * end of the image, remove those added blank
-                        * lines.
+                        * We would want to prevent write_out_results()
+                        * from taking place in apply_patch() that follows
+                        * the callchain led us here, which is:
+                        * apply_patch->check_patch_list->check_patch->
+                        * apply_data->apply_fragments->apply_one_fragment
                         */
-                       while (new_blank_lines_at_end--)
-                               remove_last_line(&postimage);
+                       if (ws_error_action == die_on_ws_error)
+                               apply = 0;
                }
 
                /*

diff --git a/cache.h b/cache.h
index 099a32e5fc536830fadc31cc68663ac3495a2a61..7152feaef222f4f3c23884f43aafb12376a0242a 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -845,7 +845,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
 #define WS_SPACE_BEFORE_TAB    02
 #define WS_INDENT_WITH_NON_TAB 04
 #define WS_CR_AT_EOL           010
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
+#define WS_BLANK_AT_EOF        020
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);

diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index fedc8b9277778064e558a3d2fad90e330b035085..3a77a9af87a15f9c73795afd7e27d1e29a92db32 100755 (executable)
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -201,4 +201,30 @@ test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '
        test_cmp expect one
 '
 
+test_expect_success 'blank at EOF with --whitespace=warn' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       echo >>one &&
+       cat one >expect &&
+       git diff -- one >patch &&
+
+       git checkout one &&
+       git apply --whitespace=warn patch 2>error &&
+       test_cmp expect one &&
+       grep "new blank line at EOF" error
+'
+
+test_expect_success 'blank at EOF with --whitespace=error' '
+       { echo a; echo b; echo c; } >one &&
+       git add one &&
+       cat one >expect &&
+       echo >>one &&
+       git diff -- one >patch &&
+
+       git checkout one &&
+       test_must_fail git apply --whitespace=error patch 2>error &&
+       test_cmp expect one &&
+       grep "new blank line at EOF" error
+'
+
 test_done

diff --git a/ws.c b/ws.c
index 7a7ff130a34942506e6068105ac5946c9404bf18..d56636be7b3f28b589da3692e31a920a57653e74 100644 (file)
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
        { "space-before-tab", WS_SPACE_BEFORE_TAB },
        { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
        { "cr-at-eol", WS_CR_AT_EOL },
+       { "blank-at-eof", WS_BLANK_AT_EOF },
 };
 
 unsigned parse_whitespace_rule(const char *string)
@@ -113,6 +114,11 @@ char *whitespace_error_string(unsigned ws)
                        strbuf_addstr(&err, ", ");
                strbuf_addstr(&err, "indent with spaces");
        }
+       if (ws & WS_BLANK_AT_EOF) {
+               if (err.len)
+                       strbuf_addstr(&err, ", ");
+               strbuf_addstr(&err, "new blank line at EOF");
+       }
        return strbuf_detach(&err, NULL);
 }