diff options
author | Junio C Hamano <gitster@pobox.com> | 2009-10-17 11:11:03 +0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2009-10-17 11:11:03 +0400 |
commit | 9981c808b4b9307d58cedddd17689c2313a0d34a (patch) | |
tree | 4646517a38a111cd0b2e565ecb20dfaf9f59e8b3 | |
parent | 7641eb400f8dc1c8b8b0ca8f325c753ae566e156 (diff) | |
parent | d91ba8fa88a83ec70588700a56c8a4c1e8beb1be (diff) |
Merge branch 'jc/maint-blank-at-eof'
* jc/maint-blank-at-eof:
diff -B: colour whitespace errors
diff.c: emit_add_line() takes only the rest of the line
diff.c: split emit_line() from the first char and the rest of the line
diff.c: shuffling code around
diff --whitespace: fix blank lines at end
core.whitespace: split trailing-space into blank-at-{eol,eof}
diff --color: color blank-at-eof
diff --whitespace=warn/error: fix blank-at-eof check
diff --whitespace=warn/error: obey blank-at-eof
diff.c: the builtin_diff() deals with only two-file comparison
apply --whitespace: warn blank but not necessarily empty lines at EOF
apply --whitespace=warn/error: diagnose blank at EOF
apply.c: split check_whitespace() into two
apply --whitespace=fix: detect new blank lines at eof correctly
apply --whitespace=fix: fix handling of blank lines at the eof
-rw-r--r-- | Documentation/config.txt | 6 | ||||
-rw-r--r-- | builtin-apply.c | 61 | ||||
-rw-r--r-- | cache.h | 4 | ||||
-rw-r--r-- | diff.c | 407 | ||||
-rwxr-xr-x | t/t4015-diff-whitespace.sh | 11 | ||||
-rwxr-xr-x | t/t4019-diff-wserror.sh | 11 | ||||
-rwxr-xr-x | t/t4124-apply-ws-rule.sh | 91 | ||||
-rw-r--r-- | ws.c | 19 |
8 files changed, 437 insertions, 173 deletions
diff --git a/Documentation/config.txt b/Documentation/config.txt index cd1781498e..5a8f57cbf7 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -416,13 +416,17 @@ core.whitespace:: consider them as errors. You can prefix `-` to disable any of them (e.g. `-trailing-space`): + -* `trailing-space` treats trailing whitespaces at the end of the line +* `blank-at-eol` treats trailing whitespaces at the end of the line as an error (enabled by default). * `space-before-tab` treats a space character that appears immediately before a tab character in the initial indent part of the line as an 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). +* `trailing-space` is a short-hand to cover both `blank-at-eol` and + `blank-at-eof`. * `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 c8372a0a80..f667368d16 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -153,6 +153,7 @@ struct fragment { const char *patch; int size; int rejected; + int linenr; struct fragment *next; }; @@ -1227,23 +1228,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); } /* @@ -1359,6 +1366,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); @@ -2142,6 +2150,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; @@ -2174,8 +2183,12 @@ 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 && (ws_rule & WS_BLANK_AT_EOF) && + ws_blank_line(patch + 1, plen, ws_rule)) + is_blank_context = 1; case '-': memcpy(old, patch + 1, plen); add_line_info(&preimage, old, plen, @@ -2202,7 +2215,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 '\\': @@ -2215,6 +2229,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; @@ -2296,17 +2312,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 && - postimage.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; } /* @@ -986,10 +986,12 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i * whitespace rules. * used by both diff and apply */ -#define WS_TRAILING_SPACE 01 +#define WS_BLANK_AT_EOL 01 #define WS_SPACE_BEFORE_TAB 02 #define WS_INDENT_WITH_NON_TAB 04 #define WS_CR_AT_EOL 010 +#define WS_BLANK_AT_EOF 020 +#define WS_TRAILING_SPACE (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF) #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB) extern unsigned whitespace_rule_cfg; extern unsigned whitespace_rule(const char *); @@ -174,6 +174,175 @@ static struct diff_tempfile { char tmp_path[PATH_MAX]; } diff_temp[2]; +typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len); + +struct emit_callback { + int color_diff; + unsigned ws_rule; + int blank_at_eof_in_preimage; + int blank_at_eof_in_postimage; + int lno_in_preimage; + int lno_in_postimage; + sane_truncate_fn truncate; + const char **label_path; + struct diff_words_data *diff_words; + int *found_changesp; + FILE *file; +}; + +static int count_lines(const char *data, int size) +{ + int count, ch, completely_empty = 1, nl_just_seen = 0; + count = 0; + while (0 < size--) { + ch = *data++; + if (ch == '\n') { + count++; + nl_just_seen = 1; + completely_empty = 0; + } + else { + nl_just_seen = 0; + completely_empty = 0; + } + } + if (completely_empty) + return 0; + if (!nl_just_seen) + count++; /* no trailing newline */ + return count; +} + +static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one) +{ + if (!DIFF_FILE_VALID(one)) { + mf->ptr = (char *)""; /* does not matter */ + mf->size = 0; + return 0; + } + else if (diff_populate_filespec(one, 0)) + return -1; + + mf->ptr = one->data; + mf->size = one->size; + return 0; +} + +static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule) +{ + char *ptr = mf->ptr; + long size = mf->size; + int cnt = 0; + + if (!size) + return cnt; + ptr += size - 1; /* pointing at the very end */ + if (*ptr != '\n') + ; /* incomplete line */ + else + ptr--; /* skip the last LF */ + while (mf->ptr < ptr) { + char *prev_eol; + for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--) + if (*prev_eol == '\n') + break; + if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule)) + break; + cnt++; + ptr = prev_eol - 1; + } + return cnt; +} + +static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, + struct emit_callback *ecbdata) +{ + int l1, l2, at; + unsigned ws_rule = ecbdata->ws_rule; + l1 = count_trailing_blank(mf1, ws_rule); + l2 = count_trailing_blank(mf2, ws_rule); + if (l2 <= l1) { + ecbdata->blank_at_eof_in_preimage = 0; + ecbdata->blank_at_eof_in_postimage = 0; + return; + } + at = count_lines(mf1->ptr, mf1->size); + ecbdata->blank_at_eof_in_preimage = (at - l1) + 1; + + at = count_lines(mf2->ptr, mf2->size); + ecbdata->blank_at_eof_in_postimage = (at - l2) + 1; +} + +static void emit_line_0(FILE *file, const char *set, const char *reset, + int first, const char *line, int len) +{ + int has_trailing_newline, has_trailing_carriage_return; + int nofirst; + + if (len == 0) { + has_trailing_newline = (first == '\n'); + has_trailing_carriage_return = (!has_trailing_newline && + (first == '\r')); + nofirst = has_trailing_newline || has_trailing_carriage_return; + } else { + has_trailing_newline = (len > 0 && line[len-1] == '\n'); + if (has_trailing_newline) + len--; + has_trailing_carriage_return = (len > 0 && line[len-1] == '\r'); + if (has_trailing_carriage_return) + len--; + nofirst = 0; + } + + fputs(set, file); + + if (!nofirst) + fputc(first, file); + fwrite(line, len, 1, file); + fputs(reset, file); + if (has_trailing_carriage_return) + fputc('\r', file); + if (has_trailing_newline) + fputc('\n', file); +} + +static void emit_line(FILE *file, const char *set, const char *reset, + const char *line, int len) +{ + emit_line_0(file, set, reset, line[0], line+1, len-1); +} + +static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len) +{ + if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) && + ecbdata->blank_at_eof_in_preimage && + ecbdata->blank_at_eof_in_postimage && + ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage && + ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage)) + return 0; + return ws_blank_line(line, len, ecbdata->ws_rule); +} + +static void emit_add_line(const char *reset, + struct emit_callback *ecbdata, + const char *line, int len) +{ + const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE); + const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW); + + if (!*ws) + emit_line_0(ecbdata->file, set, reset, '+', line, len); + else if (new_blank_line_at_eof(ecbdata, line, len)) + /* Blank line at EOF - paint '+' as well */ + emit_line_0(ecbdata->file, ws, reset, '+', line, len); + else { + /* Emit just the prefix, then the rest. */ + emit_line_0(ecbdata->file, set, reset, '+', "", 0); + ws_check_emit(line, len, ecbdata->ws_rule, + ecbdata->file, set, reset, ws); + } +} + static struct diff_tempfile *claim_diff_tempfile(void) { int i; for (i = 0; i < ARRAY_SIZE(diff_temp); i++) @@ -201,29 +370,6 @@ static void remove_tempfile_on_signal(int signo) raise(signo); } -static int count_lines(const char *data, int size) -{ - int count, ch, completely_empty = 1, nl_just_seen = 0; - count = 0; - while (0 < size--) { - ch = *data++; - if (ch == '\n') { - count++; - nl_just_seen = 1; - completely_empty = 0; - } - else { - nl_just_seen = 0; - completely_empty = 0; - } - } - if (completely_empty) - return 0; - if (!nl_just_seen) - count++; /* no trailing newline */ - return count; -} - static void print_line_count(FILE *file, int count) { switch (count) { @@ -239,26 +385,36 @@ static void print_line_count(FILE *file, int count) } } -static void copy_file_with_prefix(FILE *file, - int prefix, const char *data, int size, - const char *set, const char *reset) +static void emit_rewrite_lines(struct emit_callback *ecb, + int prefix, const char *data, int size) { - int ch, nl_just_seen = 1; - while (0 < size--) { - ch = *data++; - if (nl_just_seen) { - fputs(set, file); - putc(prefix, file); + const char *endp = NULL; + static const char *nneof = " No newline at end of file\n"; + const char *old = diff_get_color(ecb->color_diff, DIFF_FILE_OLD); + const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET); + + while (0 < size) { + int len; + + endp = memchr(data, '\n', size); + len = endp ? (endp - data + 1) : size; + if (prefix != '+') { + ecb->lno_in_preimage++; + emit_line_0(ecb->file, old, reset, '-', + data, len); + } else { + ecb->lno_in_postimage++; + emit_add_line(reset, ecb, data, len); } - if (ch == '\n') { - nl_just_seen = 1; - fputs(reset, file); - } else - nl_just_seen = 0; - putc(ch, file); + size -= len; + data += len; + } + if (!endp) { + const char *plain = diff_get_color(ecb->color_diff, + DIFF_PLAIN); + emit_line_0(ecb->file, plain, reset, '\\', + nneof, strlen(nneof)); } - if (!nl_just_seen) - fprintf(file, "%s\n\\ No newline at end of file\n", reset); } static void emit_rewrite_diff(const char *name_a, @@ -274,13 +430,12 @@ static void emit_rewrite_diff(const char *name_a, const char *name_a_tab, *name_b_tab; const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO); const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO); - const char *old = diff_get_color(color_diff, DIFF_FILE_OLD); - const char *new = diff_get_color(color_diff, DIFF_FILE_NEW); const char *reset = diff_get_color(color_diff, DIFF_RESET); static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT; const char *a_prefix, *b_prefix; const char *data_one, *data_two; size_t size_one, size_two; + struct emit_callback ecbdata; if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) { a_prefix = o->b_prefix; @@ -321,6 +476,22 @@ static void emit_rewrite_diff(const char *name_a, size_two = two->size; } + memset(&ecbdata, 0, sizeof(ecbdata)); + ecbdata.color_diff = color_diff; + ecbdata.found_changesp = &o->found_changes; + ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); + ecbdata.file = o->file; + if (ecbdata.ws_rule & WS_BLANK_AT_EOF) { + mmfile_t mf1, mf2; + mf1.ptr = (char *)data_one; + mf2.ptr = (char *)data_two; + mf1.size = size_one; + mf2.size = size_two; + check_blank_at_eof(&mf1, &mf2, &ecbdata); + } + ecbdata.lno_in_preimage = 1; + ecbdata.lno_in_postimage = 1; + lc_a = count_lines(data_one, size_one); lc_b = count_lines(data_two, size_two); fprintf(o->file, @@ -332,24 +503,9 @@ static void emit_rewrite_diff(const char *name_a, print_line_count(o->file, lc_b); fprintf(o->file, " @@%s\n", reset); if (lc_a) - copy_file_with_prefix(o->file, '-', data_one, size_one, old, reset); + emit_rewrite_lines(&ecbdata, '-', data_one, size_one); if (lc_b) - copy_file_with_prefix(o->file, '+', data_two, size_two, new, reset); -} - -static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one) -{ - if (!DIFF_FILE_VALID(one)) { - mf->ptr = (char *)""; /* does not matter */ - mf->size = 0; - return 0; - } - else if (diff_populate_filespec(one, 0)) - return -1; - - mf->ptr = one->data; - mf->size = one->size; - return 0; + emit_rewrite_lines(&ecbdata, '+', data_two, size_two); } struct diff_words_buffer { @@ -529,18 +685,6 @@ static void diff_words_show(struct diff_words_data *diff_words) diff_words->minus.text.size = diff_words->plus.text.size = 0; } -typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len); - -struct emit_callback { - int nparents, color_diff; - unsigned ws_rule; - sane_truncate_fn truncate; - const char **label_path; - struct diff_words_data *diff_words; - int *found_changesp; - FILE *file; -}; - static void free_diff_words_data(struct emit_callback *ecbdata) { if (ecbdata->diff_words) { @@ -566,42 +710,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix) return ""; } -static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len) -{ - int has_trailing_newline, has_trailing_carriage_return; - - has_trailing_newline = (len > 0 && line[len-1] == '\n'); - if (has_trailing_newline) - len--; - has_trailing_carriage_return = (len > 0 && line[len-1] == '\r'); - if (has_trailing_carriage_return) - len--; - - fputs(set, file); - fwrite(line, len, 1, file); - fputs(reset, file); - if (has_trailing_carriage_return) - fputc('\r', file); - if (has_trailing_newline) - fputc('\n', file); -} - -static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len) -{ - const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE); - const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW); - - if (!*ws) - emit_line(ecbdata->file, set, reset, line, len); - else { - /* Emit just the prefix, then the rest. */ - emit_line(ecbdata->file, set, reset, line, ecbdata->nparents); - ws_check_emit(line + ecbdata->nparents, - len - ecbdata->nparents, ecbdata->ws_rule, - ecbdata->file, set, reset, ws); - } -} - static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len) { const char *cp; @@ -620,10 +728,23 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u return allot - l; } +static void find_lno(const char *line, struct emit_callback *ecbdata) +{ + const char *p; + ecbdata->lno_in_preimage = 0; + ecbdata->lno_in_postimage = 0; + p = strchr(line, '-'); + if (!p) + return; /* cannot happen */ + ecbdata->lno_in_preimage = strtol(p + 1, NULL, 10); + p = strchr(p, '+'); + if (!p) + return; /* cannot happen */ + ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10); +} + static void fn_out_consume(void *priv, char *line, unsigned long len) { - int i; - int color; struct emit_callback *ecbdata = priv; const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO); const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN); @@ -650,14 +771,9 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) len = 1; } - /* This is not really necessary for now because - * this codepath only deals with two-way diffs. - */ - for (i = 0; i < len && line[i] == '@'; i++) - ; - if (2 <= i && i < len && line[i] == ' ') { - ecbdata->nparents = i - 1; + if (line[0] == '@') { len = sane_truncate_line(ecbdata, line, len); + find_lno(line, ecbdata); emit_line(ecbdata->file, diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO), reset, line, len); @@ -666,15 +782,11 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) return; } - if (len < ecbdata->nparents) { + if (len < 1) { emit_line(ecbdata->file, reset, reset, line, len); return; } - color = DIFF_PLAIN; - if (ecbdata->diff_words && ecbdata->nparents != 1) - /* fall back to normal diff */ - free_diff_words_data(ecbdata); if (ecbdata->diff_words) { if (line[0] == '-') { diff_words_append(line, len, @@ -693,20 +805,19 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) emit_line(ecbdata->file, plain, reset, line, len); return; } - for (i = 0; i < ecbdata->nparents && len; i++) { - if (line[i] == '-') - color = DIFF_FILE_OLD; - else if (line[i] == '+') - color = DIFF_FILE_NEW; - } - if (color != DIFF_FILE_NEW) { - emit_line(ecbdata->file, - diff_get_color(ecbdata->color_diff, color), - reset, line, len); - return; + if (line[0] != '+') { + const char *color = + diff_get_color(ecbdata->color_diff, + line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN); + ecbdata->lno_in_preimage++; + if (line[0] == ' ') + ecbdata->lno_in_postimage++; + emit_line(ecbdata->file, color, reset, line, len); + } else { + ecbdata->lno_in_postimage++; + emit_add_line(reset, ecbdata, line + 1, len - 1); } - emit_add_line(reset, ecbdata, line, len); } static char *pprint_rename(const char *a, const char *b) @@ -1211,7 +1322,6 @@ struct checkdiff_t { struct diff_options *o; unsigned ws_rule; unsigned status; - int trailing_blanks_start; }; static int is_conflict_marker(const char *line, unsigned long len) @@ -1255,10 +1365,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) if (line[0] == '+') { unsigned bad; data->lineno++; - if (!ws_blank_line(line + 1, len - 1, data->ws_rule)) - data->trailing_blanks_start = 0; - else if (!data->trailing_blanks_start) - data->trailing_blanks_start = data->lineno; if (is_conflict_marker(line + 1, len - 1)) { data->status |= 1; fprintf(data->o->file, @@ -1278,14 +1384,12 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) data->o->file, set, reset, ws); } else if (line[0] == ' ') { data->lineno++; - data->trailing_blanks_start = 0; } else if (line[0] == '@') { char *plus = strchr(line, '+'); if (plus) data->lineno = strtol(plus, NULL, 10) - 1; else die("invalid diff"); - data->trailing_blanks_start = 0; } } @@ -1562,6 +1666,8 @@ static void builtin_diff(const char *name_a, ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF); ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); + if (ecbdata.ws_rule & WS_BLANK_AT_EOF) + check_blank_at_eof(&mf1, &mf2, &ecbdata); ecbdata.file = o->file; xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; @@ -1704,11 +1810,22 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data, &xpp, &xecfg, &ecb); - if ((data.ws_rule & WS_TRAILING_SPACE) && - data.trailing_blanks_start) { - fprintf(o->file, "%s:%d: ends with blank lines.\n", - data.filename, data.trailing_blanks_start); - data.status = 1; /* report errors */ + if (data.ws_rule & WS_BLANK_AT_EOF) { + struct emit_callback ecbdata; + int blank_at_eof; + + ecbdata.ws_rule = data.ws_rule; + check_blank_at_eof(&mf1, &mf2, &ecbdata); + blank_at_eof = ecbdata.blank_at_eof_in_preimage; + + if (blank_at_eof) { + static char *err; + if (!err) + err = whitespace_error_string(WS_BLANK_AT_EOF); + fprintf(o->file, "%s:%d: %s.\n", + data.filename, blank_at_eof, err); + data.status = 1; /* report errors */ + } } } free_and_return: diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 6d13da30da..8dd147d78f 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -362,10 +362,17 @@ test_expect_success 'line numbers in --check output are correct' ' ' -test_expect_success 'checkdiff detects trailing blank lines' ' +test_expect_success 'checkdiff detects new trailing blank lines (1)' ' echo "foo();" >x && echo "" >>x && - git diff --check | grep "ends with blank" + git diff --check | grep "new blank line" +' + +test_expect_success 'checkdiff detects new trailing blank lines (2)' ' + { echo a; echo b; echo; echo; } >x && + git add x && + { echo a; echo; echo; echo; echo; } >x && + git diff --check | grep "new blank line" ' test_expect_success 'checkdiff allows new blank lines' ' diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh index 84a1fe3115..3a3663fbcb 100755 --- a/t/t4019-diff-wserror.sh +++ b/t/t4019-diff-wserror.sh @@ -165,7 +165,7 @@ test_expect_success 'trailing empty lines (1)' ' rm -f .gitattributes && test_must_fail git diff --check >output && - grep "ends with blank lines." output && + grep "new blank line at" output && grep "trailing whitespace" output ' @@ -190,4 +190,13 @@ test_expect_success 'do not color trailing cr in context' ' ' +test_expect_success 'color new trailing blank lines' ' + { echo a; echo b; echo; echo; } >x && + git add x && + { echo a; echo; echo; echo; echo c; echo; echo; echo; echo; } >x && + git diff --color x >output && + cnt=$(grep "${blue_grep}" output | wc -l) && + test $cnt = 2 +' + test_done diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index fac2093d7f..ca26397590 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -170,4 +170,95 @@ test_expect_success 'trailing whitespace & no newline at the end of file' ' grep "^$" target ' +test_expect_success 'blank at EOF with --whitespace=fix (1)' ' + : these can fail depending on what we did before + git config --unset core.whitespace + rm -f .gitattributes + + { echo a; echo b; echo c; } >one && + git add one && + { echo a; echo b; echo c; } >expect && + { cat expect; echo; } >one && + git diff -- one >patch && + + git checkout one && + git apply --whitespace=fix patch && + test_cmp expect one +' + +test_expect_success 'blank at EOF with --whitespace=fix (2)' ' + { echo a; echo b; echo c; } >one && + git add one && + { echo a; echo c; } >expect && + { cat expect; echo; echo; } >one && + git diff -- one >patch && + + git checkout one && + git apply --whitespace=fix patch && + test_cmp expect one +' + +test_expect_success 'blank at EOF with --whitespace=fix (3)' ' + { echo a; echo b; echo; } >one && + git add one && + { echo a; echo c; echo; } >expect && + { cat expect; echo; echo; } >one && + git diff -- one >patch && + + git checkout one && + git apply --whitespace=fix patch && + test_cmp expect one +' + +test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' ' + { echo a; echo b; echo; echo; echo; echo; echo; echo d; } >one && + git add one && + { echo a; echo c; echo; echo; echo; echo; echo; echo; echo d; } >expect && + cp expect one && + git diff -- one >patch && + + git checkout one && + git apply --whitespace=fix patch && + 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_expect_success 'blank but not empty at EOF' ' + { 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_done @@ -16,6 +16,8 @@ static struct whitespace_rule { { "space-before-tab", WS_SPACE_BEFORE_TAB, 0 }, { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB, 0 }, { "cr-at-eol", WS_CR_AT_EOL, 1 }, + { "blank-at-eol", WS_BLANK_AT_EOL, 0 }, + { "blank-at-eof", WS_BLANK_AT_EOF, 0 }, }; unsigned parse_whitespace_rule(const char *string) @@ -102,8 +104,17 @@ unsigned whitespace_rule(const char *pathname) char *whitespace_error_string(unsigned ws) { struct strbuf err = STRBUF_INIT; - if (ws & WS_TRAILING_SPACE) + if ((ws & WS_TRAILING_SPACE) == WS_TRAILING_SPACE) strbuf_addstr(&err, "trailing whitespace"); + else { + if (ws & WS_BLANK_AT_EOL) + strbuf_addstr(&err, "trailing whitespace"); + if (ws & WS_BLANK_AT_EOF) { + if (err.len) + strbuf_addstr(&err, ", "); + strbuf_addstr(&err, "new blank line at EOF"); + } + } if (ws & WS_SPACE_BEFORE_TAB) { if (err.len) strbuf_addstr(&err, ", "); @@ -141,11 +152,11 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule, } /* Check for trailing whitespace. */ - if (ws_rule & WS_TRAILING_SPACE) { + if (ws_rule & WS_BLANK_AT_EOL) { for (i = len - 1; i >= 0; i--) { if (isspace(line[i])) { trailing_whitespace = i; - result |= WS_TRAILING_SPACE; + result |= WS_BLANK_AT_EOL; } else break; @@ -261,7 +272,7 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro /* * Strip trailing whitespace */ - if (ws_rule & WS_TRAILING_SPACE) { + if (ws_rule & WS_BLANK_AT_EOL) { if (0 < len && src[len - 1] == '\n') { add_nl_to_tail = 1; len--; |