Skip to content

Commit d68fe26

Browse files
committed
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 <[email protected]>
1 parent aeb84b0 commit d68fe26

File tree

2 files changed

+54
-22
lines changed

2 files changed

+54
-22
lines changed

diff.c

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -491,8 +491,10 @@ struct emit_callback {
491491
struct xdiff_emit_state xm;
492492
int color_diff;
493493
unsigned ws_rule;
494-
int blank_at_eof;
494+
int blank_at_eof_in_preimage;
495+
int blank_at_eof_in_postimage;
495496
int lno_in_preimage;
497+
int lno_in_postimage;
496498
sane_truncate_fn truncate;
497499
const char **label_path;
498500
struct diff_words_data *diff_words;
@@ -542,18 +544,26 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
542544
fputc('\n', file);
543545
}
544546

547+
static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
548+
{
549+
if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
550+
ecbdata->blank_at_eof_in_preimage &&
551+
ecbdata->blank_at_eof_in_postimage &&
552+
ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
553+
ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
554+
return 0;
555+
return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
556+
}
557+
545558
static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
546559
{
547560
const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
548561
const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
549562

550563
if (!*ws)
551564
emit_line(ecbdata->file, set, reset, line, len);
552-
else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
553-
ecbdata->blank_at_eof &&
554-
(ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&
555-
ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))
556-
/* Blank line at EOF */
565+
else if (new_blank_line_at_eof(ecbdata, line, len))
566+
/* Blank line at EOF - paint '+' as well */
557567
emit_line(ecbdata->file, ws, reset, line, len);
558568
else {
559569
/* Emit just the prefix, then the rest. */
@@ -581,12 +591,19 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
581591
return allot - l;
582592
}
583593

584-
static int find_preimage_lno(const char *line)
594+
static void find_lno(const char *line, struct emit_callback *ecbdata)
585595
{
586-
char *p = strchr(line, '-');
596+
const char *p;
597+
ecbdata->lno_in_preimage = 0;
598+
ecbdata->lno_in_postimage = 0;
599+
p = strchr(line, '-');
587600
if (!p)
588-
return 0; /* should not happen */
589-
return strtol(p+1, NULL, 10);
601+
return; /* cannot happen */
602+
ecbdata->lno_in_preimage = strtol(p + 1, NULL, 10);
603+
p = strchr(p, '+');
604+
if (!p)
605+
return; /* cannot happen */
606+
ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
590607
}
591608

592609
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)
613630

614631
if (line[0] == '@') {
615632
len = sane_truncate_line(ecbdata, line, len);
616-
ecbdata->lno_in_preimage = find_preimage_lno(line);
633+
find_lno(line, ecbdata);
617634
emit_line(ecbdata->file,
618635
diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
619636
reset, line, len);
@@ -651,10 +668,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
651668
diff_get_color(ecbdata->color_diff,
652669
line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
653670
ecbdata->lno_in_preimage++;
671+
if (line[0] == ' ')
672+
ecbdata->lno_in_postimage++;
654673
emit_line(ecbdata->file, color, reset, line, len);
655-
return;
674+
} else {
675+
ecbdata->lno_in_postimage++;
676+
emit_add_line(reset, ecbdata, line, len);
656677
}
657-
emit_add_line(reset, ecbdata, line, len);
658678
}
659679

660680
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)
14701490
return cnt;
14711491
}
14721492

1473-
static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)
1493+
static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
1494+
struct emit_callback *ecbdata)
14741495
{
14751496
int l1, l2, at;
1497+
unsigned ws_rule = ecbdata->ws_rule;
14761498
l1 = count_trailing_blank(mf1, ws_rule);
14771499
l2 = count_trailing_blank(mf2, ws_rule);
1478-
if (l2 <= l1)
1479-
return 0;
1480-
/* starting where? */
1500+
if (l2 <= l1) {
1501+
ecbdata->blank_at_eof_in_preimage = 0;
1502+
ecbdata->blank_at_eof_in_postimage = 0;
1503+
return;
1504+
}
14811505
at = count_lines(mf1->ptr, mf1->size);
1482-
return (at - l1) + 1; /* the line number counts from 1 */
1506+
ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
1507+
1508+
at = count_lines(mf2->ptr, mf2->size);
1509+
ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
14831510
}
14841511

14851512
static void builtin_diff(const char *name_a,
@@ -1572,8 +1599,7 @@ static void builtin_diff(const char *name_a,
15721599
ecbdata.found_changesp = &o->found_changes;
15731600
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
15741601
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
1575-
ecbdata.blank_at_eof =
1576-
adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);
1602+
check_blank_at_eof(&mf1, &mf2, &ecbdata);
15771603
ecbdata.file = o->file;
15781604
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
15791605
xecfg.ctxlen = o->context;
@@ -1699,7 +1725,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
16991725
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
17001726

17011727
if (data.ws_rule & WS_BLANK_AT_EOF) {
1702-
int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);
1728+
struct emit_callback ecbdata;
1729+
int blank_at_eof;
1730+
1731+
ecbdata.ws_rule = data.ws_rule;
1732+
check_blank_at_eof(&mf1, &mf2, &ecbdata);
1733+
blank_at_eof = ecbdata.blank_at_eof_in_preimage;
1734+
17031735
if (blank_at_eof) {
17041736
static char *err;
17051737
if (!err)

t/t4019-diff-wserror.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ test_expect_success 'do not color trailing cr in context' '
193193
test_expect_success 'color new trailing blank lines' '
194194
{ echo a; echo b; echo; echo; } >x &&
195195
git add x &&
196-
{ echo a; echo; echo; echo; echo; } >x &&
196+
{ echo a; echo; echo; echo; echo c; echo; echo; echo; echo; } >x &&
197197
git diff --color x >output &&
198198
cnt=$(grep "${blue_grep}" output | wc -l) &&
199199
test $cnt = 2

0 commit comments

Comments
 (0)