From b8d9c1a66b99ad3ca8069add010dafdd1bc6cab8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 23:59:25 -0700 Subject: diff.c: the builtin_diff() deals with only two-file comparison The combined diff is implemented in combine_diff() and fn_out_consume() codepath never has to deal with anything but two-file comparision. Drop nparents from the emit_callback structure and simplify the code. Signed-off-by: Junio C Hamano --- diff.c | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 6fea3c034..1eddd590e 100644 --- a/diff.c +++ b/diff.c @@ -489,7 +489,7 @@ typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len); struct emit_callback { struct xdiff_emit_state xm; - int nparents, color_diff; + int color_diff; unsigned ws_rule; sane_truncate_fn truncate; const char **label_path; @@ -549,9 +549,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons 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, + emit_line(ecbdata->file, set, reset, line, 1); + ws_check_emit(line + 1, len - 1, ecbdata->ws_rule, ecbdata->file, set, reset, ws); } } @@ -576,7 +575,6 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u 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); @@ -598,13 +596,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) ecbdata->label_path[0] = ecbdata->label_path[1] = NULL; } - /* 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); emit_line(ecbdata->file, diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO), @@ -614,15 +606,12 @@ 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, @@ -641,13 +630,10 @@ 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 (line[0] == '-') + color = DIFF_FILE_OLD; + else if (line[0] == '+') + color = DIFF_FILE_NEW; if (color != DIFF_FILE_NEW) { emit_line(ecbdata->file, diff_get_color(ecbdata->color_diff, color), -- cgit v1.2.1 From 5b5061efd88e1d113a4484369dfab654b43364de Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 22:30:27 -0700 Subject: diff --whitespace=warn/error: obey blank-at-eof The "diff --check" code used to conflate trailing-space whitespace error class with this, but now we have a proper separate error class, we should check it under blank-at-eof, not trailing-space. The whitespace error is not about _having_ blank lines at end, but about adding _new_ blank lines. To keep the message consistent with what is given by "git apply", call whitespace_error_string() to generate it, instead of using a hardcoded custom message. Signed-off-by: Junio C Hamano --- diff.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 1eddd590e..a693d184c 100644 --- a/diff.c +++ b/diff.c @@ -1650,10 +1650,14 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, ecb.priv = &data; xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb); - if ((data.ws_rule & WS_TRAILING_SPACE) && + if ((data.ws_rule & WS_BLANK_AT_EOF) && data.trailing_blanks_start) { - fprintf(o->file, "%s:%d: ends with blank lines.\n", - data.filename, data.trailing_blanks_start); + static char *err; + + if (!err) + err = whitespace_error_string(WS_BLANK_AT_EOF); + fprintf(o->file, "%s:%d: %s\n", + data.filename, data.trailing_blanks_start, err); data.status = 1; /* report errors */ } } -- cgit v1.2.1 From 467babf8d059caee9587567452fc8b46505b4e67 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 3 Sep 2009 23:39:43 -0700 Subject: diff --whitespace=warn/error: fix blank-at-eof check The "diff --check" logic used to share the same issue as the one fixed for "git apply" earlier in this series, in that a patch that adds new blank lines at end could appear as @@ -l,5 +m,7 @@$ _context$ _context$ -deleted$ +$ +$ +$ _$ _$ where _ stands for SP and $ shows a end-of-line. Instead of looking at each line in the patch in the callback, simply count the blank lines from the end in two versions, and notice the presence of new ones. Signed-off-by: Junio C Hamano --- diff.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 16 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index a693d184c..c19c4760f 100644 --- a/diff.c +++ b/diff.c @@ -1149,7 +1149,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) @@ -1193,10 +1192,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, @@ -1216,14 +1211,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; } } @@ -1437,6 +1430,44 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi return NULL; } +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 int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule) +{ + int l1, l2, at; + l1 = count_trailing_blank(mf1, ws_rule); + l2 = count_trailing_blank(mf2, ws_rule); + if (l2 <= l1) + return 0; + /* starting where? */ + at = count_lines(mf1->ptr, mf1->size); + return (at - l1) + 1; /* the line number counts from 1 */ +} + static void builtin_diff(const char *name_a, const char *name_b, struct diff_filespec *one, @@ -1650,15 +1681,16 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, ecb.priv = &data; xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb); - if ((data.ws_rule & WS_BLANK_AT_EOF) && - data.trailing_blanks_start) { - static char *err; - - if (!err) - err = whitespace_error_string(WS_BLANK_AT_EOF); - fprintf(o->file, "%s:%d: %s\n", - data.filename, data.trailing_blanks_start, err); - data.status = 1; /* report errors */ + if (data.ws_rule & WS_BLANK_AT_EOF) { + int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule); + 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: -- cgit v1.2.1 From 690ed8436326484fe7e3f4deac4cffd780c7d630 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 4 Sep 2009 00:41:15 -0700 Subject: diff --color: color blank-at-eof Since the coloring logic processed the patch output one line at a time, we couldn't easily color code the new blank lines at the end of file. Reuse the adds_blank_at_eof() function to find where the runs of such blank lines start, keep track of the line number in the preimage while processing the patch output one line at a time, and paint the new blank lines that appear after that line to implement this. Signed-off-by: Junio C Hamano --- diff.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index c19c4760f..2b285b8ce 100644 --- a/diff.c +++ b/diff.c @@ -491,6 +491,8 @@ struct emit_callback { struct xdiff_emit_state xm; int color_diff; unsigned ws_rule; + int blank_at_eof; + int lno_in_preimage; sane_truncate_fn truncate; const char **label_path; struct diff_words_data *diff_words; @@ -547,6 +549,12 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons if (!*ws) emit_line(ecbdata->file, set, reset, line, len); + else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) && + ecbdata->blank_at_eof && + (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) && + ws_blank_line(line + 1, len - 1, ecbdata->ws_rule)) + /* Blank line at EOF */ + emit_line(ecbdata->file, ws, reset, line, len); else { /* Emit just the prefix, then the rest. */ emit_line(ecbdata->file, set, reset, line, 1); @@ -573,9 +581,16 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u return allot - l; } +static int find_preimage_lno(const char *line) +{ + char *p = strchr(line, '-'); + if (!p) + return 0; /* should not happen */ + return strtol(p+1, NULL, 10); +} + static void fn_out_consume(void *priv, char *line, unsigned long len) { - 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); @@ -598,6 +613,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) if (line[0] == '@') { len = sane_truncate_line(ecbdata, line, len); + ecbdata->lno_in_preimage = find_preimage_lno(line); emit_line(ecbdata->file, diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO), reset, line, len); @@ -611,7 +627,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) return; } - color = DIFF_PLAIN; if (ecbdata->diff_words) { if (line[0] == '-') { diff_words_append(line, len, @@ -630,14 +645,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) emit_line(ecbdata->file, plain, reset, line, len); return; } - if (line[0] == '-') - color = DIFF_FILE_OLD; - else if (line[0] == '+') - color = DIFF_FILE_NEW; - if (color != DIFF_FILE_NEW) { - emit_line(ecbdata->file, - diff_get_color(ecbdata->color_diff, color), - reset, line, len); + + if (line[0] != '+') { + const char *color = + diff_get_color(ecbdata->color_diff, + line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN); + ecbdata->lno_in_preimage++; + emit_line(ecbdata->file, color, reset, line, len); return; } emit_add_line(reset, ecbdata, line, len); @@ -1557,6 +1571,9 @@ 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) + ecbdata.blank_at_eof = + adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule); ecbdata.file = o->file; xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; -- cgit v1.2.1 From d68fe26f3e03b230ac9bbbcf002a9acdc4bebde9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 14 Sep 2009 22:05:57 -0700 Subject: diff --whitespace: fix blank lines at end The earlier logic tried to colour any and all blank lines that were added beyond the last blank line in the original, but this was very wrong. If you added 96 blank lines, a non-blank line, and then 3 blank lines at the end, only the last 3 lines should trigger the error, not the earlier 96 blank lines. We need to also make sure that the lines are after the last non-blank line in the postimage as well before deciding to paint them. Signed-off-by: Junio C Hamano --- diff.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 21 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 2b285b8ce..63a3bfc33 100644 --- a/diff.c +++ b/diff.c @@ -491,8 +491,10 @@ struct emit_callback { struct xdiff_emit_state xm; int color_diff; unsigned ws_rule; - int blank_at_eof; + 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; @@ -542,6 +544,17 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char fputc('\n', file); } +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 + 1, len - 1, 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); @@ -549,11 +562,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons if (!*ws) emit_line(ecbdata->file, set, reset, line, len); - else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) && - ecbdata->blank_at_eof && - (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) && - ws_blank_line(line + 1, len - 1, ecbdata->ws_rule)) - /* Blank line at EOF */ + else if (new_blank_line_at_eof(ecbdata, line, len)) + /* Blank line at EOF - paint '+' as well */ emit_line(ecbdata->file, ws, reset, line, len); else { /* Emit just the prefix, then the rest. */ @@ -581,12 +591,19 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u return allot - l; } -static int find_preimage_lno(const char *line) +static void find_lno(const char *line, struct emit_callback *ecbdata) { - char *p = strchr(line, '-'); + const char *p; + ecbdata->lno_in_preimage = 0; + ecbdata->lno_in_postimage = 0; + p = strchr(line, '-'); if (!p) - return 0; /* should not happen */ - return strtol(p+1, NULL, 10); + 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) @@ -613,7 +630,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) if (line[0] == '@') { len = sane_truncate_line(ecbdata, line, len); - ecbdata->lno_in_preimage = find_preimage_lno(line); + find_lno(line, ecbdata); emit_line(ecbdata->file, diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO), reset, line, len); @@ -651,10 +668,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) 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); - return; + } else { + ecbdata->lno_in_postimage++; + emit_add_line(reset, ecbdata, line, len); } - emit_add_line(reset, ecbdata, line, len); } static char *pprint_rename(const char *a, const char *b) @@ -1470,16 +1490,23 @@ static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule) return cnt; } -static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule) +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) - return 0; - /* starting where? */ + 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); - return (at - l1) + 1; /* the line number counts from 1 */ + 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 builtin_diff(const char *name_a, @@ -1572,8 +1599,7 @@ static void builtin_diff(const char *name_a, 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) - ecbdata.blank_at_eof = - adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule); + check_blank_at_eof(&mf1, &mf2, &ecbdata); ecbdata.file = o->file; xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; @@ -1699,7 +1725,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb); if (data.ws_rule & WS_BLANK_AT_EOF) { - int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule); + 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) -- cgit v1.2.1