diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 7f9c27f8..8cfd6b99 100644 --- a/src/structs/ContentMove.js +++ b/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 nextPrio = currMoved ? /** @type {ContentMove} */ (currMoved.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 (currMoved !== null) { - this.overrides.add(currMoved) - transaction._mergeStructs.push(start) + const prevMove = start.moved // this is the same as prevMove + const nextPrio = prevMove ? /** @type {ContentMove} */ (prevMove.content).priority : -1 + 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 (prevMove !== null) { + if (/** @type {ContentMove} */ (prevMove.content).isCollapsed()) { + 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) && prevMove.id.clock < (transaction.beforeState.get(prevMove.id.client) || 0)) { + if (prevMove && !transaction.prevMoved.has(start) && !addsStruct(transaction, prevMove)) { // 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) { - /** @type {ContentMove} */ (currMoved.content).overrides.add(item) + } else if (prevMove != null) { + /** @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) { - // potentially we can integrate the items that reIntegrateItem overrides - content.overrides.forEach(reIntegrate) - } else { - content.integrate(transaction, reIntegrateItem) + // content is not yet transformed to a ContentDeleted + if (content.getRef() === 11) { + if (reIntegrateItem.deleted) { + // potentially we can integrate the items that reIntegrateItem overrides + content.overrides.forEach(reIntegrate) + } else { + content.integrate(transaction, reIntegrateItem) + } } } this.overrides.forEach(reIntegrate) diff --git a/src/structs/Item.js b/src/structs/Item.js index b23ed6d3..d200bda4 100644 --- a/src/structs/Item.js +++ b/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) + } } /** diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index e0c9a854..7cc7685a 100644 --- a/src/types/AbstractType.js +++ b/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} 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} type * @param {function(C,number,AbstractType):R} f @@ -443,6 +448,9 @@ export const typeListMap = (type, f) => { } /** + * + * @todo remove! + * * @param {AbstractType} type * @return {IterableIterator} * @@ -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. * diff --git a/src/utils/Transaction.js b/src/utils/Transaction.js index 95749017..78e855fd 100644 --- a/src/utils/Transaction.js +++ b/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) diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index 4c6bbd81..9ff62ebe 100644 --- a/src/utils/YEvent.js +++ b/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: [] } diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index 49c7684d..ff1ba336 100644 --- a/tests/y-array.tests.js +++ b/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