diff --git a/src/index.js b/src/index.js index 1aebeb81..94a28012 100644 --- a/src/index.js +++ b/src/index.js @@ -115,7 +115,8 @@ export { noAttributionsManager, iterateStructsByIdSet, createAttributionManagerFromDiff, - createIdSet + createIdSet, + cloneDoc } from './internals.js' const glo = /** @type {any} */ (typeof globalThis !== 'undefined' diff --git a/src/types/YText.js b/src/types/YText.js index aa3da2c5..0e01daf3 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -1158,42 +1158,47 @@ export class YText extends AbstractType { if (ir !== rslice.length - 1) { itemContent.splice(idrange.len) } - am.readContent(cs, item.id.client, idrange.clock, item.deleted, content, idrange.exists) + am.readContent(cs, item.id.client, idrange.clock, item.deleted, content, idrange.exists ? 2 : 0) } } } else { for (; item !== null && cs.length < 50; item = item.right) { - am.readContent(cs, item.id.client, item.id.clock, item.deleted, item.content, !item.deleted) + am.readContent(cs, item.id.client, item.id.clock, item.deleted, item.content, 1) } } for (let i = 0; i < cs.length; i++) { const c = cs[i] - const renderDelete = c.deleted && (c.attrs != null || c.render) - const renderInsert = !c.deleted && (c.render || c.attrs != null) - const attribution = (renderDelete || renderInsert) ? createAttributionFromAttributionItems(c.attrs, c.deleted).attribution : null + // render (attributed) content even if it was deleted + const renderContent = c.render && (!c.deleted || c.attrs != null) + // content that was just deleted. It is not rendered as an insertion, because it doesn't + // have any attributes. + const renderDelete = c.render && c.deleted + // existing content that should be retained, only adding changed attributes + const retainContent = !c.render && (!c.deleted || c.attrs != null) + const attribution = renderContent ? createAttributionFromAttributionItems(c.attrs, c.deleted).attribution : null switch (c.content.constructor) { case ContentType: case ContentEmbed: - if (renderInsert) { + if (renderContent) { d.usedAttributes = currentAttributes usingCurrentAttributes = true d.insert(c.content.getContent()[0], null, attribution) } else if (renderDelete) { d.delete(1) - } else if (!c.deleted) { + } else if (retainContent) { d.usedAttributes = changedAttributes usingChangedAttributes = true d.retain(1) } break case ContentString: - if (renderInsert || (renderDelete && attribution?.delete != null)) { + if (renderContent) { d.usedAttributes = currentAttributes usingCurrentAttributes = true d.insert(/** @type {ContentString} */ (c.content).str, null, attribution) } else if (renderDelete) { d.delete(c.content.getLength()) - } else if (!c.deleted) { + } else if (retainContent) { d.usedAttributes = changedAttributes usingChangedAttributes = true d.retain(c.content.getLength()) @@ -1204,7 +1209,7 @@ export class YText extends AbstractType { const currAttrVal = currentAttributes[key] ?? null // @todo write a function "updateCurrentAttributes" and "updateChangedAttributes" // # Update Attributes - if (renderDelete || renderInsert) { + if (renderContent || renderDelete) { // create fresh references if (usingCurrentAttributes) { currentAttributes = object.assign({}, currentAttributes) @@ -1215,30 +1220,34 @@ export class YText extends AbstractType { changedAttributes = object.assign({}, changedAttributes) } } - if (renderInsert) { - if (equalAttrs(value, currAttrVal)) { - // item.delete(transaction) - } else if (equalAttrs(value, previousAttributes[key] ?? null)) { - delete changedAttributes[key] - } else { - changedAttributes[key] = value + if (renderContent || renderDelete) { + if (c.deleted) { + // content was deleted, but is possibly attributed + if (equalAttrs(value, currAttrVal)) { + // nop + } else if (equalAttrs(currAttrVal, previousAttributes[key] ?? null) && changedAttributes[key] !== undefined) { + delete changedAttributes[key] + } else { + changedAttributes[key] = currAttrVal + } + // current attributes doesn't change + previousAttributes[key] = value + } else { // !c.deleted + // content was inserted, and is possibly attributed + if (equalAttrs(value, currAttrVal)) { + // item.delete(transaction) + } else if (equalAttrs(value, previousAttributes[key] ?? null)) { + delete changedAttributes[key] + } else { + changedAttributes[key] = value + } + if (value == null) { + delete currentAttributes[key] + } else { + currentAttributes[key] = value + } } - if (value == null) { - delete currentAttributes[key] - } else { - currentAttributes[key] = value - } - } else if (renderDelete) { - if (equalAttrs(value, currAttrVal)) { - // nop - } else if (equalAttrs(currAttrVal, previousAttributes[key] ?? null) && changedAttributes[key] !== undefined) { - delete changedAttributes[key] - } else { - changedAttributes[key] = currAttrVal - } - // current attributes doesn't change - previousAttributes[key] = value - } else if (!c.deleted) { + } else if (retainContent && !c.deleted) { // fresh reference to currentAttributes only if (usingCurrentAttributes) { currentAttributes = object.assign({}, currentAttributes) diff --git a/src/utils/AttributionManager.js b/src/utils/AttributionManager.js index d8078485..95c6abae 100644 --- a/src/utils/AttributionManager.js +++ b/src/utils/AttributionManager.js @@ -80,13 +80,13 @@ export class AttributedContent { * @param {AbstractContent} content * @param {boolean} deleted * @param {Array> | null} attrs - * @param {boolean} render + * @param {0|1|2} renderBehavior */ - constructor (content, deleted, attrs, render) { + constructor (content, deleted, attrs, renderBehavior) { this.content = content this.deleted = deleted this.attrs = attrs - this.render = render + this.render = renderBehavior === 0 ? false : (renderBehavior === 1 ? (!deleted || attrs != null) : true) } } @@ -100,7 +100,7 @@ export class AbstractAttributionManager { * @param {number} _clock * @param {boolean} _deleted * @param {AbstractContent} _content - * @param {boolean} _shouldRender - whether this should render or just result in a `retain` operation + * @param {0|1|2} _shouldRender - 0: if undeleted or attributed, render as a retain operation. 1: render only if undeleted or attributed. 2: render as insert operation (if unattributed and deleted, render as delete). */ readContent (_contents, _client, _clock, _deleted, _content, _shouldRender) { error.methodUnimplemented() @@ -130,7 +130,7 @@ export class TwosetAttributionManager { * @param {number} clock * @param {boolean} deleted * @param {AbstractContent} content - * @param {boolean} shouldRender - whether this should render or just result in a `retain` operation + * @param {0|1|2} shouldRender - whether this should render or just result in a `retain` operation */ readContent (contents, client, clock, deleted, content, shouldRender) { const slice = (deleted ? this.deletes : this.inserts).slice(client, clock, content.getLength()) @@ -140,7 +140,7 @@ export class TwosetAttributionManager { if (s.len < c.getLength()) { content = c.splice(s.len) } - if (!deleted || s.attrs != null) { + if (!deleted || s.attrs != null || shouldRender) { contents.push(new AttributedContent(c, deleted, s.attrs, shouldRender)) } }) @@ -161,7 +161,7 @@ export class NoAttributionsManager { * @param {number} _clock * @param {boolean} deleted * @param {AbstractContent} content - * @param {boolean} shouldRender - whether this should render or just result in a `retain` operation + * @param {0|1|2} shouldRender - whether this should render or just result in a `retain` operation */ readContent (contents, _client, _clock, deleted, content, shouldRender) { if (!deleted || shouldRender) { @@ -236,7 +236,7 @@ export class DiffAttributionManager { * @param {number} clock * @param {boolean} deleted * @param {AbstractContent} content - * @param {boolean} shouldRender - whether this should render or just result in a `retain` operation + * @param {0|1|2} shouldRender - whether this should render or just result in a `retain` operation */ readContent (contents, client, clock, deleted, content, shouldRender) { const slice = (deleted ? this.deletes : this.inserts).slice(client, clock, content.getLength()) @@ -244,10 +244,11 @@ export class DiffAttributionManager { if (content instanceof ContentDeleted && slice[0].attrs != null && !this.inserts.has(client, clock)) { // Retrieved item is never more fragmented than the newer item. const prevItem = getItem(this._prevDocStore, createID(client, clock)) + const originalContentLen = content.getLength() content = prevItem.length > 1 ? prevItem.content.copy() : prevItem.content // trim itemContent to the correct size. - const diffStart = prevItem.id.clock - clock - const diffEnd = prevItem.id.clock + prevItem.length - clock - content.getLength() + const diffStart = clock - prevItem.id.clock + const diffEnd = prevItem.id.clock + prevItem.length - clock - originalContentLen if (diffStart > 0) { content = content.splice(diffStart) } @@ -260,7 +261,7 @@ export class DiffAttributionManager { if (s.len < c.getLength()) { content = c.splice(s.len) } - if (!deleted || s.attrs != null || shouldRender) { + if (shouldRender || !deleted || s.attrs != null) { contents.push(new AttributedContent(c, deleted, s.attrs, shouldRender)) } }) @@ -307,7 +308,7 @@ export class SnapshotAttributionManager { * @param {number} clock * @param {boolean} _deleted * @param {AbstractContent} content - * @param {boolean} shouldRender - whether this should render or just result in a `retain` operation + * @param {0|1|2} shouldRender - whether this should render or just result in a `retain` operation */ readContent (contents, client, clock, _deleted, content, shouldRender) { if ((this.nextSnapshot.sv.get(client) ?? 0) <= clock) return // future item that should not be displayed @@ -321,12 +322,12 @@ export class SnapshotAttributionManager { content = c.splice(s.len) } if (nonExistend) return - if (!deleted || shouldRender || (s.attrs != null && s.attrs.length > 0)) { + if (shouldRender || !deleted || (s.attrs != null && s.attrs.length > 0)) { let attrsWithoutChange = s.attrs?.filter(attr => attr.name !== 'change') ?? null if (s.attrs?.length === 0) { attrsWithoutChange = null } - contents.push(new AttributedContent(c, deleted, attrsWithoutChange, !deleted)) + contents.push(new AttributedContent(c, deleted, attrsWithoutChange, shouldRender)) } }) } diff --git a/src/utils/Doc.js b/src/utils/Doc.js index 317f8c21..83c65f3a 100644 --- a/src/utils/Doc.js +++ b/src/utils/Doc.js @@ -11,7 +11,9 @@ import { YXmlElement, YXmlFragment, transact, - ContentDoc, Item, Transaction, YEvent // eslint-disable-line + applyUpdate, + ContentDoc, Item, Transaction, YEvent, // eslint-disable-line + encodeStateAsUpdate } from '../internals.js' import { ObservableV2 } from 'lib0/observable' @@ -345,3 +347,12 @@ export class Doc extends ObservableV2 { super.destroy() } } + +/** + * @param {Doc} ydoc + */ +export const cloneDoc = ydoc => { + const clone = new Doc() + applyUpdate(clone, encodeStateAsUpdate(ydoc)) + return clone +} diff --git a/tests/attribution.tests.js b/tests/attribution.tests.js new file mode 100644 index 00000000..c4707a7c --- /dev/null +++ b/tests/attribution.tests.js @@ -0,0 +1,34 @@ +/** + * Testing if encoding/decoding compatibility and integration compatibility is given. + * We expect that the document always looks the same, even if we upgrade the integration algorithm, or add additional encoding approaches. + * + * The v1 documents were generated with Yjs v13.2.0 based on the randomisized tests. + */ + +import * as Y from '../src/index.js' +import * as t from 'lib0/testing' +import * as delta from '../src/utils/Delta.js' + +/** + * @param {t.TestCase} _tc + */ +export const testAttributedEvents = _tc => { + const ydoc = new Y.Doc() + const ytext = ydoc.getText() + ytext.insert(0, 'hello world') + const v1 = Y.cloneDoc(ydoc) + ydoc.transact(() => { + ytext.delete(6, 5) + }) + let am = Y.createAttributionManagerFromDiff(v1, ydoc) + const c1 = ytext.getDelta(am) + t.compare(c1, delta.createTextDelta().insert('hello ').insert('world', null, { delete: [] })) + let calledObserver = false + ytext.observe(event => { + const d = event.getDelta(am) + t.compare(d, delta.createTextDelta().retain(11).insert('!', null, { insert: [] })) + calledObserver = true + }) + ytext.insert(11, '!') + t.assert(calledObserver) +} diff --git a/tests/index.js b/tests/index.js index 753174da..148b524b 100644 --- a/tests/index.js +++ b/tests/index.js @@ -14,6 +14,7 @@ import * as relativePositions from './relativePositions.tests.js' import * as delta from './delta.tests.js' import * as idset from './IdSet.tests.js' import * as idmap from './IdMap.tests.js' +import * as attribution from './attribution.tests.js' import { runTests } from 'lib0/testing' import { isBrowser, isNode } from 'lib0/environment' @@ -24,7 +25,7 @@ if (isBrowser) { } const tests = { - doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, delta, idset, idmap + doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, delta, idset, idmap, attribution } const run = async () => {