Skip to content

Commit 467babf

Browse files
committed
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 <[email protected]>
1 parent 5b5061e commit 467babf

File tree

2 files changed

+55
-16
lines changed

2 files changed

+55
-16
lines changed

diff.c

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,6 @@ struct checkdiff_t {
11491149
struct diff_options *o;
11501150
unsigned ws_rule;
11511151
unsigned status;
1152-
int trailing_blanks_start;
11531152
};
11541153

11551154
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)
11931192
if (line[0] == '+') {
11941193
unsigned bad;
11951194
data->lineno++;
1196-
if (!ws_blank_line(line + 1, len - 1, data->ws_rule))
1197-
data->trailing_blanks_start = 0;
1198-
else if (!data->trailing_blanks_start)
1199-
data->trailing_blanks_start = data->lineno;
12001195
if (is_conflict_marker(line + 1, len - 1)) {
12011196
data->status |= 1;
12021197
fprintf(data->o->file,
@@ -1216,14 +1211,12 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
12161211
data->o->file, set, reset, ws);
12171212
} else if (line[0] == ' ') {
12181213
data->lineno++;
1219-
data->trailing_blanks_start = 0;
12201214
} else if (line[0] == '@') {
12211215
char *plus = strchr(line, '+');
12221216
if (plus)
12231217
data->lineno = strtol(plus, NULL, 10) - 1;
12241218
else
12251219
die("invalid diff");
1226-
data->trailing_blanks_start = 0;
12271220
}
12281221
}
12291222

@@ -1437,6 +1430,44 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi
14371430
return NULL;
14381431
}
14391432

1433+
static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
1434+
{
1435+
char *ptr = mf->ptr;
1436+
long size = mf->size;
1437+
int cnt = 0;
1438+
1439+
if (!size)
1440+
return cnt;
1441+
ptr += size - 1; /* pointing at the very end */
1442+
if (*ptr != '\n')
1443+
; /* incomplete line */
1444+
else
1445+
ptr--; /* skip the last LF */
1446+
while (mf->ptr < ptr) {
1447+
char *prev_eol;
1448+
for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
1449+
if (*prev_eol == '\n')
1450+
break;
1451+
if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
1452+
break;
1453+
cnt++;
1454+
ptr = prev_eol - 1;
1455+
}
1456+
return cnt;
1457+
}
1458+
1459+
static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)
1460+
{
1461+
int l1, l2, at;
1462+
l1 = count_trailing_blank(mf1, ws_rule);
1463+
l2 = count_trailing_blank(mf2, ws_rule);
1464+
if (l2 <= l1)
1465+
return 0;
1466+
/* starting where? */
1467+
at = count_lines(mf1->ptr, mf1->size);
1468+
return (at - l1) + 1; /* the line number counts from 1 */
1469+
}
1470+
14401471
static void builtin_diff(const char *name_a,
14411472
const char *name_b,
14421473
struct diff_filespec *one,
@@ -1650,15 +1681,16 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
16501681
ecb.priv = &data;
16511682
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
16521683

1653-
if ((data.ws_rule & WS_BLANK_AT_EOF) &&
1654-
data.trailing_blanks_start) {
1655-
static char *err;
1656-
1657-
if (!err)
1658-
err = whitespace_error_string(WS_BLANK_AT_EOF);
1659-
fprintf(o->file, "%s:%d: %s\n",
1660-
data.filename, data.trailing_blanks_start, err);
1661-
data.status = 1; /* report errors */
1684+
if (data.ws_rule & WS_BLANK_AT_EOF) {
1685+
int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);
1686+
if (blank_at_eof) {
1687+
static char *err;
1688+
if (!err)
1689+
err = whitespace_error_string(WS_BLANK_AT_EOF);
1690+
fprintf(o->file, "%s:%d: %s.\n",
1691+
data.filename, blank_at_eof, err);
1692+
data.status = 1; /* report errors */
1693+
}
16621694
}
16631695
}
16641696
free_and_return:

t/t4015-diff-whitespace.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,13 @@ test_expect_success 'checkdiff detects new trailing blank lines (1)' '
341341
git diff --check | grep "new blank line"
342342
'
343343

344+
test_expect_success 'checkdiff detects new trailing blank lines (2)' '
345+
{ echo a; echo b; echo; echo; } >x &&
346+
git add x &&
347+
{ echo a; echo; echo; echo; echo; } >x &&
348+
git diff --check | grep "new blank line"
349+
'
350+
344351
test_expect_success 'checkdiff allows new blank lines' '
345352
git checkout x &&
346353
mv x y &&

0 commit comments

Comments
 (0)