From 100e436e2cf6893b62d18c2346ed047e07598545 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Mon, 11 Jul 2022 18:35:16 +0200 Subject: [PATCH] cleanup --- src/index.js | 1 - src/internals.js | 2 +- src/structs/ContentMove.js | 36 ++-- src/structs/Item.js | 4 +- src/types/AbstractType.js | 186 ++----------------- src/types/YArray.js | 21 ++- src/types/YText.js | 4 +- src/types/YXmlElement.js | 31 ---- src/types/YXmlFragment.js | 38 +--- src/utils/{ListIterator.js => ListWalker.js} | 32 ++-- src/utils/YEvent.js | 4 +- src/utils/encoding.js | 12 -- tests/index.js | 3 + tests/testHelper.js | 27 --- tests/y-array.tests.js | 29 ++- 15 files changed, 106 insertions(+), 324 deletions(-) rename src/utils/{ListIterator.js => ListWalker.js} (95%) diff --git a/src/index.js b/src/index.js index fe74a1d2..ced9005a 100644 --- a/src/index.js +++ b/src/index.js @@ -48,7 +48,6 @@ export { findRootTypeKey, findIndexSS, getItem, - typeListToArraySnapshot, typeMapGetSnapshot, createDocFromSnapshot, iterateDeletedStructs, diff --git a/src/internals.js b/src/internals.js index e84e7ad2..3b93bbbe 100644 --- a/src/internals.js +++ b/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' diff --git a/src/structs/ContentMove.js b/src/structs/ContentMove.js index 985e7fa8..ab7aef6c 100644 --- a/src/structs/ContentMove.js +++ b/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} */ (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} diff --git a/src/structs/Item.js b/src/structs/Item.js index d200bda4..d7b7e5bc 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -762,7 +762,7 @@ export const readItemContent = (decoder, info) => contentRefs[info & binary.BITS * @type {Array} */ 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 ] diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index 7cc7685a..1730919c 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -10,8 +10,8 @@ import { createID, ContentAny, ContentBinary, - ListIterator, - ContentDoc, YText, YArray, UpdateEncoderV1, UpdateEncoderV2, Doc, Snapshot, Transaction, EventHandler, YEvent, Item, // eslint-disable-line + ListWalker, + 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} 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` - return f(new ListIterator(yarray).forward(tr, index, true)) + if (searchMarker === null || yarray._start === null || index < freshSearchMarkerDistance) { + 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 createFreshMarker = searchMarker.length < maxSearchMarker && (math.abs(sm.index - index) > 5 || newIsCheaper) - const fsm = createFreshMarker ? (newIsCheaper ? new ListIterator(yarray) : sm.clone()) : sm + const newIsCheaper = math.abs(sm.index - index) >= index + const createFreshMarker = searchMarker.length < maxSearchMarker && (math.abs(sm.index - index) > freshSearchMarkerDistance || newIsCheaper) + 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} searchMarker + * @param {Array} 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} + * @type {null | Array} */ this._searchMarker = null /** @@ -376,157 +369,6 @@ export const typeListToArray = type => { return cs } -/** - * @param {AbstractType} type - * @param {Snapshot} snapshot - * @return {Array} - * - * @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} 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} type - * @param {function(C,number,AbstractType):R} f - * @return {Array} - * - * @private - * @function - */ -export const typeListMap = (type, f) => { - /** - * @type {Array} - */ - const result = [] - typeListForEach(type, (c, i) => { - result.push(f(c, i, type)) - }) - return result -} - -/** - * - * @todo remove! - * - * @param {AbstractType} type - * @return {IterableIterator} - * - * @private - * @function - */ -export const typeListCreateIterator = type => { - let n = type._start - /** - * @type {Array|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} type - * @param {function(any,number,AbstractType):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} parent diff --git a/src/types/YArray.js b/src/types/YArray.js index 23958889..5b2324a9 100644 --- a/src/types/YArray.js +++ b/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} + * @type {Array} */ 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} */ [Symbol.iterator] () { + // @todo, this could be optimized using a real iterator return this.toArray().values() } diff --git a/src/types/YText.js b/src/types/YText.js index e327a640..a05b1dd4 100644 --- a/src/types/YText.js +++ b/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} + * @type {Array} */ this._searchMarker = [] } diff --git a/src/types/YXmlElement.js b/src/types/YXmlElement.js index 464951b4..41fe9d62 100644 --- a/src/types/YXmlElement.js +++ b/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} [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. diff --git a/src/types/YXmlFragment.js b/src/types/YXmlFragment.js index 9f957475..db019260 100644 --- a/src/types/YXmlFragment.js +++ b/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} [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) ) diff --git a/src/utils/ListIterator.js b/src/utils/ListWalker.js similarity index 95% rename from src/utils/ListIterator.js rename to src/utils/ListWalker.js index aa6c877b..59cfb440 100644 --- a/src/utils/ListIterator.js +++ b/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? - * @todo check that inserting character one after another always reuses ListIterators + * Structure that helps to iterate through list-like structures. This is a useful abstraction that keeps track of move operations. */ -export class ListIterator { +export class ListWalker { /** * @param {AbstractType} 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 = [] /** diff --git a/src/utils/YEvent.js b/src/utils/YEvent.js index a8ea497f..0b615c1d 100644 --- a/src/utils/YEvent.js +++ b/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 diff --git a/src/utils/encoding.js b/src/utils/encoding.js index 29af8bc6..aa2fb1f3 100644 --- a/src/utils/encoding.js +++ b/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 diff --git a/tests/index.js b/tests/index.js index 2f8d8874..b10ac27e 100644 --- a/tests/index.js +++ b/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 diff --git a/tests/testHelper.js b/tests/testHelper.js index 9963e16e..4df1d11e 100644 --- a/tests/testHelper.js +++ b/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()) } diff --git a/tests/y-array.tests.js b/tests/y-array.tests.js index c7306525..f7066505 100644 --- a/tests/y-array.tests.js +++ b/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} - * - * @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)) } }