diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 1366b2d5..a196ac02 100644 --- a/src/structs/ContentMove.js +++ b/src/structs/ContentMove.js @@ -6,6 +6,7 @@ import * as math from 'lib0/math' import { AbstractType, ContentType, RelativePosition, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Transaction, Item, StructStore, getItem, getItemCleanStart, getItemCleanEnd // eslint-disable-line } from '../internals.js' +import { decodeRelativePosition, encodeRelativePosition } from 'yjs' /** * @param {ContentMove} moved @@ -246,8 +247,12 @@ export class ContentMove { * @param {number} offset */ write (encoder, offset) { - encoder.writeAny(this.start) - encoder.writeAny(this.end) + const isCollapsed = this.isCollapsed() + encoding.writeUint8(encoder.restEncoder, isCollapsed ? 1 : 0) + encoder.writeBuf(encodeRelativePosition(this.start)) + if (!isCollapsed) { + encoder.writeBuf(encodeRelativePosition(this.end)) + } encoding.writeVarUint(encoder.restEncoder, this.priority) } @@ -257,6 +262,10 @@ export class ContentMove { getRef () { return 11 } + + isCollapsed () { + return this.start.item === this.end.item && this.start.item !== null + } } /** @@ -266,4 +275,12 @@ export class ContentMove { * @param {UpdateDecoderV1 | UpdateDecoderV2} decoder * @return {ContentMove} */ -export const readContentMove = decoder => new ContentMove(decoder.readAny(), decoder.readAny(), decoding.readVarUint(decoder.restDecoder)) +export const readContentMove = decoder => { + const isCollapsed = decoding.readUint8(decoder.restDecoder) === 1 + const start = decodeRelativePosition(decoder.readBuf()) + const end = isCollapsed ? start.clone() : decodeRelativePosition(decoder.readBuf()) + if (isCollapsed) { + end.assoc = -1 + } + return new ContentMove(start, end, decoding.readVarUint(decoder.restDecoder)) +} diff --git a/src/types/YArray.js b/src/types/YArray.js index 3a0d806e..e04f5793 100644 --- a/src/types/YArray.js +++ b/src/types/YArray.js @@ -137,6 +137,34 @@ 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) + * + * @param {number} index + * @param {number} target + */ + move (index, target) { + if (index === target || index + 1 === target || index >= this.length) { + // 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 left = createRelativePositionFromTypeIndex(this, index, 1) + const right = left.clone() + right.assoc = -1 + useSearchMarker(transaction, this, target, walker => { + walker.insertMove(transaction, left, right) + }) + }) + } else { + const content = /** @type {Array} */ (this._prelimContent).splice(index, 1) + ;/** @type {Array} */ (this._prelimContent).splice(target, 0, ...content) + } + } + /** * @param {number} start Inclusive move-start * @param {number} end Inclusive move-end @@ -144,7 +172,7 @@ export class YArray extends AbstractType { * @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. */ - move (start, end, target, assocStart = 1, assocEnd = -1) { + moveRange (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 diff --git a/src/utils/ListIterator.js b/src/utils/ListIterator.js index 48c6760c..4fd830c8 100644 --- a/src/utils/ListIterator.js +++ b/src/utils/ListIterator.js @@ -122,8 +122,17 @@ export class ListIterator { len += this.rel this.rel = 0 } - while (item && !this.reachedEnd && (len > 0 || (len === 0 && (!item.countable || item.deleted || item === this.currMoveEnd || item.moved !== this.currMove)))) { - if (item.countable && !item.deleted && item.moved === this.currMove && len > 0) { + while ((!this.reachedEnd || this.currMove !== null) && (len > 0 || (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)) { + item = /** @type {Item} */ (this.currMove) // we iterate to the right after the current condition + const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null } + this.currMove = move + this.currMoveStart = start + this.currMoveEnd = end + this.reachedEnd = false + } else if (item === null) { + break + } else if (item.countable && !item.deleted && item.moved === this.currMove && len > 0) { len -= item.length if (len < 0) { this.rel = item.length + len @@ -141,13 +150,6 @@ export class ListIterator { item = start continue } - if (item === this.currMoveEnd) { - item = /** @type {Item} */ (this.currMove) // we iterate to the right after the current condition - const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null } - this.currMove = move - this.currMoveStart = start - this.currMoveEnd = end - } if (item.right) { item = item.right } else { @@ -159,6 +161,23 @@ export class ListIterator { return this } + /** + * @param {Transaction} tr + */ + reduceMoves (tr) { + let item = this.nextItem + if (item !== null) { + while (item === this.currMoveStart) { + item = /** @type {Item} */ (this.currMove) // we iterate to the left after the current condition + const { start, end, move } = this.movedStack.pop() || { start: null, end: null, move: null } + this.currMove = move + this.currMoveStart = start + this.currMoveEnd = end + } + this.nextItem = item + } + } + /** * @param {Transaction} tr * @param {number} len @@ -249,7 +268,7 @@ export class ListIterator { this.reachedEnd = true } } - if (this.nextItem && !this.reachedEnd && len > 0) { + if (this.nextItem && (!this.reachedEnd || this.currMove !== null) && len > 0) { this.forward(tr, 0) } } @@ -317,6 +336,7 @@ export class ListIterator { * @param {Array} content */ insertContents (tr, content) { + this.reduceMoves(tr) this._splitRel(tr) const parent = this.type const store = tr.doc.store diff --git a/src/utils/RelativePosition.js b/src/utils/RelativePosition.js index 719d7ba6..ea3e33e9 100644 --- a/src/utils/RelativePosition.js +++ b/src/utils/RelativePosition.js @@ -75,6 +75,10 @@ export class RelativePosition { */ this.assoc = assoc } + + clone () { + return new RelativePosition(this.type, this.tname, this.item, this.assoc) + } } /** diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index 10c757de..f50b9d70 100644 --- a/src/utils/YEvent.js +++ b/src/utils/YEvent.js @@ -213,7 +213,7 @@ export class YEvent { continue // do not move to item.right } } else if (item.moved !== currMove) { - if (!currMoveIsNew && item.countable && !this.adds(item) && (this.transaction.prevMoved.get(item) || null) === currMove) { + if (!currMoveIsNew && item.countable && item.moved && !this.adds(item) && !this.adds(item.moved) && (this.transaction.prevMoved.get(item) || null) === currMove) { if (lastOp === null || lastOp.delete === undefined) { packOp() lastOp = { delete: 0 } diff --git a/src/utils/updates.js b/src/utils/updates.js index fa6add00..0a9072c5 100644 --- a/src/utils/updates.js +++ b/src/utils/updates.js @@ -308,6 +308,7 @@ export const mergeUpdatesV2 = (updates, YDecoder = UpdateDecoderV2, YEncoder = U // Note: Should handle that some operations cannot be applied yet () while (true) { + // @todo this incurs an exponential overhead. We could instead only sort the item that changed. // Write higher clients first ⇒ sort by clientID & clock and remove decoders without content lazyStructDecoders = lazyStructDecoders.filter(dec => dec.curr !== null) lazyStructDecoders.sort( diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index 6cc65c8e..3c5ff268 100644 --- a/tests/y-array.tests.js +++ b/tests/y-array.tests.js @@ -440,7 +440,7 @@ export const testMove = tc => { // move in uninitialized type const yarr = new Y.Array() yarr.insert(0, [1, 2, 3]) - yarr.move(1, 1, 0) + yarr.move(1, 0) // @ts-ignore t.compare(yarr._prelimContent, [2, 1, 3]) } @@ -460,13 +460,13 @@ export const testMove = tc => { event1 = event }) array0.insert(0, [1, 2, 3]) - array0.move(1, 1, 0) + array0.move(1, 0) t.compare(array0.toArray(), [2, 1, 3]) t.compare(event0.delta, [{ insert: [2] }, { retain: 1 }, { delete: 1 }]) Y.applyUpdate(users[1], Y.encodeStateAsUpdate(users[0])) t.compare(array1.toArray(), [2, 1, 3]) t.compare(event1.delta, [{ insert: [2, 1, 3] }]) - array0.move(0, 0, 2) + array0.move(0, 2) t.compare(array0.toArray(), [1, 2, 3]) t.compare(event0.delta, [{ delete: 1 }, { retain: 1 }, { insert: [2] }]) compare(users) @@ -500,6 +500,19 @@ const getUniqueNumber = () => _uniqueNumber++ * @todo to replace content to a separate data structure so we know that insert & returns work as expected!!! */ const arrayTransactions = [ + function move (user, gen) { + const yarray = user.getArray('array') + if (yarray.length === 0) { + return + } + const pos = prng.int32(gen, 0, yarray.length - 1) + const newPos = prng.int32(gen, 0, yarray.length) + const oldContent = yarray.toArray() + yarray.move(pos, newPos) + const [x] = oldContent.splice(pos, 1) + oldContent.splice(pos < newPos ? newPos - 1 : newPos, 0, x) + t.compareArrays(yarray.toArray(), oldContent) // we want to make sure that fastSearch markers insert at the correct position + }, function insert (user, gen) { const yarray = user.getArray('array') const uniqueNumber = getUniqueNumber() @@ -558,11 +571,49 @@ const arrayTransactions = [ } ] +/** + * @param {Y.Doc} user + */ +const monitorArrayTestObject = user => { + /** + * @type {Array} + */ + const arr = [] + const yarr = user.getArray('array') + yarr.observe(event => { + let currpos = 0 + const delta = event.delta + for (let i = 0; i < delta.length; i++) { + const d = delta[i] + if (d.insert != null) { + arr.splice(currpos, 0, ...(/** @type {Array} */ (d.insert))) + currpos += /** @type {Array} */ (d.insert).length + } else if (d.retain != null) { + currpos += d.retain + } else { + arr.splice(currpos, d.delete) + } + } + }) + return arr +} + +/** + * @param {{ testObjects: Array>, users: Array }} cmp + */ +const compareTestobjects = cmp => { + const arrs = cmp.testObjects + for (let i = 0; i < arrs.length; i++) { + const type = cmp.users[i].getArray('array') + t.compareArrays(arrs[i], type.toArray()) + } +} + /** * @param {t.TestCase} tc */ export const testRepeatGeneratingYarrayTests6 = tc => { - applyRandomTests(tc, arrayTransactions, 10) + compareTestobjects(applyRandomTests(tc, arrayTransactions, 3, monitorArrayTestObject)) } /**