implement solid move-range approach - tests not running

src/types/AbstractType.js

@@ -33,6 +33,7 @@ const maxSearchMarker = 80
  * @param {AbstractType<any>} yarray
  * @param {number} index
  * @param {function(ListIterator):T} f
+ * @return T
  */
 export const useSearchMarker = (tr, yarray, index, f) => {
   const searchMarker = yarray._searchMarker

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<any>} */ (this._prelimContent).splice(startIndex, endIndex - startIndex + 1)
-      ;/** @type {Array<any>} */ (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<any>} */ (this._prelimContent).splice(start, end - start + 1)
+      const content = /** @type {Array<any>} */ (this._prelimContent).splice(startIndex, endIndex - startIndex + 1)
       ;/** @type {Array<any>} */ (this._prelimContent).splice(target, 0, ...content)
     }
   }

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<Item>}
+   */
+  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<Item>}
+   */
+  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
 }

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)