@matheusbsbs / matheus-bagliano / commits / 77b15bbd88

Fork 0

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>

Junio C Hamano committed Sep 3, 2009 at 16:02 UTC 77b15bbd88e2f48de093ff0e60de6dbc11e3329e

5 files changed +54 -10

Documentation/config.txt

@@ -389,6 +389,8 @@ core.whitespace::

389 error (enabled by default).

390 * `indent-with-non-tab` treats a line that is indented with 8 or more

391 space characters as an error (not enabled by default).

392 +* `blank-at-eof` treats blank lines added at the end of file as an error

393 + (enabled by default).

394 * `cr-at-eol` treats a carriage-return at the end of line as

395 part of the line terminator, i.e. with it, `trailing-space`

396 does not trigger if the character before such a carriage-return

builtin-apply.c

+18 -9

@@ -126,6 +126,7 @@ struct fragment {

126 const char *patch;

127 int size;

128 int rejected;

129 + int linenr;

130 struct fragment *next;

131 };

132

@@ -1193,6 +1194,7 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc

1194 int len;

1195

1196 fragment = xcalloc(1, sizeof(*fragment));

1197 + fragment->linenr = linenr;

1198 len = parse_fragment(line, size, patch, fragment);

1199 if (len <= 0)

1200 die("corrupt patch at line %d", linenr);

@@ -2079,17 +2081,24 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,

2081 }

2082

2083 if (applied_pos >= 0) {

2082 - if (ws_error_action == correct_ws_error &&

2083 - new_blank_lines_at_end &&

2084 - preimage.nr + applied_pos == img->nr) {

2084 + if (new_blank_lines_at_end &&

2085 + preimage.nr + applied_pos == img->nr &&

2086 + (ws_rule & WS_BLANK_AT_EOF) &&

2087 + ws_error_action != nowarn_ws_error) {

2088 + record_ws_error(WS_BLANK_AT_EOF, "+", 1, frag->linenr);

2089 + if (ws_error_action == correct_ws_error) {

2090 + while (new_blank_lines_at_end--)

2091 + remove_last_line(&postimage);

2092 + }

2093 /*

2086 - * If the patch application adds blank lines

2087 - * at the end, and if the patch applies at the

2088 - * end of the image, remove those added blank

2089 - * lines.

2094 + * We would want to prevent write_out_results()

2095 + * from taking place in apply_patch() that follows

2096 + * the callchain led us here, which is:

2097 + * apply_patch->check_patch_list->check_patch->

2098 + * apply_data->apply_fragments->apply_one_fragment

2099 */

2091 - while (new_blank_lines_at_end--)

2092 - remove_last_line(&postimage);

2100 + if (ws_error_action == die_on_ws_error)

2101 + apply = 0;

2102 }

2103

2104 /*

cache.h

+2 -1

@@ -845,7 +845,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i

845 #define WS_SPACE_BEFORE_TAB 02

846 #define WS_INDENT_WITH_NON_TAB 04

847 #define WS_CR_AT_EOL 010

848 -#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)

848 +#define WS_BLANK_AT_EOF 020

849 +#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)

850 extern unsigned whitespace_rule_cfg;

851 extern unsigned whitespace_rule(const char *);

852 extern unsigned parse_whitespace_rule(const char *);

t/t4124-apply-ws-rule.sh

+26

@@ -201,4 +201,30 @@ test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '

201 test_cmp expect one

202 '

203

204 +test_expect_success 'blank at EOF with --whitespace=warn' '

205 + { echo a; echo b; echo c; } >one &&

206 + git add one &&

207 + echo >>one &&

208 + cat one >expect &&

209 + git diff -- one >patch &&

210 +

211 + git checkout one &&

212 + git apply --whitespace=warn patch 2>error &&

213 + test_cmp expect one &&

214 + grep "new blank line at EOF" error

215 +'

216 +

217 +test_expect_success 'blank at EOF with --whitespace=error' '

218 + { echo a; echo b; echo c; } >one &&

219 + git add one &&

220 + cat one >expect &&

221 + echo >>one &&

222 + git diff -- one >patch &&

223 +

224 + git checkout one &&

225 + test_must_fail git apply --whitespace=error patch 2>error &&

226 + test_cmp expect one &&

227 + grep "new blank line at EOF" error

228 +'

229 +

230 test_done

ws.c

@@ -15,6 +15,7 @@ static struct whitespace_rule {

15 { "space-before-tab", WS_SPACE_BEFORE_TAB },

16 { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },

17 { "cr-at-eol", WS_CR_AT_EOL },

18 + { "blank-at-eof", WS_BLANK_AT_EOF },

19 };

21 unsigned parse_whitespace_rule(const char *string)

@@ -113,6 +114,11 @@ char *whitespace_error_string(unsigned ws)

114 strbuf_addstr(&err, ", ");

115 strbuf_addstr(&err, "indent with spaces");

116 }

117 + if (ws & WS_BLANK_AT_EOF) {

118 + if (err.len)

119 + strbuf_addstr(&err, ", ");

120 + strbuf_addstr(&err, "new blank line at EOF");

121 + }

122 return strbuf_detach(&err, NULL);

123 }

124