From ef2035c5e55f852905b012dfb2dd242b4f77da70 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 4 Sep 2009 01:41:47 -0700 Subject: apply --whitespace=fix: fix handling of blank lines at the eof b94f2ed (builtin-apply.c: make it more line oriented, 2008-01-26) broke the logic used to detect if a hunk adds blank lines at the end of the file. With the new code after that commit: - img holds the contents of the file that the hunk is being applied to; - preimage has the lines the hunk expects to be in img; and - postimage has the lines the hunk wants to update the part in img that corresponds to preimage with. and we need to compare if the last line of preimage (not postimage) matches the last line of img to see if the hunk applies at the end of the file. Signed-off-by: Junio C Hamano --- builtin-apply.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin-apply.c') diff --git a/builtin-apply.c b/builtin-apply.c index 7a1ff041f1..5b5bde4f39 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2069,7 +2069,7 @@ 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 && - postimage.nr + applied_pos == img->nr) { + preimage.nr + applied_pos == img->nr) { /* * If the patch application adds blank lines * at the end, and if the patch applies at the -- cgit v1.2.3 From efa574438fca4ed328d2e47ecdb6363c50a905ec Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 14:08:20 -0700 Subject: apply --whitespace=fix: detect new blank lines at eof correctly The command tries to strip blank lines at the end of the file added by a patch. It is done by first detecting if a hunk in patch has additional blank lines at the end of itself, and if so checking if such a hunk applies at the end of file. This patch addresses a bug in the logic to implement the former (the previous one addressed a bug in the latter). If the original ends with blank lines, often the patch hunk ends like this: @@ -l,5 +m,7 @@$ _context$ _context$ -deleted$ +$ +$ +$ _$ _$ where _ stands for SP and $ shows a end-of-line. This example patch adds three trailing blank lines, but the code fails to notice it, because it only pays attention to added blank lines at the very end of the hunk. In this example, the three added blank lines do not appear textually at the end in the patch, even though you can see that they are indeed added at the end, if you rearrange the diff like this: @@ -l,5 +m,7 @@$ _context$ _context$ -deleted$ _$ _$ +$ +$ +$ The fix is not to reset the number of (candidate) added blank lines at the end when the loop sees a context line that is empty. Signed-off-by: Junio C Hamano --- builtin-apply.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'builtin-apply.c') diff --git a/builtin-apply.c b/builtin-apply.c index 5b5bde4f39..c5e4048c61 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -1913,6 +1913,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag, int len = linelen(patch, size); int plen, added; int added_blank_line = 0; + int is_blank_context = 0; if (!len) break; @@ -1945,8 +1946,11 @@ static int apply_one_fragment(struct image *img, struct fragment *frag, *new++ = '\n'; add_line_info(&preimage, "\n", 1, LINE_COMMON); add_line_info(&postimage, "\n", 1, LINE_COMMON); + is_blank_context = 1; break; case ' ': + if (plen && patch[1] == '\n') + is_blank_context = 1; case '-': memcpy(old, patch + 1, plen); add_line_info(&preimage, old, plen, @@ -1986,6 +1990,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag, } if (added_blank_line) new_blank_lines_at_end++; + else if (is_blank_context) + ; else new_blank_lines_at_end = 0; patch += len; -- cgit v1.2.3 From 92a1747eeab6bc497db748e79ca4f029b1c1dc10 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 22:26:33 -0700 Subject: apply.c: split check_whitespace() into two This splits the logic to record the presence of whitespace errors out of the check_whitespace() function, which checks and then records. The new function, record_ws_error(), can be used by the blank-at-eof check that does not use ws_check() logic to report its findings in the same output format. Signed-off-by: Junio C Hamano --- builtin-apply.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'builtin-apply.c') diff --git a/builtin-apply.c b/builtin-apply.c index c5e4048c61..80ddf55ba5 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -1055,23 +1055,29 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc return -1; } -static void check_whitespace(const char *line, int len, unsigned ws_rule) +static void record_ws_error(unsigned result, const char *line, int len, int linenr) { char *err; - unsigned result = ws_check(line + 1, len - 1, ws_rule); + if (!result) return; whitespace_error++; if (squelch_whitespace_errors && squelch_whitespace_errors < whitespace_error) - ; - else { - err = whitespace_error_string(result); - fprintf(stderr, "%s:%d: %s.\n%.*s\n", - patch_input_file, linenr, err, len - 2, line + 1); - free(err); - } + return; + + err = whitespace_error_string(result); + fprintf(stderr, "%s:%d: %s.\n%.*s\n", + patch_input_file, linenr, err, len, line); + free(err); +} + +static void check_whitespace(const char *line, int len, unsigned ws_rule) +{ + unsigned result = ws_check(line + 1, len - 1, ws_rule); + + record_ws_error(result, line + 1, len - 2, linenr); } /* -- cgit v1.2.3 From 77b15bbd88e2f48de093ff0e60de6dbc11e3329e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 16:02:32 -0700 Subject: 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 --- builtin-apply.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) (limited to 'builtin-apply.c') diff --git a/builtin-apply.c b/builtin-apply.c index 80ddf55ba5..37d3bc069b 100644 --- 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; } /* -- cgit v1.2.3 From 94ea026b358f9fd5395d9344a80f2d5a50e93e1f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 4 Sep 2009 02:25:57 -0700 Subject: apply --whitespace: warn blank but not necessarily empty lines at EOF The whitespace error of adding blank lines at the end of file should trigger if you added a non-empty line at the end, if the contents of the line is full of whitespaces. Signed-off-by: Junio C Hamano --- builtin-apply.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'builtin-apply.c') diff --git a/builtin-apply.c b/builtin-apply.c index 37d3bc069b..6662cc438c 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -1957,7 +1957,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag, is_blank_context = 1; break; case ' ': - if (plen && patch[1] == '\n') + if (plen && (ws_rule & WS_BLANK_AT_EOF) && + ws_blank_line(patch + 1, plen, ws_rule)) is_blank_context = 1; case '-': memcpy(old, patch + 1, plen); @@ -1985,7 +1986,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag, (first == '+' ? 0 : LINE_COMMON)); new += added; if (first == '+' && - added == 1 && new[-1] == '\n') + (ws_rule & WS_BLANK_AT_EOF) && + ws_blank_line(patch + 1, plen, ws_rule)) added_blank_line = 1; break; case '@': case '\\': -- cgit v1.2.3