I believe there is a bug in document.js
on line 1363:
const segmentOldStart =
lastChange
? traverse(>>>lastChange.oldEnd<<<, traversal(segmentNewStart, lastChange.newEnd))
: segmentNewStart
If I understand the intent of the authors correctly, then it should traverse
from lastChange.newEnd
, not oldEnd
.
Here's basic example. Lets assume there are two operations to integrate in the following order (1, then 2):
int foo() {
/* (2) insert
multiple lines */
// (1) delete single line
}
You will have them reordered in collectSegments
because of segment index (which I believe is based on the appearance order, rather than modification time/sequence number). Now having operations [2, 1] being processed by computeChangesForSegments
, you need to adjust absolute positions of operation 1, to make sure it accounts for all the integrated changes that appear in the document before it. These calls to traverse
/traversal
are supposed to do it, but traverse
call is adding the distance to previous segment's (modification 2) oldEnd
position, which for multi-line changes will be different from newEnd
: old (end=start) is line 1 (insertion on line 1), but new is line 2 (start=1, end=2). This causes subsequent delete operation to be adjusted off by 1 line. This, in turn, when applied to the actual editor will delete text on the previous line (multiple lines */
) rather than the expected one (// (1) delete single line
).
So if that is true, and it should be changed to newEnd
, then it might as well be replaced with just the segmentNewStart
, because it will always produce the same value: you measure distance from lastChange.newEnd
to segmentNewStart
and apply it to lastChange.newEnd
.
In fact, if my understanding of what is old
and new
(before and after current modification?) is correct, then I can't think of a case where oldStart
will be different from a newStart
(both insertion or deletion).