cleanup

src/index.js

@@ -48,7 +48,6 @@ export {
   findRootTypeKey,
   findIndexSS,
   getItem,
-  typeListToArraySnapshot,
   typeMapGetSnapshot,
   createDocFromSnapshot,
   iterateDeletedStructs,

src/internals.js

@@ -8,7 +8,7 @@ export * from './utils/encoding.js'
 export * from './utils/EventHandler.js'
 export * from './utils/ID.js'
 export * from './utils/isParentOf.js'
-export * from './utils/ListIterator.js'
+export * from './utils/ListWalker.js'
 export * from './utils/logging.js'
 export * from './utils/PermanentUserData.js'
 export * from './utils/RelativePosition.js'

src/structs/ContentMove.js

@@ -13,31 +13,36 @@ import {
 /**
  * @param {ContentMove | { start: RelativePosition, end: RelativePosition }} moved
  * @param {Transaction} tr
+ * @param {boolean} split
  * @return {{ start: Item, end: Item }} $start (inclusive) is the beginning and $end (inclusive) is the end of the moved area
  */
-export const getMovedCoords = (moved, tr) => {
+export const getMovedCoords = (moved, tr, split) => {
+  const store = tr.doc.store
+  const startItem = moved.start.item
+  const endItem = moved.end.item
   let start // this (inclusive) is the beginning of the moved area
   let end // this (exclusive) is the first item after start that is not part of the moved area
-  if (moved.start.item) {
+  if (startItem) {
     if (moved.start.assoc < 0) {
-      start = getItemCleanEnd(tr, moved.start.item) // @todo Try using getItem after all tests succeed again.
+      // We know that the items have already been split, hence getItem suffices.
+      start = split ? getItemCleanEnd(tr, startItem) : getItem(store, startItem)
       start = start.right
     } else {
-      start = getItemCleanStart(tr, moved.start.item)
+      start = split ? getItemCleanStart(tr, startItem) : getItem(store, startItem)
     }
   } else if (moved.start.tname != null) {
     start = tr.doc.get(moved.start.tname)._start
   } else if (moved.start.type) {
-    start = /** @type {ContentType} */ (getItem(tr.doc.store, moved.start.type).content).type._start
+    start = /** @type {ContentType} */ (getItem(store, moved.start.type).content).type._start
   } else {
     error.unexpectedCase()
   }
-  if (moved.end.item) {
+  if (endItem) {
     if (moved.end.assoc < 0) {
-      end = getItemCleanEnd(tr, moved.end.item)
+      end = split ? getItemCleanEnd(tr, endItem) : getItem(store, endItem)
       end = end.right
     } else {
-      end = getItemCleanStart(tr, moved.end.item)
+      end = split ? getItemCleanStart(tr, endItem) : getItem(store, endItem)
     }
   } else {
     error.unexpectedCase()
@@ -60,7 +65,7 @@ export const findMoveLoop = (tr, moved, movedItem, trackedMovedItems) => {
   /**
    * @type {{ start: Item | null, end: Item | null }}
    */
-  let { start, end } = getMovedCoords(moved, tr)
+  let { start, end } = getMovedCoords(moved, tr, false)
   while (start !== end && start != null) {
     if (
       !start.deleted &&
@@ -152,10 +157,11 @@ export class ContentMove {
   integrate (transaction, item) {
     const sm = /** @type {AbstractType<any>} */ (item.parent)._searchMarker
     if (sm) sm.length = 0
+    const movedCoords = getMovedCoords(this, transaction, true)
     /**
-     * @type {{ start: Item | null, end: Item | null }}
+     * @type {{ start: Item | null, end: item | null }}
      */
-    let { start, end } = getMovedCoords(this, transaction)
+    let { start, end } = movedCoords
     let maxPriority = 0
     // If this ContentMove was created locally, we set prio = -1. This indicates
     // that we want to set prio to the current prio-maximum of the moved range.
@@ -169,7 +175,10 @@ export class ContentMove {
             prevMove.deleteAsCleanup(transaction, adaptPriority)
           }
           this.overrides.add(prevMove)
-          transaction._mergeStructs.push(start) // @todo is this needed?
+          if (start !== movedCoords.start) {
+            // only add this to mergeStructs if this is not the first item
+            transaction._mergeStructs.push(start)
+          }
         }
         maxPriority = math.max(maxPriority, nextPrio)
         // was already moved
@@ -201,7 +210,7 @@ export class ContentMove {
     /**
      * @type {{ start: Item | null, end: Item | null }}
      */
-    let { start, end } = getMovedCoords(this, transaction)
+    let { start, end } = getMovedCoords(this, transaction, false)
     while (start !== end && start != null) {
       if (start.moved === item) {
         const prevMoved = transaction.prevMoved.get(start)
@@ -268,7 +277,6 @@ export class ContentMove {
 
 /**
  * @private
- * @todo use binary encoding option for start & end relpos's
  *
  * @param {UpdateDecoderV1 | UpdateDecoderV2} decoder
  * @return {ContentMove}

src/structs/Item.js

@@ -762,7 +762,7 @@ export const readItemContent = (decoder, info) => contentRefs[info & binary.BITS
  * @type {Array<function(UpdateDecoderV1 | UpdateDecoderV2):AbstractContent>}
  */
 export const contentRefs = [
-  () => { error.unexpectedCase() }, // GC is not ItemContent
+  error.unexpectedCase, // GC is not ItemContent
   readContentDeleted, // 1
   readContentJSON, // 2
   readContentBinary, // 3
@@ -772,7 +772,7 @@ export const contentRefs = [
   readContentType, // 7
   readContentAny, // 8
   readContentDoc, // 9
-  () => { error.unexpectedCase() }, // 10 - Skip is not ItemContent
+  error.unexpectedCase, // 10 - Skip is not ItemContent
   readContentMove // 11
 ]
 

src/types/AbstractType.js

@@ -10,8 +10,8 @@ import {
   createID,
   ContentAny,
   ContentBinary,
-  ListIterator,
+  ListWalker,
-  ContentDoc, YText, YArray, UpdateEncoderV1, UpdateEncoderV2, Doc, Snapshot, Transaction, EventHandler, YEvent, Item, // eslint-disable-line
+  ContentDoc, UpdateEncoderV1, UpdateEncoderV2, Doc, Snapshot, Transaction, EventHandler, YEvent, Item, // eslint-disable-line
 } from '../internals.js'
 
 import * as map from 'lib0/map'
@@ -19,7 +19,8 @@ import * as iterator from 'lib0/iterator'
 import * as error from 'lib0/error'
 import * as math from 'lib0/math'
 
-const maxSearchMarker = 80
+const maxSearchMarker = 300
+const freshSearchMarkerDistance = 30
 
 /**
  * Search marker help us to find positions in the associative array faster.
@@ -32,25 +33,25 @@ const maxSearchMarker = 80
  * @param {Transaction} tr
  * @param {AbstractType<any>} yarray
  * @param {number} index
- * @param {function(ListIterator):T} f
+ * @param {function(ListWalker):T} f
  * @return T
  */
 export const useSearchMarker = (tr, yarray, index, f) => {
   const searchMarker = yarray._searchMarker
-  if (searchMarker === null || yarray._start === null) { // @todo add condition `index < 5`
+  if (searchMarker === null || yarray._start === null || index < freshSearchMarkerDistance) {
-    return f(new ListIterator(yarray).forward(tr, index, true))
+    return f(new ListWalker(yarray).forward(tr, index, true))
   }
   if (searchMarker.length === 0) {
-    const sm = new ListIterator(yarray).forward(tr, index, true)
+    const sm = new ListWalker(yarray).forward(tr, index, true)
     searchMarker.push(sm)
     if (sm.nextItem) sm.nextItem.marker = true
   }
   const sm = searchMarker.reduce(
     (a, b, arrayIndex) => math.abs(index - a.index) < math.abs(index - b.index) ? a : b
   )
-  const newIsCheaper = math.abs(sm.index - index) > index // @todo use >= index
+  const newIsCheaper = math.abs(sm.index - index) >= index
-  const createFreshMarker = searchMarker.length < maxSearchMarker && (math.abs(sm.index - index) > 5 || newIsCheaper)
+  const createFreshMarker = searchMarker.length < maxSearchMarker && (math.abs(sm.index - index) > freshSearchMarkerDistance || newIsCheaper)
-  const fsm = createFreshMarker ? (newIsCheaper ? new ListIterator(yarray) : sm.clone()) : sm
+  const fsm = createFreshMarker ? (newIsCheaper ? new ListWalker(yarray) : sm.clone()) : sm
   const prevItem = /** @type {Item} */ (sm.nextItem)
   if (createFreshMarker) {
     searchMarker.push(fsm)
@@ -61,14 +62,6 @@ export const useSearchMarker = (tr, yarray, index, f) => {
   } else {
     fsm.forward(tr, -diff, true)
   }
-  // @todo remove this test
-  /*
-  const otherTesting = new ListIterator(yarray)
-  otherTesting.forward(tr, index)
-  if (otherTesting.nextItem !== fsm.nextItem || otherTesting.index !== fsm.index || otherTesting.reachedEnd !== fsm.reachedEnd) {
-    throw new Error('udtirane')
-  }
-  */
   const result = f(fsm)
   if (fsm.reachedEnd) {
     fsm.reachedEnd = false
@@ -101,10 +94,10 @@ export const useSearchMarker = (tr, yarray, index, f) => {
  *
  * This should be called before doing a deletion!
  *
- * @param {Array<ListIterator>} searchMarker
+ * @param {Array<ListWalker>} searchMarker
  * @param {number} index
  * @param {number} len If insertion, len is positive. If deletion, len is negative.
- * @param {ListIterator|null} origSearchMarker Do not update this searchmarker because it is the one we used to manipulate. @todo !=null for improved perf in ytext
+ * @param {ListWalker|null} origSearchMarker Do not update this searchmarker because it is the one we used to manipulate. @todo !=null for improved perf in ytext
  */
 export const updateMarkerChanges = (searchMarker, index, len, origSearchMarker) => {
   for (let i = searchMarker.length - 1; i >= 0; i--) {
@@ -197,7 +190,7 @@ export class AbstractType {
      */
     this._dEH = createEventHandler()
     /**
-     * @type {null | Array<ListIterator>}
+     * @type {null | Array<ListWalker>}
      */
     this._searchMarker = null
     /**
@@ -376,157 +369,6 @@ export const typeListToArray = type => {
   return cs
 }
 
-/**
- * @param {AbstractType<any>} type
- * @param {Snapshot} snapshot
- * @return {Array<any>}
- *
- * @private
- * @function
- */
-export const typeListToArraySnapshot = (type, snapshot) => {
-  const cs = []
-  let n = type._start
-  while (n !== null) {
-    if (n.countable && isVisible(n, snapshot)) {
-      const c = n.content.getContent()
-      for (let i = 0; i < c.length; i++) {
-        cs.push(c[i])
-      }
-    }
-    n = n.right
-  }
-  return cs
-}
-
-/**
- * Executes a provided function on once on overy element of this YArray.
- *
- * @todo remove!
- *
- * @param {AbstractType<any>} type
- * @param {function(any,number,any):void} f A function to execute on every element of this YArray.
- *
- * @private
- * @function
- */
-export const typeListForEach = (type, f) => {
-  let index = 0
-  let n = type._start
-  while (n !== null) {
-    if (n.countable && !n.deleted) {
-      const c = n.content.getContent()
-      for (let i = 0; i < c.length; i++) {
-        f(c[i], index++, type)
-      }
-    }
-    n = n.right
-  }
-}
-
-/**
- *
- * @todo remove!
- *
- * @template C,R
- * @param {AbstractType<any>} type
- * @param {function(C,number,AbstractType<any>):R} f
- * @return {Array<R>}
- *
- * @private
- * @function
- */
-export const typeListMap = (type, f) => {
-  /**
-   * @type {Array<any>}
-   */
-  const result = []
-  typeListForEach(type, (c, i) => {
-    result.push(f(c, i, type))
-  })
-  return result
-}
-
-/**
- *
- * @todo remove!
- *
- * @param {AbstractType<any>} type
- * @return {IterableIterator<any>}
- *
- * @private
- * @function
- */
-export const typeListCreateIterator = type => {
-  let n = type._start
-  /**
-   * @type {Array<any>|null}
-   */
-  let currentContent = null
-  let currentContentIndex = 0
-  return {
-    [Symbol.iterator] () {
-      return this
-    },
-    next: () => {
-      // find some content
-      if (currentContent === null) {
-        while (n !== null && n.deleted) {
-          n = n.right
-        }
-        // check if we reached the end, no need to check currentContent, because it does not exist
-        if (n === null) {
-          return {
-            done: true,
-            value: undefined
-          }
-        }
-        // we found n, so we can set currentContent
-        currentContent = n.content.getContent()
-        currentContentIndex = 0
-        n = n.right // we used the content of n, now iterate to next
-      }
-      const value = currentContent[currentContentIndex++]
-      // check if we need to empty currentContent
-      if (currentContent.length <= currentContentIndex) {
-        currentContent = null
-      }
-      return {
-        done: false,
-        value
-      }
-    }
-  }
-}
-
-/**
- *
- * @todo remove!
- *
- * Executes a provided function on once on overy element of this YArray.
- * Operates on a snapshotted state of the document.
- *
- * @param {AbstractType<any>} type
- * @param {function(any,number,AbstractType<any>):void} f A function to execute on every element of this YArray.
- * @param {Snapshot} snapshot
- *
- * @private
- * @function
- */
-export const typeListForEachSnapshot = (type, f, snapshot) => {
-  let index = 0
-  let n = type._start
-  while (n !== null) {
-    if (n.countable && isVisible(n, snapshot)) {
-      const c = n.content.getContent()
-      for (let i = 0; i < c.length; i++) {
-        f(c[i], index++, type)
-      }
-    }
-    n = n.right
-  }
-}
-
 /**
  * @param {Transaction} transaction
  * @param {AbstractType<any>} parent

src/types/YArray.js

@@ -8,7 +8,7 @@ import {
   YArrayRefID,
   callTypeObservers,
   transact,
-  ListIterator,
+  ListWalker,
   useSearchMarker,
   createRelativePositionFromTypeIndex,
   UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Transaction, Item, // eslint-disable-line
@@ -45,7 +45,7 @@ export class YArray extends AbstractType {
      */
     this._prelimContent = []
     /**
-     * @type {Array<ListIterator>}
+     * @type {Array<ListWalker>}
      */
     this._searchMarker = []
   }
@@ -141,7 +141,15 @@ export class YArray extends AbstractType {
   /**
    * Move a single item from $index to $target.
    *
-   * @todo make sure that collapsed moves are removed (i.e. when moving the same item twice)
+   * If the original item is to the left of $target, then the index of the item will decrement.
+   *
+   * ```js
+   * yarray.insert(0, [1, 2, 3])
+   * yarray.move(0, 3) // move "1" to index 3
+   * yarray.toArray() // => [2, 3, 1]
+   * yarray.move(2, 0) // move "1" to index 0
+   * yarray.toArray() // => [1, 2, 3]
+   * ```
    *
    * @param {number} index
    * @param {number} target
@@ -254,7 +262,7 @@ export class YArray extends AbstractType {
    */
   toArray () {
     return transact(/** @type {Doc} */ (this.doc), tr =>
-      new ListIterator(this).slice(tr, this.length)
+      new ListWalker(this).slice(tr, this.length)
     )
   }
 
@@ -293,7 +301,7 @@ export class YArray extends AbstractType {
    */
   map (f) {
     return transact(/** @type {Doc} */ (this.doc), tr =>
-      new ListIterator(this).map(tr, f)
+      new ListWalker(this).map(tr, f)
     )
   }
 
@@ -304,7 +312,7 @@ export class YArray extends AbstractType {
    */
   forEach (f) {
     return transact(/** @type {Doc} */ (this.doc), tr =>
-      new ListIterator(this).forEach(tr, f)
+      new ListWalker(this).forEach(tr, f)
     )
   }
 
@@ -312,6 +320,7 @@ export class YArray extends AbstractType {
    * @return {IterableIterator<T>}
    */
   [Symbol.iterator] () {
+    // @todo, this could be optimized using a real iterator
     return this.toArray().values()
   }
 

src/types/YText.js

@@ -28,7 +28,7 @@ import {
   ContentType,
   useSearchMarker,
   findIndexCleanStart,
-  ListIterator, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, ID, Doc, Item, Snapshot, Transaction // eslint-disable-line
+  ListWalker, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, ID, Doc, Item, Snapshot, Transaction // eslint-disable-line
 } from '../internals.js'
 
 import * as object from 'lib0/object'
@@ -785,7 +785,7 @@ export class YText extends AbstractType {
      */
     this._pending = string !== undefined ? [() => this.insert(0, string)] : []
     /**
-     * @type {Array<ListIterator>}
+     * @type {Array<ListWalker>}
      */
     this._searchMarker = []
   }

src/types/YXmlElement.js

@@ -7,7 +7,6 @@ import {
   typeMapSet,
   typeMapGet,
   typeMapGetAll,
-  typeListForEach,
   YXmlElementRefID,
   YXmlText, ContentType, AbstractType, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Snapshot, Doc, Item // eslint-disable-line
 } from '../internals.js'
@@ -185,36 +184,6 @@ export class YXmlElement extends YXmlFragment {
     return typeMapGetAll(this)
   }
 
-  /**
-   * Creates a Dom Element that mirrors this YXmlElement.
-   *
-   * @param {Document} [_document=document] The document object (you must define
-   *                                        this when calling this method in
-   *                                        nodejs)
-   * @param {Object<string, any>} [hooks={}] Optional property to customize how hooks
-   *                                             are presented in the DOM
-   * @param {any} [binding] You should not set this property. This is
-   *                               used if DomBinding wants to create a
-   *                               association to the created DOM type.
-   * @return {Node} The {@link https://developer.mozilla.org/en-US/docs/Web/API/Element|Dom Element}
-   *
-   * @public
-   */
-  toDOM (_document = document, hooks = {}, binding) {
-    const dom = _document.createElement(this.nodeName)
-    const attrs = this.getAttributes()
-    for (const key in attrs) {
-      dom.setAttribute(key, attrs[key])
-    }
-    typeListForEach(this, yxml => {
-      dom.appendChild(yxml.toDOM(_document, hooks, binding))
-    })
-    if (binding !== undefined) {
-      binding._createAssociation(dom, this)
-    }
-    return dom
-  }
-
   /**
    * Transform the properties of this type to binary and write it to an
    * BinaryEncoder.

src/types/YXmlFragment.js

@@ -6,8 +6,6 @@ import {
   YXmlEvent,
   YXmlElement,
   AbstractType,
-  typeListMap,
-  typeListForEach,
   typeListInsertGenericsAfter,
   typeListToArray,
   YXmlFragmentRefID,
@@ -15,7 +13,8 @@ import {
   transact,
   typeListSlice,
   useSearchMarker,
-  UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, ContentType, Transaction, Item, YXmlText, YXmlHook, Snapshot // eslint-disable-line
+  UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, ContentType, Transaction, Item, YXmlText, YXmlHook, Snapshot, // eslint-disable-line
+  ListWalker
 } from '../internals.js'
 
 import * as error from 'lib0/error'
@@ -254,7 +253,10 @@ export class YXmlFragment extends AbstractType {
    * @return {string} The string representation of all children.
    */
   toString () {
-    return typeListMap(this, xml => xml.toString()).join('')
+    if (this.doc != null) {
+      return transact(this.doc, tr => new ListWalker(this).map(tr, xml => xml.toString()).join(''))
+    }
+    return ''
   }
 
   /**
@@ -264,32 +266,6 @@ export class YXmlFragment extends AbstractType {
     return this.toString()
   }
 
-  /**
-   * Creates a Dom Element that mirrors this YXmlElement.
-   *
-   * @param {Document} [_document=document] The document object (you must define
-   *                                        this when calling this method in
-   *                                        nodejs)
-   * @param {Object<string, any>} [hooks={}] Optional property to customize how hooks
-   *                                             are presented in the DOM
-   * @param {any} [binding] You should not set this property. This is
-   *                               used if DomBinding wants to create a
-   *                               association to the created DOM type.
-   * @return {Node} The {@link https://developer.mozilla.org/en-US/docs/Web/API/Element|Dom Element}
-   *
-   * @public
-   */
-  toDOM (_document = document, hooks = {}, binding) {
-    const fragment = _document.createDocumentFragment()
-    if (binding !== undefined) {
-      binding._createAssociation(fragment, this)
-    }
-    typeListForEach(this, xmlType => {
-      fragment.insertBefore(xmlType.toDOM(_document, hooks, binding), null)
-    })
-    return fragment
-  }
-
   /**
    * Inserts new content at an index.
    *
@@ -302,7 +278,7 @@ export class YXmlFragment extends AbstractType {
    */
   insert (index, content) {
     if (this.doc !== null) {
-      return transact(/** @type {Doc} */ (this.doc), transaction =>
+      return transact(this.doc, transaction =>
         useSearchMarker(transaction, this, index, walker =>
           walker.insertArrayValue(transaction, content)
         )

src/utils/ListWalker.js

@@ -30,7 +30,7 @@ const lengthExceeded = error.create('Length exceeded!')
  * computed item.
  *
  * @param {Transaction} tr
- * @param {ListIterator} li
+ * @param {ListWalker} li
  */
 const popMovedStack = (tr, li) => {
   let { start, end, move } = li.movedStack.pop() || { start: null, end: null, move: null }
@@ -49,7 +49,7 @@ const popMovedStack = (tr, li) => {
         )
       )
     ) {
-      const coords = getMovedCoords(moveContent, tr)
+      const coords = getMovedCoords(moveContent, tr, false)
       start = coords.start
       end = coords.end
     }
@@ -61,10 +61,9 @@ const popMovedStack = (tr, li) => {
 }
 
 /**
- * @todo rename to walker?
+ * Structure that helps to iterate through list-like structures. This is a useful abstraction that keeps track of move operations.
- * @todo check that inserting character one after another always reuses ListIterators
  */
-export class ListIterator {
+export class ListWalker {
   /**
    * @param {AbstractType<any>} type
    */
@@ -105,7 +104,7 @@ export class ListIterator {
   }
 
   clone () {
-    const iter = new ListIterator(this.type)
+    const iter = new ListWalker(this.type)
     iter.index = this.index
     iter.rel = this.rel
     iter.nextItem = this.nextItem
@@ -169,11 +168,6 @@ export class ListIterator {
     }
     let item = /** @type {Item} */ (this.nextItem)
     this.index += len
-    // @todo this condition is not needed, better to remove it (can always be applied)
-    if (this.rel) {
-      len += this.rel
-      this.rel = 0
-    }
     // eslint-disable-next-line no-unmodified-loop-condition
     while ((!this.reachedEnd || this.currMove !== null) && (len > 0 || (skipUncountables && len === 0 && item && (!item.countable || item.deleted || item === this.currMoveEnd || (this.reachedEnd && this.currMoveEnd === null) || item.moved !== this.currMove)))) {
       if (item === this.currMoveEnd || (this.currMoveEnd === null && this.reachedEnd && this.currMove)) {
@@ -192,7 +186,7 @@ export class ListIterator {
         if (this.currMove) {
           this.movedStack.push({ start: this.currMoveStart, end: this.currMoveEnd, move: this.currMove })
         }
-        const { start, end } = getMovedCoords(item.content, tr)
+        const { start, end } = getMovedCoords(item.content, tr, false)
         this.currMove = item
         this.currMoveStart = start
         this.currMoveEnd = end
@@ -205,7 +199,7 @@ export class ListIterator {
       if (item.right) {
         item = item.right
       } else {
-        this.reachedEnd = true // @todo we need to ensure to iterate further if this.currMoveEnd === null
+        this.reachedEnd = true
       }
     }
     this.index -= len
@@ -250,7 +244,7 @@ export class ListIterator {
   /**
    * @param {Transaction} tr
    * @param {number} len
-   * @return {ListIterator}
+   * @return {ListWalker}
    */
   backward (tr, len) {
     if (this.index - len < 0) {
@@ -287,7 +281,7 @@ export class ListIterator {
         if (this.currMove) {
           this.movedStack.push({ start: this.currMoveStart, end: this.currMoveEnd, move: this.currMove })
         }
-        const { start, end } = getMovedCoords(item.content, tr)
+        const { start, end } = getMovedCoords(item.content, tr, false)
         this.currMove = item
         this.currMoveStart = start
         this.currMoveEnd = end
@@ -336,7 +330,6 @@ export class ListIterator {
         }
         if (nextItem.right) {
           nextItem = nextItem.right
-          this.nextItem = nextItem // @todo move this after the while loop
         } else {
           this.reachedEnd = true
         }
@@ -345,9 +338,6 @@ export class ListIterator {
         // always set nextItem before any method call
         this.nextItem = nextItem
         this.forward(tr, 0, true)
-        if (this.nextItem == null) {
-          throw new Error('debug me') // @todo remove
-        }
         nextItem = this.nextItem
       }
     }
@@ -604,7 +594,7 @@ const concatArrayContent = (content, added) => {
  * * Delete the stack-items that both of them have in common
  *
  * @param {Transaction} tr
- * @param {ListIterator} walker
+ * @param {ListWalker} walker
  * @param {number} len
  * @return {Array<{ start: RelativePosition, end: RelativePosition }>}
  */
@@ -713,7 +703,7 @@ export const getMinimalListViewRanges = (tr, walker, len) => {
   const normalizedRanges = array.flatten(ranges.map(range => {
     // A subset of a range could be moved by another move with a higher priority.
     // If that is the case, we need to ignore those moved items.
-    const { start, end } = getMovedCoords(range, tr)
+    const { start, end } = getMovedCoords(range, tr, false)
     const move = range.move
     const ranges = []
     /**

src/utils/YEvent.js

@@ -226,9 +226,9 @@ export class YEvent {
             } else if (item === null) {
               break
             } else if (item.content.constructor === ContentMove) {
-              if (item.moved === currMove && (!item.deleted || (this.deletes(item) && !this.adds(item)))) { // @todo !item.deleted || this.deletes(item)
+              if (item.moved === currMove && (!item.deleted || (this.deletes(item) && !this.adds(item)))) {
                 movedStack.push({ end: currMoveEnd, move: currMove, isNew: currMoveIsNew, isDeleted: currMoveIsDeleted })
-                const { start, end } = getMovedCoords(item.content, tr)
+                const { start, end } = getMovedCoords(item.content, tr, true) // We must split items for move-ranges, for single moves no splitting suffices
                 currMove = item
                 currMoveEnd = end
                 currMoveIsNew = this.adds(item) || currMoveIsNew

src/utils/encoding.js

@@ -193,7 +193,6 @@ export const readClientsStructRefs = (decoder, doc) => {
         }
       }
     }
-    // console.log('time to read: ', performance.now() - start) // @todo remove
   }
   return clientRefs
 }
@@ -389,10 +388,6 @@ export const readUpdateV2 = (decoder, ydoc, transactionOrigin, structDecoder = n
     const store = doc.store
     // let start = performance.now()
     const ss = readClientsStructRefs(structDecoder, doc)
-    // console.log('time to read structs: ', performance.now() - start) // @todo remove
-    // start = performance.now()
-    // console.log('time to merge: ', performance.now() - start) // @todo remove
-    // start = performance.now()
     const restStructs = integrateStructs(transaction, store, ss)
     const pending = store.pendingStructs
     if (pending) {
@@ -416,8 +411,6 @@ export const readUpdateV2 = (decoder, ydoc, transactionOrigin, structDecoder = n
     } else {
       store.pendingStructs = restStructs
     }
-    // console.log('time to integrate: ', performance.now() - start) // @todo remove
-    // start = performance.now()
     const dsRest = readAndApplyDeleteSet(structDecoder, transaction, store)
     if (store.pendingDs) {
       // @todo we could make a lower-bound state-vector check as we do above
@@ -437,11 +430,6 @@ export const readUpdateV2 = (decoder, ydoc, transactionOrigin, structDecoder = n
       // Either dsRest == null && pendingDs == null OR dsRest != null
       store.pendingDs = dsRest
     }
-    // console.log('time to cleanup: ', performance.now() - start) // @todo remove
-    // start = performance.now()
-
-    // console.log('time to resume delete readers: ', performance.now() - start) // @todo remove
-    // start = performance.now()
     if (retry) {
       const update = /** @type {{update: Uint8Array}} */ (store.pendingStructs).update
       store.pendingStructs = null

tests/index.js

@@ -10,6 +10,7 @@ import * as doc from './doc.tests.js'
 import * as snapshot from './snapshot.tests.js'
 import * as updates from './updates.tests.js'
 import * as relativePositions from './relativePositions.tests.js'
+import * as Y from './testHelper.js'
 
 import { runTests } from 'lib0/testing'
 import { isBrowser, isNode } from 'lib0/environment'
@@ -17,6 +18,8 @@ import * as log from 'lib0/logging'
 
 if (isBrowser) {
   log.createVConsole(document.body)
+  // @ts-ignore
+  window.Y = Y
 }
 runTests({
   doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions

tests/testHelper.js

@@ -373,33 +373,6 @@ export const compare = users => {
     t.compare(Y.encodeStateVector(users[i]), Y.encodeStateVector(users[i + 1]))
     compareDS(Y.createDeleteSetFromStructStore(users[i].store), Y.createDeleteSetFromStructStore(users[i + 1].store))
     compareStructStores(users[i].store, users[i + 1].store)
-    // @todo
-    // test list-iterator
-    // console.log('dutiraneduiaentdr', users[0].getArray('array')._searchMarker)
-    /*
-    {
-      const user = users[0]
-      user.transact(tr => {
-        const type = user.getArray('array')
-        Y.useSearchMarker(tr, type, type.length, walker => {
-          for (let i = type.length; i >= 0; i--) {
-            const otherWalker = new Y.ListIterator(type)
-            otherWalker.forward(tr, walker.index)
-            otherWalker.forward(tr, 0)
-            walker.forward(tr, 0)
-            t.assert(walker.index === i)
-            t.assert(walker.left === otherWalker.left)
-            t.assert(walker.right === otherWalker.right)
-            t.assert(walker.nextItem === otherWalker.nextItem)
-            t.assert(walker.reachedEnd === otherWalker.reachedEnd)
-            if (i > 0) {
-              walker.backward(tr, 1)
-            }
-          }
-        })
-      })
-    }
-    */
   }
   users.map(u => u.destroy())
 }

tests/y-array.tests.js

@@ -534,6 +534,32 @@ export const testMoveSingleItemRemovesPrev = tc => {
   t.assert(items.filter(item => !item.deleted).length === 3)
 }
 
+/**
+ * Check that the searchMarker is reused correctly.
+ *
+ * @param {t.TestCase} tc
+ */
+export const testListWalkerReusesSearchMarker = tc => {
+  const ydoc = new Y.Doc()
+  const yarray = ydoc.getArray()
+  const iterations = 100
+  for (let i = 0; i < iterations; i++) {
+    yarray.insert(0, [i])
+  }
+  /**
+   * @type {any}
+   */
+  let prevSm = null
+  for (let i = 0; i < iterations; i++) {
+    const v = yarray.get(i)
+    t.assert(v === iterations - i - 1)
+    t.assert(yarray._searchMarker.length <= 1)
+    const sm = yarray._searchMarker[0]
+    t.assert(prevSm == null || sm === prevSm)
+    prevSm = sm
+  }
+}
+
 /**
  * @param {t.TestCase} tc
  */
@@ -617,8 +643,6 @@ const getUniqueNumber = () => _uniqueNumber++
 
 /**
  * @type {Array<function(Doc,prng.PRNG,any):void>}
- *
- * @todo to replace content to a separate data structure so we know that insert & returns work as expected!!!
  */
 const arrayTransactions = [
   function move (user, gen) {
@@ -732,6 +756,7 @@ const compareTestobjects = cmp => {
   for (let i = 0; i < arrs.length; i++) {
     const type = cmp.users[i].getArray('array')
     t.compareArrays(arrs[i], type.toArray())
+    t.compareArrays(arrs[i], Array.from(type))
   }
 }