revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-22 21:40:55 +01:00
Code Issues Releases Wiki Activity

When mapping phantom comments across changes, correct an off-by-one issue

Browse source 
Summary:
Ref T13617. When an inline comment is added inside a block of added lines, it currently ends up off-by-one when porting forward.

This is a disagreement between the mapping engine and the display engine about what "offset" means. Choose the simpler of the two interpretations.

Test Plan:
  - Created a revision with the diff in T13617.
  - Added an inline in the middle of the added block.
  - Updated the revision with the same diff.
    - Before: inline incorrectly moves up by one line.
    - After: inline maps correctly.

Maniphest Tasks: T13617

Differential Revision: https://secure.phabricator.com/D21572
This commit is contained in:
epriestley 2021-02-20 19:58:04 -08:00
parent 20a54a3006
commit 6bfa990254
3 changed files with 46 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__phutil_library_map__.php
View file

@ -6519,7 +6519,7 @@ phutil_register_library_map(array(
'DarkConsoleXHProfPluginAPI' => 'Phobject',
'DifferentialAction' => 'Phobject',
'DifferentialActionEmailCommand' => 'MetaMTAEmailTransactionCommand',
'DifferentialAdjustmentMapTestCase' => 'PhutilTestCase',
'DifferentialAdjustmentMapTestCase' => 'PhabricatorTestCase',
'DifferentialAffectedPath' => 'DifferentialDAO',
'DifferentialAsanaRepresentationField' => 'DifferentialCustomField',
'DifferentialAuditorsCommitMessageField' => 'DifferentialCommitMessageCustomField',

2
src/applications/differential/parser/DifferentialLineAdjustmentMap.php
View file

@ -78,7 +78,7 @@ final class DifferentialLineAdjustmentMap extends Phobject {
// If we're tracing the first line and this block is collapsing,
// compute the offset from the top of the block.
if (!$is_end && $this->isInverse) {
$offset = 0;
$offset = 1;
$cursor = $line - 1;
while (isset($nmap[$cursor])) {
$prev = $nmap[$cursor];

49
src/applications/differential/storage/__tests__/DifferentialAdjustmentMapTestCase.php
View file

@ -1,6 +1,7 @@
<?php
final class DifferentialAdjustmentMapTestCase extends PhutilTestCase {
final class DifferentialAdjustmentMapTestCase
extends PhabricatorTestCase {
public function testBasicMaps() {
$change_map = array(
@ -62,7 +63,6 @@ final class DifferentialAdjustmentMapTestCase extends PhutilTestCase {
DifferentialLineAdjustmentMap::newFromHunks($hunks)->getMap());
}
public function testInverseMaps() {
$change_map = array(
1 => array(1),
@ -221,6 +221,45 @@ final class DifferentialAdjustmentMapTestCase extends PhutilTestCase {
$this->assertEqual(3, $map->getFinalOffset());
}
public function testUnchangedUpdate() {
$diff1 = $this->loadHunks('insert.diff');
$diff2 = $this->loadHunks('insert.diff');
$map = DifferentialLineAdjustmentMap::newInverseMap(
DifferentialLineAdjustmentMap::newFromHunks($diff1));
$map->addMapToChain(
DifferentialLineAdjustmentMap::newFromHunks($diff2));
$actual = array();
for ($ii = 1; $ii <= 16; $ii++) {
$actual[$ii] = array(
$map->mapLine($ii, false),
$map->mapLine($ii, true),
);
}
$expect = array(
1 => array(array(false, false, 1), array(false, false, 1)),
2 => array(array(false, false, 2), array(false, false, 2)),
3 => array(array(false, false, 3), array(false, false, 3)),
4 => array(array(false, false, 4), array(false, false, 4)),
5 => array(array(false, false, 5), array(false, false, 5)),
6 => array(array(false, false, 6), array(false, false, 6)),
7 => array(array(false, false, 7), array(false, false, 7)),
8 => array(array(false, false, 8), array(false, false, 8)),
9 => array(array(false, false, 9), array(false, false, 9)),
10 => array(array(false, false, 10), array(false, false, 13)),
11 => array(array(false, 1, 10), array(false, false, 14)),
12 => array(array(false, 2, 10), array(false, false, 14)),
13 => array(array(false, 3, 10), array(false, false, 14)),
14 => array(array(false, false, 14), array(false, false, 14)),
15 => array(array(false, false, 15), array(false, false, 15)),
16 => array(array(false, false, 16), array(false, false, 16)),
);
$this->assertEqual($expect, $actual);
}
public function testChainMaps() {
// This test simulates porting inlines forward across a rebase.
@ -258,9 +297,9 @@ final class DifferentialAdjustmentMapTestCase extends PhutilTestCase {
5 => array(array(false, false, 3), array(false, false, 3)),
6 => array(array(false, false, 4), array(false, false, 4)),
7 => array(array(false, false, 5), array(false, false, 8)),
8 => array(array(false, 0, 5), array(false, false, 9)),
9 => array(array(false, 1, 5), array(false, false, 9)),
10 => array(array(false, 2, 5), array(false, false, 9)),
8 => array(array(false, 1, 5), array(false, false, 9)),
9 => array(array(false, 2, 5), array(false, false, 9)),
10 => array(array(false, 3, 5), array(false, false, 9)),
11 => array(array(false, false, 9), array(false, false, 9)),
12 => array(array(false, false, 10), array(false, false, 10)),
13 => array(array(false, false, 11), array(false, false, 11)),