Skip to content

Commit fd50623

Browse files
committed
Merge branch 'jk/diff-color-moved-fix' into maint
The experimental "color moved lines differently in diff output" feature was buggy around "ignore whitespace changes" edges, whihch has been corrected. * jk/diff-color-moved-fix: diff: handle NULs in get_string_hash() diff: fix whitespace-skipping with --color-moved t4015: test the output of "diff --color-moved -b" t4015: check "negative" case for "-w --color-moved" t4015: refactor --color-moved whitespace test
2 parents e18b1df + b66b507 commit fd50623

File tree

2 files changed

+188
-42
lines changed

2 files changed

+188
-42
lines changed

diff.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ static int next_byte(const char **cp, const char **endp,
712712
{
713713
int retval;
714714

715-
if (*cp > *endp)
715+
if (*cp >= *endp)
716716
return -1;
717717

718718
if (isspace(**cp)) {
@@ -729,7 +729,12 @@ static int next_byte(const char **cp, const char **endp,
729729
if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {
730730
while (*cp < *endp && isspace(**cp))
731731
(*cp)++;
732-
/* return the first non-ws character via the usual below */
732+
/*
733+
* return the first non-ws character via the usual
734+
* below, unless we ate all of the bytes
735+
*/
736+
if (*cp >= *endp)
737+
return -1;
733738
}
734739
}
735740

@@ -750,9 +755,9 @@ static int moved_entry_cmp(const struct diff_options *diffopt,
750755
return a->es->len != b->es->len || memcmp(ap, bp, a->es->len);
751756

752757
if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_AT_EOL)) {
753-
while (ae > ap && isspace(*ae))
758+
while (ae > ap && isspace(ae[-1]))
754759
ae--;
755-
while (be > bp && isspace(*be))
760+
while (be > bp && isspace(be[-1]))
756761
be--;
757762
}
758763

@@ -775,9 +780,9 @@ static unsigned get_string_hash(struct emitted_diff_symbol *es, struct diff_opti
775780
int c;
776781

777782
strbuf_reset(&sb);
778-
while (ae > ap && isspace(*ae))
783+
while (ae > ap && isspace(ae[-1]))
779784
ae--;
780-
while ((c = next_byte(&ap, &ae, o)) > 0)
785+
while ((c = next_byte(&ap, &ae, o)) >= 0)
781786
strbuf_addch(&sb, c);
782787

783788
return memhash(sb.buf, sb.len);

t/t4015-diff-whitespace.sh

Lines changed: 177 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,48 +1318,59 @@ test_expect_success 'no effect from --color-moved with --word-diff' '
13181318
test_cmp expect actual
13191319
'
13201320

1321-
test_expect_success 'move detection ignoring whitespace ' '
1321+
test_expect_success 'set up whitespace tests' '
13221322
git reset --hard &&
1323-
cat <<\EOF >lines.txt &&
1324-
line 1
1325-
line 2
1326-
line 3
1327-
line 4
1328-
long line 5
1329-
long line 6
1330-
long line 7
1331-
EOF
1332-
git add lines.txt &&
1333-
git commit -m "add poetry" &&
1334-
cat <<\EOF >lines.txt &&
1335-
long line 5
1323+
# Note that these lines have no leading or trailing whitespace.
1324+
cat <<-\EOF >lines.txt &&
1325+
line 1
1326+
line 2
1327+
line 3
1328+
line 4
1329+
line 5
13361330
long line 6
13371331
long line 7
1338-
line 1
1339-
line 2
1340-
line 3
1341-
line 4
1342-
EOF
1343-
test_config color.diff.oldMoved "magenta" &&
1344-
test_config color.diff.newMoved "cyan" &&
1332+
long line 8
1333+
long line 9
1334+
EOF
1335+
git add lines.txt &&
1336+
git commit -m "add poetry" &&
1337+
git config color.diff.oldMoved "magenta" &&
1338+
git config color.diff.newMoved "cyan"
1339+
'
1340+
1341+
test_expect_success 'move detection ignoring whitespace ' '
1342+
q_to_tab <<-\EOF >lines.txt &&
1343+
Qlong line 6
1344+
Qlong line 7
1345+
Qlong line 8
1346+
Qchanged long line 9
1347+
line 1
1348+
line 2
1349+
line 3
1350+
line 4
1351+
line 5
1352+
EOF
13451353
git diff HEAD --no-renames --color-moved --color |
13461354
grep -v "index" |
13471355
test_decode_color >actual &&
13481356
cat <<-\EOF >expected &&
13491357
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
13501358
<BOLD>--- a/lines.txt<RESET>
13511359
<BOLD>+++ b/lines.txt<RESET>
1352-
<CYAN>@@ -1,7 +1,7 @@<RESET>
1353-
<GREEN>+<RESET> <GREEN>long line 5<RESET>
1360+
<CYAN>@@ -1,9 +1,9 @@<RESET>
13541361
<GREEN>+<RESET> <GREEN>long line 6<RESET>
13551362
<GREEN>+<RESET> <GREEN>long line 7<RESET>
1363+
<GREEN>+<RESET> <GREEN>long line 8<RESET>
1364+
<GREEN>+<RESET> <GREEN>changed long line 9<RESET>
13561365
line 1<RESET>
13571366
line 2<RESET>
13581367
line 3<RESET>
13591368
line 4<RESET>
1360-
<RED>-long line 5<RESET>
1369+
line 5<RESET>
13611370
<RED>-long line 6<RESET>
13621371
<RED>-long line 7<RESET>
1372+
<RED>-long line 8<RESET>
1373+
<RED>-long line 9<RESET>
13631374
EOF
13641375
test_cmp expected actual &&
13651376
@@ -1370,21 +1381,160 @@ EOF
13701381
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
13711382
<BOLD>--- a/lines.txt<RESET>
13721383
<BOLD>+++ b/lines.txt<RESET>
1373-
<CYAN>@@ -1,7 +1,7 @@<RESET>
1374-
<CYAN>+<RESET> <CYAN>long line 5<RESET>
1384+
<CYAN>@@ -1,9 +1,9 @@<RESET>
13751385
<CYAN>+<RESET> <CYAN>long line 6<RESET>
13761386
<CYAN>+<RESET> <CYAN>long line 7<RESET>
1387+
<CYAN>+<RESET> <CYAN>long line 8<RESET>
1388+
<GREEN>+<RESET> <GREEN>changed long line 9<RESET>
1389+
line 1<RESET>
1390+
line 2<RESET>
1391+
line 3<RESET>
1392+
line 4<RESET>
1393+
line 5<RESET>
1394+
<MAGENTA>-long line 6<RESET>
1395+
<MAGENTA>-long line 7<RESET>
1396+
<MAGENTA>-long line 8<RESET>
1397+
<RED>-long line 9<RESET>
1398+
EOF
1399+
test_cmp expected actual
1400+
'
1401+
1402+
test_expect_success 'move detection ignoring whitespace changes' '
1403+
git reset --hard &&
1404+
# Lines 6-8 have a space change, but 9 is new whitespace
1405+
q_to_tab <<-\EOF >lines.txt &&
1406+
longQline 6
1407+
longQline 7
1408+
longQline 8
1409+
long liQne 9
1410+
line 1
1411+
line 2
1412+
line 3
1413+
line 4
1414+
line 5
1415+
EOF
1416+
1417+
git diff HEAD --no-renames --color-moved --color |
1418+
grep -v "index" |
1419+
test_decode_color >actual &&
1420+
cat <<-\EOF >expected &&
1421+
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
1422+
<BOLD>--- a/lines.txt<RESET>
1423+
<BOLD>+++ b/lines.txt<RESET>
1424+
<CYAN>@@ -1,9 +1,9 @@<RESET>
1425+
<GREEN>+<RESET><GREEN>long line 6<RESET>
1426+
<GREEN>+<RESET><GREEN>long line 7<RESET>
1427+
<GREEN>+<RESET><GREEN>long line 8<RESET>
1428+
<GREEN>+<RESET><GREEN>long li ne 9<RESET>
1429+
line 1<RESET>
1430+
line 2<RESET>
1431+
line 3<RESET>
1432+
line 4<RESET>
1433+
line 5<RESET>
1434+
<RED>-long line 6<RESET>
1435+
<RED>-long line 7<RESET>
1436+
<RED>-long line 8<RESET>
1437+
<RED>-long line 9<RESET>
1438+
EOF
1439+
test_cmp expected actual &&
1440+
1441+
git diff HEAD --no-renames -b --color-moved --color |
1442+
grep -v "index" |
1443+
test_decode_color >actual &&
1444+
cat <<-\EOF >expected &&
1445+
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
1446+
<BOLD>--- a/lines.txt<RESET>
1447+
<BOLD>+++ b/lines.txt<RESET>
1448+
<CYAN>@@ -1,9 +1,9 @@<RESET>
1449+
<CYAN>+<RESET><CYAN>long line 6<RESET>
1450+
<CYAN>+<RESET><CYAN>long line 7<RESET>
1451+
<CYAN>+<RESET><CYAN>long line 8<RESET>
1452+
<GREEN>+<RESET><GREEN>long li ne 9<RESET>
1453+
line 1<RESET>
1454+
line 2<RESET>
1455+
line 3<RESET>
1456+
line 4<RESET>
1457+
line 5<RESET>
1458+
<MAGENTA>-long line 6<RESET>
1459+
<MAGENTA>-long line 7<RESET>
1460+
<MAGENTA>-long line 8<RESET>
1461+
<RED>-long line 9<RESET>
1462+
EOF
1463+
test_cmp expected actual
1464+
'
1465+
1466+
test_expect_success 'move detection ignoring whitespace at eol' '
1467+
git reset --hard &&
1468+
# Lines 6-9 have new eol whitespace, but 9 also has it in the middle
1469+
q_to_tab <<-\EOF >lines.txt &&
1470+
long line 6Q
1471+
long line 7Q
1472+
long line 8Q
1473+
longQline 9Q
1474+
line 1
1475+
line 2
1476+
line 3
1477+
line 4
1478+
line 5
1479+
EOF
1480+
1481+
# avoid cluttering the output with complaints about our eol whitespace
1482+
test_config core.whitespace -blank-at-eol &&
1483+
1484+
git diff HEAD --no-renames --color-moved --color |
1485+
grep -v "index" |
1486+
test_decode_color >actual &&
1487+
cat <<-\EOF >expected &&
1488+
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
1489+
<BOLD>--- a/lines.txt<RESET>
1490+
<BOLD>+++ b/lines.txt<RESET>
1491+
<CYAN>@@ -1,9 +1,9 @@<RESET>
1492+
<GREEN>+<RESET><GREEN>long line 6 <RESET>
1493+
<GREEN>+<RESET><GREEN>long line 7 <RESET>
1494+
<GREEN>+<RESET><GREEN>long line 8 <RESET>
1495+
<GREEN>+<RESET><GREEN>long line 9 <RESET>
1496+
line 1<RESET>
1497+
line 2<RESET>
1498+
line 3<RESET>
1499+
line 4<RESET>
1500+
line 5<RESET>
1501+
<RED>-long line 6<RESET>
1502+
<RED>-long line 7<RESET>
1503+
<RED>-long line 8<RESET>
1504+
<RED>-long line 9<RESET>
1505+
EOF
1506+
test_cmp expected actual &&
1507+
1508+
git diff HEAD --no-renames --ignore-space-at-eol --color-moved --color |
1509+
grep -v "index" |
1510+
test_decode_color >actual &&
1511+
cat <<-\EOF >expected &&
1512+
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
1513+
<BOLD>--- a/lines.txt<RESET>
1514+
<BOLD>+++ b/lines.txt<RESET>
1515+
<CYAN>@@ -1,9 +1,9 @@<RESET>
1516+
<CYAN>+<RESET><CYAN>long line 6 <RESET>
1517+
<CYAN>+<RESET><CYAN>long line 7 <RESET>
1518+
<CYAN>+<RESET><CYAN>long line 8 <RESET>
1519+
<GREEN>+<RESET><GREEN>long line 9 <RESET>
13771520
line 1<RESET>
13781521
line 2<RESET>
13791522
line 3<RESET>
13801523
line 4<RESET>
1381-
<MAGENTA>-long line 5<RESET>
1524+
line 5<RESET>
13821525
<MAGENTA>-long line 6<RESET>
13831526
<MAGENTA>-long line 7<RESET>
1527+
<MAGENTA>-long line 8<RESET>
1528+
<RED>-long line 9<RESET>
13841529
EOF
13851530
test_cmp expected actual
13861531
'
13871532

1533+
test_expect_success 'clean up whitespace-test colors' '
1534+
git config --unset color.diff.oldMoved &&
1535+
git config --unset color.diff.newMoved
1536+
'
1537+
13881538
test_expect_success '--color-moved block at end of diff output respects MIN_ALNUM_COUNT' '
13891539
git reset --hard &&
13901540
>bar &&
@@ -1530,13 +1680,4 @@ test_expect_success 'move detection with submodules' '
15301680
test_cmp expect decoded_actual
15311681
'
15321682

1533-
test_expect_success 'move detection with whitespace changes' '
1534-
test_when_finished "git reset --hard" &&
1535-
test_seq 10 >test &&
1536-
git add test &&
1537-
sed s/3/42/ <test >test.tmp &&
1538-
mv test.tmp test &&
1539-
git -c diff.colormoved diff --ignore-space-change -- test
1540-
'
1541-
15421683
test_done

0 commit comments

Comments
 (0)