From 4547b356415efd0a482bdf89c09aaaaf09b98a92 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Mon, 11 May 2020 01:45:27 +0200 Subject: [PATCH] cleanup formatting attributes --- src/index.js | 1 + src/types/AbstractType.js | 16 ++++ src/types/YText.js | 159 +++++++++++++++++++++++++++++++- tests/y-text.tests.js | 189 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 364 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 8fd0e0d4..5d3ad00c 100644 --- a/src/index.js +++ b/src/index.js @@ -26,6 +26,7 @@ export { ContentType, AbstractType, RelativePosition, + getTypeChildren, createRelativePositionFromTypeIndex, createRelativePositionFromJSON, createAbsolutePositionFromRelativePosition, diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index 94250ec0..b5014a99 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -19,6 +19,22 @@ import * as iterator from 'lib0/iterator.js' import * as error from 'lib0/error.js' import * as encoding from 'lib0/encoding.js' // eslint-disable-line +/** + * Accumulate all (list) children of a type and return them as an Array. + * + * @param {AbstractType} t + * @return {Array} + */ +export const getTypeChildren = t => { + let s = t._start + const arr = [] + while (s) { + arr.push(s) + s = s.right + } + return arr +} + /** * Call event listeners with an event. This will also add an event to all * parents (for `.observeDeep` handlers). diff --git a/src/types/YText.js b/src/types/YText.js index a9c768b6..e02b2514 100644 --- a/src/types/YText.js +++ b/src/types/YText.js @@ -14,15 +14,19 @@ import { callTypeObservers, transact, ContentEmbed, + GC, ContentFormat, ContentString, splitSnapshotAffectedStructs, + iterateDeletedStructs, + iterateStructs, ID, Doc, Item, Snapshot, Transaction // eslint-disable-line } from '../internals.js' import * as decoding from 'lib0/decoding.js' // eslint-disable-line import * as encoding from 'lib0/encoding.js' import * as object from 'lib0/object.js' +import * as map from 'lib0/map.js' /** * @param {any} a @@ -320,6 +324,110 @@ const formatText = (transaction, parent, left, right, currentAttributes, length, return insertNegatedAttributes(transaction, parent, left, right, negatedAttributes) } +/** + * Call this function after string content has been deleted in order to + * clean up formatting Items. + * + * @param {Transaction} transaction + * @param {Item} start + * @param {Item|null} end exclusive end, automatically iterates to the next Content Item + * @param {Map} startAttributes + * @param {Map} endAttributes This attribute is modified! + * @return {number} The amount of formatting Items deleted. + * + * @function + */ +const cleanupFormattingGap = (transaction, start, end, startAttributes, endAttributes) => { + while (end && end.content.constructor !== ContentString && end.content.constructor !== ContentEmbed) { + if (!end.deleted && end.content.constructor === ContentFormat) { + updateCurrentAttributes(endAttributes, /** @type {ContentFormat} */ (end.content)) + } + end = end.right + } + let cleanups = 0 + while (start !== end) { + if (!start.deleted) { + const content = start.content + switch (content.constructor) { + case ContentFormat: { + const { key, value } = /** @type {ContentFormat} */ (content) + if ((endAttributes.get(key) || null) !== value || (startAttributes.get(key) || null) === value) { + // Either this format is overwritten or it is not necessary because the attribute already existed. + start.delete(transaction) + cleanups++ + } + break + } + } + } + start = /** @type {Item} */ (start.right) + } + return cleanups +} + +/** + * @param {Transaction} transaction + * @param {Item | null} item + */ +const cleanupContextlessFormattingGap = (transaction, item) => { + // iterate until item.right is null or content + while (item && item.right && (item.right.deleted || (item.right.content.constructor !== ContentString && item.right.content.constructor !== ContentEmbed))) { + item = item.right + } + const attrs = new Set() + // iterate back until a content item is found + while (item && (item.deleted || (item.content.constructor !== ContentString && item.content.constructor !== ContentEmbed))) { + if (!item.deleted && item.content.constructor === ContentFormat) { + const key = /** @type {ContentFormat} */ (item.content).key + if (attrs.has(key)) { + item.delete(transaction) + } else { + attrs.add(key) + } + } + item = item.left + } +} + +/** + * This function is experimental and subject to change / be removed. + * + * Ideally, we don't need this function at all. Formatting attributes should be cleaned up + * automatically after each change. This function iterates twice over the complete YText type + * and removes unnecessary formatting attributes. This is also helpful for testing. + * + * This function won't be exported anymore as soon as there is confidence that the YText type works as intended. + * + * @param {YText} type + * @return {number} How many formatting attributes have been cleaned up. + */ +export const cleanupYTextFormatting = type => { + let res = 0 + transact(/** @type {Doc} */ (type.doc), transaction => { + let start = /** @type {Item} */ (type._start) + let end = type._start + let startAttributes = map.create() + const currentAttributes = map.copy(startAttributes) + while (end) { + if (end.deleted === false) { + switch (end.content.constructor) { + case ContentFormat: + updateCurrentAttributes(currentAttributes, /** @type {ContentFormat} */ (end.content)) + break + case ContentEmbed: + case ContentString: + res += cleanupFormattingGap(transaction, start, end, startAttributes, currentAttributes) + startAttributes = map.copy(currentAttributes) + start = end + break + } + } + end = end.right + } + }) + return res +} + /** * @param {Transaction} transaction * @param {Item|null} left @@ -332,6 +440,8 @@ const formatText = (transaction, parent, left, right, currentAttributes, length, * @function */ const deleteText = (transaction, left, right, currentAttributes, length) => { + const startAttrs = map.copy(currentAttributes) + const start = right while (length > 0 && right !== null) { if (right.deleted === false) { switch (right.content.constructor) { @@ -351,6 +461,9 @@ const deleteText = (transaction, left, right, currentAttributes, length) => { left = right right = right.right } + if (start) { + cleanupFormattingGap(transaction, start, right, startAttrs, map.copy(currentAttributes)) + } return { left, right } } @@ -649,7 +762,48 @@ export class YText extends AbstractType { * @param {Set} parentSubs Keys changed on this type. `null` if list was modified. */ _callObserver (transaction, parentSubs) { - callTypeObservers(this, transaction, new YTextEvent(this, transaction)) + const event = new YTextEvent(this, transaction) + const doc = transaction.doc + // If a remote change happened, we try to cleanup potential formatting duplicates. + if (!transaction.local) { + // check if another formatting item was inserted + let foundFormattingItem = false + for (const [client, afterClock] of transaction.afterState) { + const clock = transaction.beforeState.get(client) || 0 + if (afterClock === clock) { + continue + } + iterateStructs(transaction, /** @type {Array} */ (doc.store.clients.get(client)), clock, afterClock, item => { + // @ts-ignore + if (!item.deleted && item.content.constructor === ContentFormat) { + foundFormattingItem = true + } + }) + if (foundFormattingItem) { + break + } + } + transact(doc, t => { + if (foundFormattingItem) { + // If a formatting item was inserted, we simply clean the whole type. + // We need to compute currentAttributes for the current position anyway. + cleanupYTextFormatting(this) + } else { + // If no formatting attribute was inserted, we can make due with contextless + // formatting cleanups. + // Contextless: it is not necessary to compute currentAttributes for the affected position. + iterateDeletedStructs(t, transaction.deleteSet, item => { + if (item instanceof GC) { + return + } + if (item.parent === this) { + cleanupContextlessFormattingGap(t, item) + } + }) + } + }) + } + callTypeObservers(this, transaction, event) } /** @@ -916,6 +1070,9 @@ export class YText extends AbstractType { * @public */ format (index, length, attributes) { + if (length === 0) { + return + } const y = this.doc if (y !== null) { transact(y, transaction => { diff --git a/tests/y-text.tests.js b/tests/y-text.tests.js index ae3634a2..ff08615e 100644 --- a/tests/y-text.tests.js +++ b/tests/y-text.tests.js @@ -1,5 +1,8 @@ import * as Y from './testHelper.js' import * as t from 'lib0/testing.js' +import * as prng from 'lib0/prng.js' +import * as math from 'lib0/math.js' + const { init, compare } = Y /** @@ -180,3 +183,189 @@ export const testToDeltaEmbedNoAttributes = tc => { const delta0 = text0.toDelta() t.compare(delta0, [{ insert: 'a', attributes: { bold: true } }, { insert: { image: 'imageSrc.png' } }, { insert: 'b', attributes: { bold: true } }], 'toDelta does not set attributes key when no attributes are present') } + +/** + * @param {t.TestCase} tc + */ +export const testFormattingRemoved = tc => { + const { text0 } = init(tc, { users: 1 }) + text0.insert(0, 'ab', { bold: true }) + text0.delete(0, 2) + t.assert(Y.getTypeChildren(text0).length === 1) +} + +/** + * @param {t.TestCase} tc + */ +export const testFormattingRemovedInMidText = tc => { + const { text0 } = init(tc, { users: 1 }) + text0.insert(0, '1234') + text0.insert(2, 'ab', { bold: true }) + text0.delete(2, 2) + t.assert(Y.getTypeChildren(text0).length === 3) +} + +// RANDOM TESTS + +let charCounter = 0 + +const marks = [ + { bold: true }, + { italic: true }, + { italic: true, color: '#888' } +] + +const marksChoices = [ + undefined, + ...marks +] + +/** + * @type Array + */ +const qChanges = [ + /** + * @param {Y.Doc} y + * @param {prng.PRNG} gen + */ + (y, gen) => { // insert text + const ytext = y.getText('text') + const insertPos = prng.int32(gen, 0, ytext.toString().length) + const attrs = prng.oneOf(gen, marksChoices) + const text = charCounter++ + prng.word(gen) + ytext.insert(insertPos, text, attrs) + }, + /** + * @param {Y.Doc} y + * @param {prng.PRNG} gen + */ + (y, gen) => { // insert embed + const ytext = y.getText('text') + const insertPos = prng.int32(gen, 0, ytext.toString().length) + ytext.insertEmbed(insertPos, { image: 'https://user-images.githubusercontent.com/5553757/48975307-61efb100-f06d-11e8-9177-ee895e5916e5.png' }) + }, + /** + * @param {Y.Doc} y + * @param {prng.PRNG} gen + */ + (y, gen) => { // delete text + const ytext = y.getText('text') + const contentLen = ytext.toString().length + const insertPos = prng.int32(gen, 0, contentLen) + const overwrite = math.min(prng.int32(gen, 0, contentLen - insertPos), 2) + ytext.delete(insertPos, overwrite) + }, + /** + * @param {Y.Doc} y + * @param {prng.PRNG} gen + */ + (y, gen) => { // format text + const ytext = y.getText('text') + const contentLen = ytext.toString().length + const insertPos = prng.int32(gen, 0, contentLen) + const overwrite = math.min(prng.int32(gen, 0, contentLen - insertPos), 2) + const format = prng.oneOf(gen, marks) + ytext.format(insertPos, overwrite, format) + }, + /** + * @param {Y.Doc} y + * @param {prng.PRNG} gen + */ + (y, gen) => { // insert codeblock + const ytext = y.getText('text') + const insertPos = prng.int32(gen, 0, ytext.toString().length) + const text = charCounter++ + prng.word(gen) + const ops = [] + if (insertPos > 0) { + ops.push({ retain: insertPos }) + } + ops.push({ insert: text }, { insert: '\n', format: { 'code-block': true } }) + ytext.applyDelta(ops) + } +] + +/** + * @param {any} result + */ +const checkResult = result => { + for (let i = 1; i < result.testObjects.length; i++) { + const p1 = result.users[i].getText('text').toDelta() + const p2 = result.users[i].getText('text').toDelta() + t.compare(p1, p2) + } + // Uncomment this to find formatting-cleanup issues + // const cleanups = Y.cleanupYTextFormatting(result.users[0].getText('text')) + // t.assert(cleanups === 0) + return result +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges1 = tc => { + const { users } = checkResult(Y.applyRandomTests(tc, qChanges, 1)) + const cleanups = Y.cleanupYTextFormatting(users[0].getText('text')) + t.assert(cleanups === 0) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges2 = tc => { + const { users } = checkResult(Y.applyRandomTests(tc, qChanges, 2)) + const cleanups = Y.cleanupYTextFormatting(users[0].getText('text')) + t.assert(cleanups === 0) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges2Repeat = tc => { + for (let i = 0; i < 1000; i++) { + const { users } = checkResult(Y.applyRandomTests(tc, qChanges, 2)) + const cleanups = Y.cleanupYTextFormatting(users[0].getText('text')) + t.assert(cleanups === 0) + } +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges3 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 3)) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges30 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 30)) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges40 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 40)) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges70 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 70)) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges100 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 100)) +} + +/** + * @param {t.TestCase} tc + */ +export const testRepeatGenerateQuillChanges300 = tc => { + checkResult(Y.applyRandomTests(tc, qChanges, 300)) +}