remove old move-ranges if collapsed

src/structs/ContentMove.js

@@ -6,7 +6,8 @@ import * as math from 'lib0/math'
 import {
   writeID,
   readID,
-  ID, AbstractType, ContentType, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line
+  ID, AbstractType, ContentType, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd, // eslint-disable-line
+  addsStruct
 } from '../internals.js'
 
 /**
@@ -160,28 +161,30 @@ export class ContentMove {
     // that we want to set prio to the current prio-maximum of the moved range.
     const adaptPriority = this.priority < 0
     while (start !== end && start != null) {
-      const currMoved = start.moved
+      const prevMove = start.moved // this is the same as prevMove
-      const nextPrio = currMoved ? /** @type {ContentMove} */ (currMoved.content).priority : -1
+      const nextPrio = prevMove ? /** @type {ContentMove} */ (prevMove.content).priority : -1
-      if (adaptPriority || nextPrio < this.priority || (currMoved != null && nextPrio === this.priority && (currMoved.id.client < item.id.client || (currMoved.id.client === item.id.client && currMoved.id.clock < item.id.clock)))) {
+      if (adaptPriority || nextPrio < this.priority || (prevMove != null && nextPrio === this.priority && (prevMove.id.client < item.id.client || (prevMove.id.client === item.id.client && prevMove.id.clock < item.id.clock)))) {
-        if (currMoved !== null) {
+        if (prevMove !== null) {
-          this.overrides.add(currMoved)
+          if (/** @type {ContentMove} */ (prevMove.content).isCollapsed()) {
-          transaction._mergeStructs.push(start)
+            prevMove.deleteAsCleanup(transaction, adaptPriority)
+          }
+          this.overrides.add(prevMove)
+          transaction._mergeStructs.push(start) // @todo is this needed?
         }
         maxPriority = math.max(maxPriority, nextPrio)
         // was already moved
-        const prevMove = start.moved
+        if (prevMove && !transaction.prevMoved.has(start) && !addsStruct(transaction, prevMove)) {
-        if (prevMove && !transaction.prevMoved.has(start) && prevMove.id.clock < (transaction.beforeState.get(prevMove.id.client) || 0)) {
           // only override prevMoved if the prevMoved item is not new
           // we need to know which item previously moved an item
           transaction.prevMoved.set(start, prevMove)
         }
         start.moved = item
         if (!start.deleted && start.content.constructor === ContentMove && findMoveLoop(transaction, start.content, start, new Set([item]))) {
-          item.deleteAsCleanup(transaction)
+          item.deleteAsCleanup(transaction, adaptPriority)
           return
         }
-      } else if (currMoved != null) {
+      } else if (prevMove != null) {
-        /** @type {ContentMove} */ (currMoved.content).overrides.add(item)
+        /** @type {ContentMove} */ (prevMove.content).overrides.add(item)
       }
       start = start.right
     }
@@ -201,6 +204,9 @@ export class ContentMove {
     let { start, end } = getMovedCoords(this, transaction)
     while (start !== end && start != null) {
       if (start.moved === item) {
+        if (!transaction.prevMoved.has(start)) {
+          transaction.prevMoved.set(start, item)
+        }
         start.moved = null
       }
       start = start.right
@@ -210,11 +216,14 @@ export class ContentMove {
      */
     const reIntegrate = reIntegrateItem => {
       const content = /** @type {ContentMove} */ (reIntegrateItem.content)
-      if (reIntegrateItem.deleted) {
+      // content is not yet transformed to a ContentDeleted
-        // potentially we can integrate the items that reIntegrateItem overrides
+      if (content.getRef() === 11) {
-        content.overrides.forEach(reIntegrate)
+        if (reIntegrateItem.deleted) {
-      } else {
+          // potentially we can integrate the items that reIntegrateItem overrides
-        content.integrate(transaction, reIntegrateItem)
+          content.overrides.forEach(reIntegrate)
+        } else {
+          content.integrate(transaction, reIntegrateItem)
+        }
       }
     }
     this.overrides.forEach(reIntegrate)

src/structs/Item.js

@@ -670,10 +670,13 @@ export class Item extends AbstractStruct {
    * the providers don't filter this message.
    *
    * @param {Transaction} transaction
+   * @param {boolean} isLocal
    */
-  deleteAsCleanup (transaction) {
+  deleteAsCleanup (transaction, isLocal) {
     this.delete(transaction)
-    addToDeleteSet(transaction.cleanupDeletions, this.id.client, this.id.clock, this.length)
+    if (!isLocal) {
+      addToDeleteSet(transaction.cleanupDeletions, this.id.client, this.id.clock, this.length)
+    }
   }
 
   /**

src/types/AbstractType.js

@@ -402,6 +402,8 @@ export const typeListToArraySnapshot = (type, snapshot) => {
 /**
  * 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.
  *
@@ -423,6 +425,9 @@ export const typeListForEach = (type, f) => {
 }
 
 /**
+ *
+ * @todo remove!
+ *
  * @template C,R
  * @param {AbstractType<any>} type
  * @param {function(C,number,AbstractType<any>):R} f
@@ -443,6 +448,9 @@ export const typeListMap = (type, f) => {
 }
 
 /**
+ *
+ * @todo remove!
+ *
  * @param {AbstractType<any>} type
  * @return {IterableIterator<any>}
  *
@@ -492,6 +500,9 @@ export const typeListCreateIterator = type => {
 }
 
 /**
+ *
+ * @todo remove!
+ *
  * Executes a provided function on once on overy element of this YArray.
  * Operates on a snapshotted state of the document.
  *

src/utils/Transaction.js

@@ -455,3 +455,10 @@ export const transact = (doc, f, origin = null, local = true) => {
   }
   return res
 }
+
+/**
+ * @param {Transaction} tr
+ * @param {AbstractStruct} struct
+ */
+export const addsStruct = (tr, struct) =>
+  struct.id.clock >= (tr.beforeState.get(struct.id.client) || 0)

src/utils/YEvent.js

@@ -7,6 +7,7 @@ import {
 
 import * as set from 'lib0/set'
 import * as array from 'lib0/array'
+import { addsStruct } from './Transaction.js'
 
 /**
  * YEvent describes the changes on a YType.
@@ -145,7 +146,7 @@ export class YEvent {
    * @return {boolean}
    */
   adds (struct) {
-    return struct.id.clock >= (this.transaction.beforeState.get(struct.id.client) || 0)
+    return addsStruct(this.transaction, struct)
   }
 
   /**
@@ -218,7 +219,7 @@ export class YEvent {
             } else if (item === null) {
               break
             } else if (item.content.constructor === ContentMove) {
-              if (item.moved === currMove) {
+              if (item.moved === currMove) { // @todo !item.deleted || this.deletes(item)
                 movedStack.push({ end: currMoveEnd, move: currMove, isNew: currMoveIsNew })
                 const { start, end } = getMovedCoords(item.content, tr)
                 currMove = item
@@ -228,7 +229,7 @@ export class YEvent {
                 continue // do not move to item.right
               }
             } else if (item.moved !== currMove) {
-              if (!currMoveIsNew && item.countable && (!item.deleted || this.deletes(item)) && !this.adds(item) && isMovedByNew(item) && (this.transaction.prevMoved.get(item) || null) === currMove) {
+              if (!currMoveIsNew && item.countable && (!item.deleted || this.deletes(item)) && !this.adds(item) && (item.moved === null || isMovedByNew(item)) && (this.transaction.prevMoved.get(item) || null) === currMove) {
                 if (lastOp === null || lastOp.delete === undefined) {
                   packOp()
                   lastOp = { delete: 0 }
@@ -245,7 +246,7 @@ export class YEvent {
                 deleted.add(item)
               }
             } else {
-              if (currMoveIsNew || this.adds(item)) {
+              if (currMoveIsNew || this.adds(item) || this.transaction.prevMoved.has(item)) {
                 if (lastOp === null || lastOp.insert === undefined) {
                   packOp()
                   lastOp = { insert: [] }

tests/y-array.tests.js

@@ -1,4 +1,4 @@
-import { init, compare, applyRandomTests, Doc, AbstractType, TestConnector } from './testHelper.js' // eslint-disable-line
+import { init, compare, applyRandomTests, Doc, AbstractType, TestConnector, Item } from './testHelper.js' // eslint-disable-line
 
 import * as Y from '../src/index.js'
 import * as t from 'lib0/testing'
@@ -512,6 +512,69 @@ export const testMove2 = tc => {
   compare(users)
 }
 
+/**
+ * @param {t.TestCase} tc
+ */
+export const testMoveSingleItemRemovesPrev = tc => {
+  const ydoc = new Y.Doc()
+  const yarray = ydoc.getArray()
+  yarray.insert(0, [1, 2, 3])
+  // @todo should be old-position to new-position. so that below move matches
+  yarray.move(0, 3)
+  t.compareArrays(yarray.toArray(), [2, 3, 1])
+  yarray.move(2, 0)
+  t.compareArrays(yarray.toArray(), [1, 2, 3])
+  let item = yarray._start
+  const items = []
+  while (item) {
+    items.push(item)
+    item = item.right
+  }
+  t.assert(items.length === 4)
+  t.assert(items.filter(item => !item.deleted).length === 3)
+}
+
+/**
+ * @param {t.TestCase} tc
+ */
+export const testMoveDeletions = tc => {
+  const ydoc = new Y.Doc()
+  const yarray = ydoc.getArray()
+  const array = yarray.toArray()
+  /**
+   * @type {any}
+   */
+  let lastDelta = []
+  yarray.observe(event => {
+    lastDelta = event.delta
+    let pos = 0
+    for (let i = 0; i < lastDelta.length; i++) {
+      const d = lastDelta[i]
+      if (d.retain != null) {
+        pos += d.retain
+      } else if (d.insert instanceof Array) {
+        array.splice(pos, 0, ...d.insert)
+        pos += d.insert.length
+      } else if (d.delete != null) {
+        array.splice(pos, d.delete)
+      }
+    }
+  })
+  yarray.insert(0, [1, 2, 3])
+  // @todo should be old-position to new-position. so that below move matches
+  yarray.move(2, 0)
+  t.compare(lastDelta, [{ insert: [3] }, { retain: 2 }, { delete: 1 }])
+  t.compareArrays(yarray.toArray(), [3, 1, 2])
+  t.compareArrays(yarray.toArray(), array)
+  ydoc.transact(tr => {
+    /** @type {Item} */ (yarray._start).delete(tr)
+  })
+  debugger
+  t.compare(lastDelta, [{ delete: 1 }, { retain: 2 }, { insert: [3] }])
+  t.compareArrays(yarray.toArray(), [1, 2, 3])
+  t.compareArrays(yarray.toArray(), array)
+}
+
 /**
  * @todo
  * @param {t.TestCase} tc