@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Fix an unusual issue with intradiff highlighting of files with uncommon end-of-file modifications
Summary:
Fixes T13539. See that task for discussion and a reproduction case.
This algorithm currently counts "\ No newline at end of file" lines as though they were normal source lines. This can cause offset issues in the rare case that a diff contains two of these lines (for each side of the file) and has changes between them (because the last line of the file was modified between the diffs).
Instead, don't count "\" as a display line.
Test Plan:
- See T13539 and PHI1740.
- Before: got fatals on the "wild" diff and the synthetic simplified version.
- After: clean intradiff rendering in both cases.
Maniphest Tasks: T13539
Differential Revision: https://secure.phabricator.com/D21267
+38
-14
+31
-7
src/applications/differential/parser/DifferentialHunkParser.php
+31
-7
src/applications/differential/parser/DifferentialHunkParser.php
···
582
582
$changes = $hunk->getSplitLines();
583
583
foreach ($changes as $line) {
584
584
$diff_type = $line[0]; // Change type in diff of diffs.
585
+
$is_same = ($diff_type === ' ');
586
+
$is_add = ($diff_type === '+');
587
+
$is_rem = ($diff_type === '-');
588
+
585
589
$orig_type = $line[1]; // Change type in the original diff.
586
-
if ($diff_type == ' ') {
590
+
591
+
if ($is_same) {
587
592
// Use the same key for lines that are next to each other.
588
593
if ($olds_cursor > $news_cursor) {
589
594
$key = $olds_cursor + 1;
···
594
599
$news[$key] = null;
595
600
$olds_cursor = $key;
596
601
$news_cursor = $key;
597
-
} else if ($diff_type == '-') {
602
+
} else if ($is_rem) {
598
603
$olds[] = array($n_old, $orig_type);
599
604
$olds_cursor++;
600
-
} else if ($diff_type == '+') {
605
+
} else if ($is_add) {
601
606
$news[] = array($n_new, $orig_type);
602
607
$news_cursor++;
608
+
} else {
609
+
throw new Exception(
610
+
pht(
611
+
'Found unknown intradiff source line, expected a line '.
612
+
'beginning with "+", "-", or " " (space): %s.',
613
+
$line));
603
614
}
604
-
if (($diff_type == '-' || $diff_type == ' ') && $orig_type != '-') {
615
+
616
+
// See T13539. Don't increment the line count if this line was removed,
617
+
// or if the line is a "No newline at end of file" marker.
618
+
$not_a_line = ($orig_type === '-' || $orig_type === '\\');
619
+
if ($not_a_line) {
620
+
continue;
621
+
}
622
+
623
+
if ($is_same || $is_rem) {
605
624
$n_old++;
606
625
}
607
-
if (($diff_type == '+' || $diff_type == ' ') && $orig_type != '-') {
626
+
627
+
if ($is_same || $is_add) {
608
628
$n_new++;
609
629
}
610
630
}
···
623
643
list($n, $type) = $olds[$i];
624
644
if ($type == '+' ||
625
645
($type == ' ' && isset($news[$i]) && $news[$i][1] != ' ')) {
626
-
$highlight_old[] = $offsets_old[$n];
646
+
if (isset($offsets_old[$n])) {
647
+
$highlight_old[] = $offsets_old[$n];
648
+
}
627
649
}
628
650
}
629
651
if (isset($news[$i])) {
630
652
list($n, $type) = $news[$i];
631
653
if ($type == '+' ||
632
654
($type == ' ' && isset($olds[$i]) && $olds[$i][1] != ' ')) {
633
-
$highlight_new[] = $offsets_new[$n];
655
+
if (isset($offsets_new[$n])) {
656
+
$highlight_new[] = $offsets_new[$n];
657
+
}
634
658
}
635
659
}
636
660
}
+7
-7
src/applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php
+7
-7
src/applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php
···
4
4
5
5
public function testDiffChangesets() {
6
6
$hunk = new DifferentialHunk();
7
-
$hunk->setChanges("+a\n b\n-c");
7
+
$hunk->setChanges("+a\n b\n-c\n");
8
8
$hunk->setNewOffset(1);
9
9
$hunk->setNewLen(2);
10
10
$left = new DifferentialChangeset();
11
11
$left->attachHunks(array($hunk));
12
12
13
13
$tests = array(
14
-
"+a\n b\n-c" => array(array(), array()),
15
-
"+a\n x\n-c" => array(array(), array()),
16
-
"+aa\n b\n-c" => array(array(1), array(11)),
17
-
" b\n-c" => array(array(1), array()),
18
-
"+a\n b\n c" => array(array(), array(13)),
19
-
"+a\n x\n c" => array(array(), array(13)),
14
+
"+a\n b\n-c\n" => array(array(), array()),
15
+
"+a\n x\n-c\n" => array(array(), array()),
16
+
"+aa\n b\n-c\n" => array(array(1), array(11)),
17
+
" b\n-c\n" => array(array(1), array()),
18
+
"+a\n b\n c\n" => array(array(), array(13)),
19
+
"+a\n x\n c\n" => array(array(), array(13)),
20
20
);
21
21
22
22
foreach ($tests as $changes => $expected) {