From 90514dd51b047febe5e9689cd9778c660191cf31 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Sat, 24 May 2025 12:56:37 +0200 Subject: [PATCH] more attribution fixes for y-quill --- src/types/AbstractType.js | 4 +-- src/types/YText.js | 17 ++++++++----- src/utils/AttributionManager.js | 43 +++++++++++++++++---------------- tests/y-xml.tests.js | 6 ++--- 4 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index 57b3aa6f..ce453e8a 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -518,7 +518,7 @@ export const typeListGetContent = (type, am) => { } for (let i = 0; i < cs.length; i++) { const c = cs[i] - const attribution = createAttributionFromAttributionItems(c.attrs, c.deleted).attribution + const attribution = createAttributionFromAttributionItems(c.attrs, c.deleted) if (c.content.isCountable()) { if (c.render || attribution != null) { d.insert(c.content.getContent(), null, attribution) @@ -1014,7 +1014,7 @@ export const typeMapGetContent = (parent, am) => { am.readContent(cs, item.id.client, item.id.clock, item.deleted, item.content, 1) const { deleted, attrs, content } = cs[cs.length - 1] const c = array.last(content.getContent()) - const { attribution } = createAttributionFromAttributionItems(attrs, deleted) + const attribution = createAttributionFromAttributionItems(attrs, deleted) if (deleted) { mapcontent[key] = { prevValue: c, value: undefined, attribution } } else { diff --git a/src/types/YText.js b/src/types/YText.js index 02729f78..2fbb1d8c 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -27,7 +27,8 @@ import { noAttributionsManager, AbstractAttributionManager, ArraySearchMarker, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Item, Transaction, // eslint-disable-line createAttributionFromAttributionItems, mergeIdSets, - diffIdSet + diffIdSet, + ContentDeleted } from '../internals.js' import * as delta from '../utils/Delta.js' @@ -900,12 +901,12 @@ 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) - const itemContent = rslice.length > 1 ? item.content.copy() : item.content + let itemContent = rslice.length > 1 ? item.content.copy() : item.content for (let ir = 0; ir < rslice.length; ir++) { const idrange = rslice[ir] const content = itemContent if (ir !== rslice.length - 1) { - itemContent.splice(idrange.len) + itemContent = itemContent.splice(idrange.len) } am.readContent(cs, item.id.client, idrange.clock, item.deleted, content, idrange.exists ? 2 : 0) } @@ -924,15 +925,19 @@ export class YText extends AbstractType { 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 + const attribution = renderContent ? createAttributionFromAttributionItems(c.attrs, c.deleted) : null switch (c.content.constructor) { + case ContentDeleted: { + if (renderDelete) d.delete(c.content.getLength()) + break + } case ContentType: case ContentEmbed: if (renderContent) { d.usedAttributes = currentAttributes usingCurrentAttributes = true if (c.deleted ? retainDeletes : retainInserts) { - d.retain(c.content.getLength(), null, attribution) + d.retain(c.content.getLength(), null, attribution ?? {}) } else { d.insert(c.content.getContent()[0], null, attribution) } @@ -949,7 +954,7 @@ export class YText extends AbstractType { d.usedAttributes = currentAttributes usingCurrentAttributes = true if (c.deleted ? retainDeletes : retainInserts) { - d.retain(/** @type {ContentString} */ (c.content).str.length, null, attribution) + d.retain(/** @type {ContentString} */ (c.content).str.length, null, attribution ?? null) } else { d.insert(/** @type {ContentString} */ (c.content).str, null, attribution) } diff --git a/src/utils/AttributionManager.js b/src/utils/AttributionManager.js index 817d86cf..573ea133 100644 --- a/src/utils/AttributionManager.js +++ b/src/utils/AttributionManager.js @@ -13,18 +13,23 @@ import { mergeIdMaps, createID, mergeIdSets, - IdSet, Item, Snapshot, Doc, AbstractContent, IdMap // eslint-disable-line + IdSet, Item, Snapshot, Doc, AbstractContent, IdMap, // eslint-disable-line + intersectSets } from '../internals.js' import * as error from 'lib0/error' import { ObservableV2 } from 'lib0/observable' /** + * @todo rename this to `insertBy`, `insertAt`, .. + * * @typedef {Object} Attribution * @property {Array} [Attribution.insert] * @property {number} [Attribution.insertedAt] - * @property {Array} [Attribution.suggest] - * @property {number} [Attribution.suggestedAt] + * @property {Array} [Attribution.acceptInsert] + * @property {number} [Attribution.acceptedDeleteAt] + * @property {Array} [Attribution.acceptDelete] + * @property {number} [Attribution.acceptedDeleteAt] * @property {Array} [Attribution.delete] * @property {number} [Attribution.deletedAt] * @property {{ [key: string]: Array }} [Attribution.attributes] @@ -35,16 +40,15 @@ import { ObservableV2 } from 'lib0/observable' * @todo SHOULD NOT RETURN AN OBJECT! * @param {Array>?} attrs * @param {boolean} deleted - whether the attributed item is deleted - * @return {{ attribution: Attribution?, retainOnly: boolean }} + * @return {Attribution?} */ export const createAttributionFromAttributionItems = (attrs, deleted) => { + if (attrs == null) return null /** - * @type {Attribution?} + * @type {Attribution} */ - let attribution = null - let retainOnly = false + const attribution = {} if (attrs != null) { - attribution = {} if (deleted) { attribution.delete = [] } else { @@ -52,12 +56,14 @@ export const createAttributionFromAttributionItems = (attrs, deleted) => { } attrs.forEach(attr => { switch (attr.name) { - case 'retain': - retainOnly = true - break + case 'acceptDelete': + delete attribution.delete + // eslint-disable-next-line no-fallthrough + case 'acceptInsert': + delete attribution.insert + // eslint-disable-next-line no-fallthrough case 'insert': - case 'delete': - case 'suggest': { + case 'delete': { const as = /** @type {import('../utils/Delta.js').Attribution} */ (attribution) const ls = as[attr.name] = as[attr.name] ?? [] ls.push(attr.val) @@ -71,7 +77,7 @@ export const createAttributionFromAttributionItems = (attrs, deleted) => { } }) } - return { attribution, retainOnly } + return attribution } /** @@ -243,13 +249,8 @@ export class DiffAttributionManager extends ObservableV2 { this._prevBOH = prevDoc.on('beforeObserverCalls', tr => { insertIntoIdSet(_prevDocInserts, tr.insertSet) insertIntoIdSet(prevDocDeletes, tr.deleteSet) - if (tr.insertSet.clients.size < 2) { - tr.insertSet.forEach((idrange, client) => { - this.inserts.delete(client, idrange.clock, idrange.len) - }) - } else { - this.inserts = diffIdMap(this.inserts, tr.insertSet) - } + insertIntoIdMap(this.inserts, createIdMapFromIdSet(intersectSets(tr.insertSet, this.inserts), [createAttributionItem('acceptInsert', 'unknown')])) + // insertIntoIdMap(this.deletes, createIdMapFromIdSet(intersectSets(tr.deleteSet, this.deletes), [createAttributionItem('acceptDelete', 'unknown')])) if (tr.deleteSet.clients.size < 2) { tr.deleteSet.forEach((attrRange, client) => { this.deletes.delete(client, attrRange.clock, attrRange.len) diff --git a/tests/y-xml.tests.js b/tests/y-xml.tests.js index 44a4eb84..48fe8f37 100644 --- a/tests/y-xml.tests.js +++ b/tests/y-xml.tests.js @@ -364,14 +364,14 @@ export const testElementAttributedContentViaDiffer = _tc => { t.group('test getContentDeep both docs synced', () => { t.info('expecting diffingAttributionManager to auto update itself') const expectedContent = delta.createArrayDelta().insert([{ nodeName: 'span', children: delta.createArrayDelta(), attributes: {} }]).insert([ - delta.createTextDelta().insert('bigworld') - ]) + delta.createTextDelta().insert('bigworld', null, { acceptInsert: ['unknown'] }) + ], null, { acceptInsert: ['unknown'] }) const attributedContent = yelement.getContentDeep(attributionManager) console.log('children', JSON.stringify(attributedContent.children.toJSON(), null, 2)) console.log('cs expec', JSON.stringify(expectedContent.toJSON(), null, 2)) console.log('attributes', attributedContent.attributes) t.assert(attributedContent.children.equals(expectedContent)) - t.compare(attributedContent.attributes, { key: { prevValue: undefined, value: '42', attribution: null } }) + t.compare(attributedContent.attributes, { key: { prevValue: undefined, value: '42', attribution: { acceptInsert: ['unknown'] } } }) t.assert(attributedContent.nodeName === 'UNDEFINED') }) }