diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 150e3d38..49870c53 100644 --- a/src/structs/ContentMove.js +++ b/src/structs/ContentMove.js @@ -18,7 +18,7 @@ export const getMovedCoords = (moved, tr) => { let end // this (exclusive) is the first item after start that is not part of the moved area if (moved.start.item) { if (moved.start.assoc < 0) { - start = getItemCleanEnd(tr, moved.start.item) + start = getItemCleanEnd(tr, moved.start.item) // @todo Try using getItem after all tests succeed again. start = start.right } else { start = getItemCleanStart(tr, moved.start.item) diff --git a/src/types/YArray.js b/src/types/YArray.js index e04f5793..28d71bbc 100644 --- a/src/types/YArray.js +++ b/src/types/YArray.js @@ -152,11 +152,11 @@ export class YArray extends AbstractType { } if (this.doc !== null) { transact(this.doc, transaction => { - const left = createRelativePositionFromTypeIndex(this, index, 1) - const right = left.clone() - right.assoc = -1 + const start = createRelativePositionFromTypeIndex(this, index, 1) + const end = start.clone() + end.assoc = -1 useSearchMarker(transaction, this, target, walker => { - walker.insertMove(transaction, left, right) + walker.insertMove(transaction, [{ start, end }]) }) }) } else { @@ -165,6 +165,32 @@ export class YArray extends AbstractType { } } + /** + * @param {number} startIndex Inclusive move-start + * @param {number} endIndex Inclusive move-end + * @param {number} target + * @param {number} assocStart >=0 if start should be associated with the right character. See relative-position assoc parameter. + * @param {number} assocEnd >= 0 if end should be associated with the right character. + */ + moveRange (startIndex, endIndex, target, assocStart = 1, assocEnd = -1) { + if (startIndex <= target && target <= endIndex) { + // It doesn't make sense to move a range into the same range (it's basically a no-op). + return + } + if (this.doc !== null) { + transact(this.doc, transaction => { + const start = createRelativePositionFromTypeIndex(this, startIndex, assocStart) + const end = createRelativePositionFromTypeIndex(this, endIndex + 1, assocEnd) + useSearchMarker(transaction, this, target, walker => { + walker.insertMove(transaction, [{ start, end }]) + }) + }) + } else { + const content = /** @type {Array} */ (this._prelimContent).splice(startIndex, endIndex - startIndex + 1) + ;/** @type {Array} */ (this._prelimContent).splice(target, 0, ...content) + } + } + /** * @param {number} start Inclusive move-start * @param {number} end Inclusive move-end @@ -172,17 +198,18 @@ export class YArray extends AbstractType { * @param {number} assocStart >=0 if start should be associated with the right character. See relative-position assoc parameter. * @param {number} assocEnd >= 0 if end should be associated with the right character. */ - moveRange (start, end, target, assocStart = 1, assocEnd = -1) { + moveRangeNew (start, end, target, assocStart = 1, assocEnd = -1) { if (start <= target && target <= end) { // It doesn't make sense to move a range into the same range (it's basically a no-op). return } if (this.doc !== null) { transact(this.doc, transaction => { - const left = createRelativePositionFromTypeIndex(this, start, assocStart) - const right = createRelativePositionFromTypeIndex(this, end + 1, assocEnd) + const ranges = useSearchMarker(transaction, this, target, walker => + getMinimalListViewRanges(walker, start, end, assocStart, assocEnd) + ) useSearchMarker(transaction, this, target, walker => { - walker.insertMove(transaction, left, right) + walker.insertMove(transaction, ranges) }) }) } else { diff --git a/src/utils/ListIterator.js b/src/utils/ListIterator.js index 146a2a98..55c2dd34 100644 --- a/src/utils/ListIterator.js +++ b/src/utils/ListIterator.js @@ -206,16 +206,36 @@ export class ListIterator { } /** + * We prefer to insert content outside of a moved range. + * Try to escape the moved range by walking to the left over deleted items. + * * @param {Transaction} tr */ - reduceMoves (tr) { - let item = this.nextItem - if (item !== null) { - while (item === this.currMoveStart) { - item = /** @type {Item} */ (this.currMove) // we iterate to the left after the current condition - popMovedStack(tr, this) + reduceMoveDepth (tr) { + let nextItem = this.nextItem + if (nextItem !== null) { + while (this.currMove) { + if (nextItem === this.currMoveStart) { + nextItem = /** @type {Item} */ (this.currMove) // we iterate to the left after the current condition + popMovedStack(tr, this) + continue + } + // check if we can iterate to the left while stepping over deleted items until we find an item === this.currMoveStart + /** + * @type {Item} nextItem + */ + let item = nextItem + while (item.deleted && item.moved === this.currMove && item !== this.currMoveStart) { + item = /** @type {Item} */ (item.left) // this must exist otherwise we miscalculated the move + } + if (item === this.currMoveStart) { + // we only want to iterate over deleted items if we can escape a move + nextItem = item + } else { + break + } } - this.nextItem = item + this.nextItem = nextItem } } @@ -391,7 +411,7 @@ export class ListIterator { * @param {Array} content */ insertContents (tr, content) { - this.reduceMoves(tr) + this.reduceMoveDepth(tr) this._splitRel(tr) const parent = this.type const store = tr.doc.store @@ -419,11 +439,10 @@ export class ListIterator { /** * @param {Transaction} tr - * @param {RelativePosition} start - * @param {RelativePosition} end + * @param {Array<{ start: RelativePosition, end: RelativePosition }>} ranges */ - insertMove (tr, start, end) { - this.insertContents(tr, [new ContentMove(start, end, -1)]) // @todo adjust priority + insertMove (tr, ranges) { + this.insertContents(tr, ranges.map(range => new ContentMove(range.start, range.end, -1))) // @todo is there a better alrogirthm to update searchmarkers? We could simply remove the markers that are in the updated range. // Also note that searchmarkers are updated in insertContents as well. const sm = this.type._searchMarker @@ -563,3 +582,56 @@ const concatArrayContent = (content, added) => { content.push(...added) return content } + +/** + * @param {Transaction} tr + * @param {ListIterator} walker + * @param {number} len + */ +const getMinimalListViewRanges = (tr, walker, len) => { + walker.reduceMoveDepth(tr) + if (walker.index + len > walker.type._length) { + throw lengthExceeded + } + walker.index += len + /** + * We store nextItem in a variable because this version cannot be null. + */ + let nextItem = /** @type {Item} */ (walker.nextItem) + const ranges = [] + while (len > 0 && !walker.reachedEnd) { + while (nextItem.countable && !walker.reachedEnd && len > 0 && nextItem !== walker.currMoveEnd) { + if (!nextItem.deleted && nextItem.moved === walker.currMove) { + const slicedContent = slice(nextItem.content, this.rel, len) + len -= slicedContent.length + value = concat(value, slicedContent) + if (this.rel + slicedContent.length === nextItem.length) { + this.rel = 0 + } else { + this.rel += slicedContent.length + continue // do not iterate to item.right + } + } + if (nextItem.right) { + nextItem = nextItem.right + this.nextItem = nextItem + } else { + this.reachedEnd = true + } + } + if ((!this.reachedEnd || this.currMove !== null) && len > 0) { + // always set nextItem before any method call + this.nextItem = nextItem + this.forward(tr, 0) + if (this.nextItem == null) { + throw new Error('debug me') // @todo remove + } + nextItem = this.nextItem + } + } + this.nextItem = nextItem + if (len < 0) { + this.index -= len + } + return ranges +} diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index baa2acb0..8185a4a8 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -569,7 +569,7 @@ const arrayTransactions = [ const newPos = _newPosAdj + (_newPosAdj > yarray.length - len ? len : 0) const oldContent = yarray.toArray() yarray.moveRange(pos, pos + len - 1, newPos) - console.log(`moving range ${pos}-${pos + len} to ${newPos}`) + console.log(`moving range ${pos}-${pos + len - 1} to ${newPos}`) const movedValues = oldContent.splice(pos, len) oldContent.splice(pos < newPos ? newPos - len : newPos, 0, ...movedValues) t.compareArrays(yarray.toArray(), oldContent) // we want to make sure that fastSearch markers insert at the correct position