From 69b7f4bfb9bf903b75b61be46887a3a313d35f32 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Thu, 5 May 2022 13:03:59 +0200 Subject: [PATCH] implement solid move-range approach - tests not running --- src/types/AbstractType.js | 1 + src/types/YArray.js | 41 +++--------- src/utils/ListIterator.js | 130 +++++++++++++++++++++++++++----------- tests/y-array.tests.js | 2 +- 4 files changed, 103 insertions(+), 71 deletions(-) diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index b2188261..41b4afe5 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -33,6 +33,7 @@ const maxSearchMarker = 80 * @param {AbstractType} yarray * @param {number} index * @param {function(ListIterator):T} f + * @return T */ export const useSearchMarker = (tr, yarray, index, f) => { const searchMarker = yarray._searchMarker diff --git a/src/types/YArray.js b/src/types/YArray.js index 28d71bbc..1cb11e74 100644 --- a/src/types/YArray.js +++ b/src/types/YArray.js @@ -11,7 +11,8 @@ import { ListIterator, useSearchMarker, createRelativePositionFromTypeIndex, - UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Transaction, Item // eslint-disable-line + UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Transaction, Item, // eslint-disable-line + getMinimalListViewRanges } from '../internals.js' /** @@ -173,47 +174,23 @@ export class YArray extends AbstractType { * @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). + if ( + (startIndex <= target && target <= endIndex) || // It doesn't make sense to move a range into the same range (it's basically a no-op). + endIndex - startIndex < 0 // require length of >= 0 + ) { 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 - * @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. - */ - 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 ranges = useSearchMarker(transaction, this, target, walker => - getMinimalListViewRanges(walker, start, end, assocStart, assocEnd) + const ranges = useSearchMarker(transaction, this, startIndex, walker => + getMinimalListViewRanges(transaction, walker, endIndex - startIndex + 1) ) useSearchMarker(transaction, this, target, walker => { walker.insertMove(transaction, ranges) }) }) } else { - const content = /** @type {Array} */ (this._prelimContent).splice(start, end - start + 1) + const content = /** @type {Array} */ (this._prelimContent).splice(startIndex, endIndex - startIndex + 1) ;/** @type {Array} */ (this._prelimContent).splice(target, 0, ...content) } } diff --git a/src/utils/ListIterator.js b/src/utils/ListIterator.js index 55c2dd34..07686061 100644 --- a/src/utils/ListIterator.js +++ b/src/utils/ListIterator.js @@ -12,6 +12,7 @@ import { ContentDoc, Doc, compareIDs, + createRelativePosition, RelativePosition, ID, AbstractContent, ContentMove, Transaction, Item, AbstractType // eslint-disable-line } from '../internals.js' @@ -328,7 +329,7 @@ export class ListIterator { } if (nextItem.right) { nextItem = nextItem.right - this.nextItem = nextItem + this.nextItem = nextItem // @todo move this after the while loop } else { this.reachedEnd = true } @@ -584,54 +585,107 @@ const concatArrayContent = (content, added) => { } /** + * Move-ranges must not cross each other. + * + * This function computes the minimal amount of ranges to move a range of content to + * a different place. + * + * Algorithm: + * * Store the current stack in $preStack and $preItem = walker.nextItem + * * Iterate forward $len items. + * * The current stack is stored is $afterStack and $ + * * Delete the stack-items that both of them have in common + * * @param {Transaction} tr * @param {ListIterator} walker * @param {number} len + * @return {Array<{ start: RelativePosition, end: RelativePosition }>} */ -const getMinimalListViewRanges = (tr, walker, len) => { - walker.reduceMoveDepth(tr) +export const getMinimalListViewRanges = (tr, walker, len) => { + if (len === 0) return [] if (walker.index + len > walker.type._length) { throw lengthExceeded } - walker.index += len + // stepping outside the current move-range as much as possible + walker.reduceMoveDepth(tr) + /** - * We store nextItem in a variable because this version cannot be null. + * @type {Array<{ start: RelativePosition, end: RelativePosition }>} */ - 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 - } + // store relevant information for the beginning, before we iterate forward + /** + * @type {Array} + */ + const preStack = walker.movedStack.map(si => si.move) + const preMove = walker.currMove + const preItem = /** @type {Item} */ (walker.nextItem) + const preRel = walker.rel + + walker.forward(tr, len) + + // store the same information for the end, after we iterate forward + /** + * @type {Array} + */ + const afterStack = walker.movedStack.map(si => si.move) + const afterMove = walker.currMove + const afterItem = /** @type {Item} */ (walker.nextItem) + const afterRel = walker.rel + + let start = createRelativePosition(walker.type, createID(preItem.id.client, preItem.id.clock + preRel), 0) + let end = walker.reachedEnd + ? createRelativePosition( + walker.type, + createID(afterItem.id.client, afterItem.id.clock + afterItem.length - 1), + -1 + ) + : createRelativePosition( + walker.type, + createID(afterItem.id.client, afterItem.id.clock + afterRel), + 0 + ) + + if (preMove) { + preStack.push(preMove) } - this.nextItem = nextItem - if (len < 0) { - this.index -= len + if (afterMove) { + afterStack.push(afterMove) } + + // remove common stack-items + while (preStack.length > 0 && preStack[0] === afterStack[0]) { + preStack.shift() + afterStack.shift() + } + + // remove stack-items that are useless for our computation (that wouldn't produce meaningful ranges) + // @todo + + while (preStack.length > 0) { + const move = /** @type {Item} */ (preStack.pop()) + ranges.push({ + start, + end: /** @type {ContentMove} */ (move.content).end + }) + start = createRelativePosition(walker.type, createID(move.id.client, move.id.clock), -1) + } + + const middleMove = { start, end } + ranges.push(middleMove) + + while (afterStack.length > 0) { + const move = /** @type {Item} */ (afterStack.pop()) + ranges.push({ + start: /** @type {ContentMove} */ (move.content).start, + end + }) + end = createRelativePosition(walker.type, createID(move.id.client, move.id.clock), 0) + } + + // Update end of the center move operation + // Move ranges must be applied in order + middleMove.end = end + return ranges } diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index 8185a4a8..e7894c8d 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -563,7 +563,7 @@ const arrayTransactions = [ return } const pos = prng.int32(gen, 0, yarray.length - 1) - const len = 1 // prng.int32(gen, 1, math.min(3, yarray.length - pos)) + const len = 1 // prng.int32(gen, 1, math.min(3, yarray.length - pos)) @todo! const _newPosAdj = prng.int32(gen, 0, yarray.length - len) // make sure that we don't insert in-between the moved range const newPos = _newPosAdj + (_newPosAdj > yarray.length - len ? len : 0)