From 52441cbb279869c8c7eb5bdc793806ff272ce84f Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Sat, 24 May 2025 01:14:21 +0200 Subject: [PATCH] [attribution] fixes for suggestion support in y-quill --- README.md | 2 + src/index.js | 1 + src/types/YText.js | 118 +++++++------------------------- src/utils/AttributionManager.js | 42 ++++++++---- src/utils/Delta.js | 2 +- tests/attribution.tests.js | 6 +- 6 files changed, 57 insertions(+), 114 deletions(-) diff --git a/README.md b/README.md index 0a4aed5c..06303df3 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,8 @@ Showcase](https://yjs-diagram.synergy.codes/). * [Open Collaboration Tools](https://www.open-collab.tools/) - Collaborative editing for your IDE or custom editor * [Typst](https://typst.app/) - Compose, edit, and automate technical documents +* [ProtonMail | Proton Docs](https://proton.me/drive/docs) - E2E encrypted +collaborative documents in Proton Drive. ## Table of Contents diff --git a/src/index.js b/src/index.js index 5c217707..691e3efc 100644 --- a/src/index.js +++ b/src/index.js @@ -117,6 +117,7 @@ export { iterateStructsByIdSet, createAttributionManagerFromDiff, createIdSet, + mergeIdSets, cloneDoc } from './internals.js' diff --git a/src/types/YText.js b/src/types/YText.js index 5ef62419..02729f78 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -130,13 +130,14 @@ export class ItemTextListPosition { } break } - default: + default: { const rightLen = this.am.contentLength(this.right) if (length < rightLen) { getItemCleanStart(transaction, createID(this.right.id.client, this.right.id.clock + length)) } length -= rightLen break + } } this.forward() } @@ -648,8 +649,8 @@ export class YTextEvent extends YEvent { * @public */ getDelta (am = noAttributionsManager) { - const whatToWatch = mergeIdSets([diffIdSet(this.transaction.insertSet, this.transaction.deleteSet), diffIdSet(this.transaction.deleteSet, this.transaction.insertSet)]) - return this.target.getDelta(am, whatToWatch) + const itemsToRender = mergeIdSets([diffIdSet(this.transaction.insertSet, this.transaction.deleteSet), diffIdSet(this.transaction.deleteSet, this.transaction.insertSet)]) + return this.target.getDelta(am, { itemsToRender, retainDeletes: true }) } /** @@ -846,12 +847,6 @@ export class YText extends AbstractType { } /** - * Render the difference to another ydoc (which can be empty) and highlight the differences with - * attributions. - * - * Note that deleted content that was not deleted in prevYdoc is rendered as an insertion with the - * attribution `{ isDeleted: true, .. }`. - * * @param {AbstractAttributionManager} am * @return {import('../utils/Delta.js').TextDelta} The Delta representation of this type. * @@ -859,92 +854,25 @@ export class YText extends AbstractType { */ getContent (am = noAttributionsManager) { return this.getDelta(am) - // this.doc ?? warnPrematureAccess() - // /** - // * @type {delta.TextDelta} - // */ - // const d = delta.createTextDelta() - // /** - // * @type {Array>} - // */ - // const cs = [] - // for (let item = this._start; item !== null; cs.length = 0) { - // // populate cs - // for (; item !== null && cs.length < 50; item = item.right) { - // am.readContent(cs, item, false) - // } - // for (let i = 0; i < cs.length; i++) { - // const { content, deleted, attrs } = cs[i] - // const { attribution, retainOnly } = createAttributionFromAttributionItems(attrs, deleted) - // switch (content.constructor) { - // case ContentString: { - // if (retainOnly) { - // d.retain(content.getLength(), null, attribution) - // } else { - // d.insert(/** @type {ContentString} */ (content).str, null, attribution) - // } - // break - // } - // case ContentType: - // case ContentEmbed: { - // if (retainOnly) { - // d.retain(content.getLength(), null, attribution) - // } else { - // d.insert(/** @type {ContentEmbed | ContentType} */ (content).getContent()[0], null, attribution) - // } - // break - // } - // case ContentFormat: { - // const contentFormat = /** @type {ContentFormat} */ (content) - // if (attribution != null) { - // /** - // * @type {import('../utils/Delta.js').Attribution} - // */ - // const formattingAttribution = object.assign({}, d.usedAttribution) - // const attributesChanged = /** @type {{ [key: string]: Array }} */ (formattingAttribution.attributes = object.assign({}, formattingAttribution.attributes ?? {})) - // if (contentFormat.value === null) { - // delete attributesChanged[contentFormat.key] - // } else { - // const by = attributesChanged[contentFormat.key] = attributesChanged[contentFormat.key]?.slice() ?? [] - // by.push(...((deleted ? attribution.delete : attribution.insert) ?? [])) - // const attributedAt = (deleted ? attribution.deletedAt : attribution.insertedAt) - // if (attributedAt) formattingAttribution.attributedAt = attributedAt - // } - // if (object.isEmpty(attributesChanged)) { - // d.useAttribution(null) - // } else { - // const attributedAt = (deleted ? attribution.deletedAt : attribution.insertedAt) - // if (attributedAt != null) formattingAttribution.attributedAt = attributedAt - // d.useAttribution(formattingAttribution) - // } - // } - // if (!deleted) { - // const currAttrs = d.usedAttributes - // if (contentFormat.value == null) { - // const nextAttrs = object.assign({}, currAttrs) - // delete nextAttrs[contentFormat.key] - // d.useAttributes(nextAttrs) - // } else { - // d.useAttributes(object.assign({}, currAttrs, { [contentFormat.key]: contentFormat.value })) - // } - // } - // break - // } - // } - // } - // } - // return d } /** + * Render the difference to another ydoc (which can be empty) and highlight the differences with + * attributions. + * + * Note that deleted content that was not deleted in prevYdoc is rendered as an insertion with the + * attribution `{ isDeleted: true, .. }`. + * * @param {AbstractAttributionManager} am - * @param {import('../utils/IdSet.js').IdSet?} itemsToRender - * @param {boolean} retainOnly - if true, retain the rendered items with attributes and attributions. + * @param {Object} [opts] + * @param {import('../utils/IdSet.js').IdSet?} [opts.itemsToRender] + * @param {boolean} [opts.retainInserts] - if true, retain rendered inserts with attributions + * @param {boolean} [opts.retainDeletes] - if true, retain rendered+attributed deletes only * @return {import('../utils/Delta.js').TextDelta} The Delta representation of this type. * * @public */ - getDelta (am = noAttributionsManager, itemsToRender = null, retainOnly = false) { + getDelta (am = noAttributionsManager, { itemsToRender = null, retainInserts = false, retainDeletes = false } = {}) { /** * @type {import('../utils/Delta.js').TextDelta} */ @@ -972,7 +900,7 @@ export class YText extends AbstractType { if (itemsToRender != null) { for (; item !== null && cs.length < 50; item = item.right) { const rslice = itemsToRender.slice(item.id.client, item.id.clock, item.length) - let itemContent = rslice.length > 1 ? item.content.copy() : item.content + const itemContent = rslice.length > 1 ? item.content.copy() : item.content for (let ir = 0; ir < rslice.length; ir++) { const idrange = rslice[ir] const content = itemContent @@ -1003,10 +931,10 @@ export class YText extends AbstractType { if (renderContent) { d.usedAttributes = currentAttributes usingCurrentAttributes = true - if (!retainOnly) { - d.insert(c.content.getContent()[0], null, attribution) - } else { + if (c.deleted ? retainDeletes : retainInserts) { d.retain(c.content.getLength(), null, attribution) + } else { + d.insert(c.content.getContent()[0], null, attribution) } } else if (renderDelete) { d.delete(1) @@ -1020,10 +948,10 @@ export class YText extends AbstractType { if (renderContent) { d.usedAttributes = currentAttributes usingCurrentAttributes = true - if (!retainOnly) { - d.insert(/** @type {ContentString} */ (c.content).str, null, attribution) - } else { + if (c.deleted ? retainDeletes : retainInserts) { d.retain(/** @type {ContentString} */ (c.content).str.length, null, attribution) + } else { + d.insert(/** @type {ContentString} */ (c.content).str, null, attribution) } } else if (renderDelete) { d.delete(c.content.getLength()) @@ -1302,7 +1230,7 @@ export class YText extends AbstractType { } /** - * @param {this} other + * @param {this} other */ [traits.EqualityTraitSymbol] (other) { return this.getContent().equals(other.getContent()) diff --git a/src/utils/AttributionManager.js b/src/utils/AttributionManager.js index 24c8e027..817d86cf 100644 --- a/src/utils/AttributionManager.js +++ b/src/utils/AttributionManager.js @@ -5,7 +5,6 @@ import { createDeleteSetFromStructStore, createIdMapFromIdSet, ContentDeleted, - Item, Snapshot, Doc, AbstractContent, IdMap, // eslint-disable-line insertIntoIdMap, insertIntoIdSet, diffIdMap, @@ -13,9 +12,12 @@ import { createAttributionItem, mergeIdMaps, createID, + mergeIdSets, + IdSet, Item, Snapshot, Doc, AbstractContent, IdMap // eslint-disable-line } from '../internals.js' import * as error from 'lib0/error' +import { ObservableV2 } from 'lib0/observable' /** * @typedef {Object} Attribution @@ -92,8 +94,13 @@ export class AttributedContent { /** * Abstract class for associating Attributions to content / changes + * + * Should fire an event when the attributions changed _after_ the original change happens. This + * Event will be used to update the attribution on the current content. + * + * @extends {ObservableV2<{change:(idset:IdSet,origin:any,local:boolean)=>void}>} */ -export class AbstractAttributionManager { +export class AbstractAttributionManager extends ObservableV2 { /** * @param {Array>} _contents - where to write the result * @param {number} _client @@ -116,25 +123,24 @@ export class AbstractAttributionManager { contentLength (_item) { error.methodUnimplemented() } - - destroy () {} } /** * @implements AbstractAttributionManager + * + * @extends {ObservableV2<{change:(idset:IdSet,origin:any,local:boolean)=>void}>} */ -export class TwosetAttributionManager { +export class TwosetAttributionManager extends ObservableV2 { /** * @param {IdMap} inserts * @param {IdMap} deletes */ constructor (inserts, deletes) { + super() this.inserts = inserts this.deletes = deletes } - destroy () {} - /** * @param {Array>} contents - where to write the result * @param {number} client @@ -174,10 +180,10 @@ export class TwosetAttributionManager { * Abstract class for associating Attributions to content / changes * * @implements AbstractAttributionManager + * + * @extends {ObservableV2<{change:(idset:IdSet,origin:any,local:boolean)=>void}>} */ -export class NoAttributionsManager { - destroy () {} - +export class NoAttributionsManager extends ObservableV2 { /** * @param {Array>} contents - where to write the result * @param {number} _client @@ -205,13 +211,16 @@ export const noAttributionsManager = new NoAttributionsManager() /** * @implements AbstractAttributionManager + * + * @extends {ObservableV2<{change:(idset:IdSet,origin:any,local:boolean)=>void}>} */ -export class DiffAttributionManager { +export class DiffAttributionManager extends ObservableV2 { /** * @param {Doc} prevDoc * @param {Doc} nextDoc */ constructor (prevDoc, nextDoc) { + super() const _nextDocInserts = createInsertionSetFromStructStore(nextDoc.store, false) // unmaintained const _prevDocInserts = createInsertionSetFromStructStore(prevDoc.store, false) // unmaintained const nextDocDeletes = createDeleteSetFromStructStore(nextDoc.store) // maintained @@ -227,7 +236,7 @@ export class DiffAttributionManager { const diffInserts = diffIdSet(tr.insertSet, _prevDocInserts) insertIntoIdMap(this.inserts, createIdMapFromIdSet(diffInserts, [])) // update deletes - const diffDeletes = diffIdSet(tr.deleteSet, prevDocDeletes) + const diffDeletes = diffIdSet(diffIdSet(tr.deleteSet, prevDocDeletes), this.inserts) insertIntoIdMap(this.deletes, createIdMapFromIdSet(diffDeletes, [])) // @todo fire update ranges on `diffInserts` and `diffDeletes` }) @@ -249,12 +258,14 @@ export class DiffAttributionManager { this.deletes = diffIdMap(this.deletes, tr.deleteSet) } // @todo fire update ranges on `tr.insertSet` and `tr.deleteSet` + this.emit('change', [mergeIdSets([tr.insertSet, tr.deleteSet]), tr.origin, tr.local]) }) this._destroyHandler = nextDoc.on('destroy', this.destroy.bind(this)) prevDoc.on('destroy', this._destroyHandler) } destroy () { + super.destroy() this._nextDoc.off('destroy', this._destroyHandler) this._prevDoc.off('destroy', this._destroyHandler) this._nextDoc.off('beforeObserverCalls', this._nextBOH) @@ -324,13 +335,16 @@ export const createAttributionManagerFromDiff = (prevDoc, nextDoc) => new DiffAt * read content similar to the previous snapshot api. Requires that `ydoc.gc` is turned off. * * @implements AbstractAttributionManager + * + * @extends {ObservableV2<{change:(idset:IdSet,origin:any,local:boolean)=>void}>} */ -export class SnapshotAttributionManager { +export class SnapshotAttributionManager extends ObservableV2 { /** * @param {Snapshot} prevSnapshot * @param {Snapshot} nextSnapshot */ constructor (prevSnapshot, nextSnapshot) { + super() this.prevSnapshot = prevSnapshot this.nextSnapshot = nextSnapshot const inserts = createIdMap() @@ -343,8 +357,6 @@ export class SnapshotAttributionManager { this.attrs = mergeIdMaps([diffIdMap(inserts, prevSnapshot.ds), deletes]) } - destroy () { } - /** * @param {Array>} contents - where to write the result * @param {number} client diff --git a/src/utils/Delta.js b/src/utils/Delta.js index 6f820667..362a6aaa 100644 --- a/src/utils/Delta.js +++ b/src/utils/Delta.js @@ -413,7 +413,7 @@ export class DeltaBuilder extends AbstractDelta { * @return {this} */ done () { - while (this.lastOp != null && this.lastOp instanceof RetainOp && this.lastOp.attributes === null) { + while (this.lastOp != null && this.lastOp instanceof RetainOp && this.lastOp.attributes === null && this.lastOp.attribution === null) { this.ops.pop() this.lastOp = this.ops[this.ops.length - 1] ?? null } diff --git a/tests/attribution.tests.js b/tests/attribution.tests.js index 54a2d686..c6bbeea2 100644 --- a/tests/attribution.tests.js +++ b/tests/attribution.tests.js @@ -20,7 +20,7 @@ export const testAttributedEvents = _tc => { ydoc.transact(() => { ytext.delete(6, 5) }) - let am = Y.createAttributionManagerFromDiff(v1, ydoc) + const am = Y.createAttributionManagerFromDiff(v1, ydoc) const c1 = ytext.getDelta(am) t.compare(c1, delta.createTextDelta().insert('hello ').insert('world', null, { delete: [] })) let calledObserver = false @@ -44,7 +44,7 @@ export const testInsertionsMindingAttributedContent = _tc => { ydoc.transact(() => { ytext.delete(6, 5) }) - let am = Y.createAttributionManagerFromDiff(v1, ydoc) + const am = Y.createAttributionManagerFromDiff(v1, ydoc) const c1 = ytext.getDelta(am) t.compare(c1, delta.createTextDelta().insert('hello ').insert('world', null, { delete: [] })) ytext.applyDelta(delta.createTextDelta().retain(11).insert('content'), am) @@ -62,7 +62,7 @@ export const testInsertionsIntoAttributedContent = _tc => { ydoc.transact(() => { ytext.insert(6, 'word') }) - let am = Y.createAttributionManagerFromDiff(v1, ydoc) + const am = Y.createAttributionManagerFromDiff(v1, ydoc) const c1 = ytext.getDelta(am) t.compare(c1, delta.createTextDelta().insert('hello ').insert('word', null, { insert: [] })) ytext.applyDelta(delta.createTextDelta().retain(9).insert('l'), am)