Skip to content

Commit 551a1b6

Browse files
committed
fix: bug + alignment in DiffNode::makeUnchanged
1 parent ddd4289 commit 551a1b6

1 file changed

Lines changed: 26 additions & 1 deletion

File tree

  • src/main/java/org/variantsync/diffdetective/variation/diff

src/main/java/org/variantsync/diffdetective/variation/diff/DiffNode.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,9 @@ public void makeUnchanged() {
908908
if (bp == null || ap == null) {
909909
// There is only one parent, which we store in this field.
910910
final DiffNode<L> p = bp == null ? ap : bp;
911+
final Time timeOfExistingEdge = bp == null ? AFTER : BEFORE;
912+
final Time timeOfNewEdge = timeOfExistingEdge.other();
913+
911914
Assert.assertTrue(p != null);
912915

913916
// If the parent is not unchanged, we have to make it unchanged so that it can be our
@@ -917,7 +920,29 @@ public void makeUnchanged() {
917920
}
918921

919922
// Now make p our parent at all times, not just at a single time.
920-
Time.forAll(t -> at(t).parent = p);
923+
// To this end, we essentially have to "patch" this node into our parent scope at timeOfNewEdge.
924+
// Technically, this means that we have to add this node to the children list of p at a specific index.
925+
// We run into the alignment problem here if there is an insertion (or multiple insertions) right next to a deleted node we make unchanged or vice versa.
926+
// Hence, the index at which to patch our node is not unique.
927+
// There are multiple heuristics or strategies we could use to determine the index:
928+
// - constant index: always use index 0 for example
929+
// - line numbers: use the index right before the node with a higher line number at timeOfNewEdge
930+
// This solution requires knowledge on line numbers which are not always present (e.g., in diffs generated in code).
931+
// - context-based patching: Try to locate the node where its neighbors at timeOfNewEdge are most similar to the neighbors at timeOfExistingEdge
932+
// This requires some knowledge on the labels to match contexts.
933+
// We lightweight context-based patching here by trying to insert the node directly right of its closest unchanged left neighbor.
934+
int patchIndex = 0; // the index at which to insert this node at timeOfNewEdge
935+
final List<DiffNode<L>> siblingsAndMe = p.getChildOrder(timeOfExistingEdge);
936+
// We start walking from our closest left neighbor towards the leftmost sibling (at index 0) and try to find the first unchanged sibling.
937+
for (int i = p.indexOfChild(this, timeOfExistingEdge) - 1; i >= 0; i--) {
938+
final DiffNode<L> candidate = siblingsAndMe.get(i);
939+
if (candidate.isNon()) { // i.e., exists at timeOfNewEdge as well
940+
// Insert ourselves as the new right neighbor of the candidate node
941+
patchIndex = p.indexOfChild(candidate, timeOfNewEdge) + 1;
942+
break;
943+
}
944+
}
945+
p.insertChild(this, patchIndex, timeOfNewEdge);
921946
}
922947
}
923948

0 commit comments

Comments
 (0)