From b6566b9a8eb1b6985ecff1e023154d2eceebc154 Mon Sep 17 00:00:00 2001 From: Ian Storm Taylor Date: Wed, 14 Sep 2016 16:46:42 -0700 Subject: [PATCH] fix history to only merge contiguous inserts/removals, fixes #312 --- lib/models/transform.js | 77 +++++++++++++++---- .../on-history/undo/delete-text/index.js | 26 +++++++ .../on-history/undo/delete-text/input.yaml | 7 ++ .../on-history/undo/delete-text/output.yaml | 7 ++ .../undo/insert-text-many-contiguous/index.js | 24 ++++++ .../insert-text-many-contiguous/input.yaml | 12 +++ .../insert-text-many-contiguous/output.yaml | 12 +++ .../index.js | 24 ++++++ .../input.yaml | 25 ++++++ .../output.yaml | 25 ++++++ .../index.js | 38 +++++++++ .../input.yaml | 25 ++++++ .../output.yaml | 25 ++++++ .../insert-text-many-non-contiguous/index.js | 26 +++++++ .../input.yaml | 12 +++ .../output.yaml | 12 +++ .../on-history/undo/insert-text/index.js | 27 +++++++ .../on-history/undo/insert-text/input.yaml | 7 ++ .../on-history/undo/insert-text/output.yaml | 7 ++ test/transforms/index.js | 30 ++++++++ 20 files changed, 435 insertions(+), 13 deletions(-) create mode 100644 test/transforms/fixtures/on-history/undo/delete-text/index.js create mode 100644 test/transforms/fixtures/on-history/undo/delete-text/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/delete-text/output.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/index.js create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/output.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/index.js create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/output.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/index.js create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/output.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/index.js create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/output.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text/index.js create mode 100644 test/transforms/fixtures/on-history/undo/insert-text/input.yaml create mode 100644 test/transforms/fixtures/on-history/undo/insert-text/output.yaml diff --git a/lib/models/transform.js b/lib/models/transform.js index 9886df566..1eb22743c 100644 --- a/lib/models/transform.js +++ b/lib/models/transform.js @@ -54,20 +54,10 @@ class Transform { // If there's a previous save point, determine if the new operations should // be merged into the previous ones. if (previous && merge == null) { - const types = operations.map(op => op.type) - const prevTypes = previous.map(op => op.type) - const edits = types.filter(type => type != 'set_selection') - const prevEdits = prevTypes.filter(type => type != 'set_selection') - const onlySelections = types.every(type => type == 'set_selection') - const onlyInserts = edits.length && edits.every(type => type == 'insert_text') - const onlyRemoves = edits.length && edits.every(type => type == 'remove_text') - const prevOnlyInserts = prevEdits.length && prevEdits.every(type => type == 'insert_text') - const prevOnlyRemoves = prevEdits.length && prevEdits.every(type => type == 'remove_text') - merge = ( - (onlySelections) || - (onlyInserts && prevOnlyInserts) || - (onlyRemoves && prevOnlyRemoves) + isOnlySelections(operations) || + isContiguousInserts(operations, previous) || + isContiguousRemoves(operations, previous) ) } @@ -92,6 +82,67 @@ Object.keys(Transforms).forEach((type) => { } }) +/** + * Check whether a list of `operations` only contains selection operations. + * + * @param {Array} operations + * @return {Boolean} + */ + +function isOnlySelections(operations) { + return operations.every(op => op.type == 'set_selection') +} + +/** + * Check whether a list of `operations` and a list of `previous` operations are + * contiguous text insertions. + * + * @param {Array} operations + * @param {Array} previous + */ + +function isContiguousInserts(operations, previous) { + const edits = operations.filter(op => op.type != 'set_selection') + const prevEdits = previous.filter(op => op.type != 'set_selection') + if (!edits.length || !prevEdits.length) return false + + const onlyInserts = edits.every(op => op.type == 'insert_text') + const prevOnlyInserts = prevEdits.every(op => op.type == 'insert_text') + if (!onlyInserts || !prevOnlyInserts) return false + + const first = edits[0] + const last = prevEdits[prevEdits.length - 1] + if (first.key != last.key) return false + if (first.offset != last.offset + last.text.length) return false + + return true +} + +/** + * Check whether a list of `operations` and a list of `previous` operations are + * contiguous text removals. + * + * @param {Array} operations + * @param {Array} previous + */ + +function isContiguousRemoves(operations, previous) { + const edits = operations.filter(op => op.type != 'set_selection') + const prevEdits = previous.filter(op => op.type != 'set_selection') + if (!edits.length || !prevEdits.length) return false + + const onlyRemoves = edits.every(op => op.type == 'remove_text') + const prevOnlyRemoves = prevEdits.every(op => op.type == 'remove_text') + if (!onlyRemoves || !prevOnlyRemoves) return false + + const first = edits[0] + const last = prevEdits[prevEdits.length - 1] + if (first.key != last.key) return false + if (first.offset + first.length != last.offset) return false + + return true +} + /** * Export. * diff --git a/test/transforms/fixtures/on-history/undo/delete-text/index.js b/test/transforms/fixtures/on-history/undo/delete-text/index.js new file mode 100644 index 000000000..6f2ee0bfa --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/delete-text/index.js @@ -0,0 +1,26 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + + const next = state + .transform() + .moveTo({ + anchorKey: first.key, + anchorOffset: 0, + focusKey: first.key, + focusOffset: first.length + }) + .delete() + .apply() + + .transform() + .undo() + .apply() + + assert.deepEqual(next.selection.toJS(), selection.toJS()) + return next +} diff --git a/test/transforms/fixtures/on-history/undo/delete-text/input.yaml b/test/transforms/fixtures/on-history/undo/delete-text/input.yaml new file mode 100644 index 000000000..27f668fe2 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/delete-text/input.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: word diff --git a/test/transforms/fixtures/on-history/undo/delete-text/output.yaml b/test/transforms/fixtures/on-history/undo/delete-text/output.yaml new file mode 100644 index 000000000..27f668fe2 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/delete-text/output.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: word diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/index.js b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/index.js new file mode 100644 index 000000000..b2cfc85e1 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/index.js @@ -0,0 +1,24 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.get(0) + + const next = state + .transform() + .collapseToStartOf(first) + .insertText('text') + .apply() + + .transform() + .insertText('text') + .apply() + + .transform() + .undo() + .apply() + + return next +} diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/input.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/input.yaml new file mode 100644 index 000000000..ee05966e8 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/input.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one + - kind: block + type: paragraph + nodes: + - kind: text + text: two diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/output.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/output.yaml new file mode 100644 index 000000000..ee05966e8 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-contiguous/output.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one + - kind: block + type: paragraph + nodes: + - kind: text + text: two diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/index.js b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/index.js new file mode 100644 index 000000000..eed456282 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/index.js @@ -0,0 +1,24 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const second = texts.get(1) + + const next = state + .transform() + .collapseToStartOf(second) + .insertText('text') + .apply() + + .transform() + .insertText('text') + .apply() + + .transform() + .undo() + .apply() + + return next +} diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/input.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/input.yaml new file mode 100644 index 000000000..e6285f32e --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/input.yaml @@ -0,0 +1,25 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph one + - kind: block + type: paragraph + nodes: + - kind: block + type: list + nodes: + - kind: text + text: list one + - kind: block + type: list + nodes: + - kind: text + text: list two + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph three diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/output.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/output.yaml new file mode 100644 index 000000000..e6285f32e --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-contiguous/output.yaml @@ -0,0 +1,25 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph one + - kind: block + type: paragraph + nodes: + - kind: block + type: list + nodes: + - kind: text + text: list one + - kind: block + type: list + nodes: + - kind: text + text: list two + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph three diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/index.js b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/index.js new file mode 100644 index 000000000..742d31ba9 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/index.js @@ -0,0 +1,38 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.get(0) + const second = texts.get(1) + const third = texts.get(2) + const fourth = texts.get(3) + + const next = state + .transform() + .collapseToStartOf(first) + .insertText('text') + .apply() + + .transform() + .collapseToStartOf(second) + .insertText('text') + .apply() + + .transform() + .collapseToStartOf(third) + .insertText('text') + .apply() + + .transform() + .collapseToStartOf(fourth) + .insertText('text') + .apply() + + .transform() + .undo() + .apply() + + return next +} diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/input.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/input.yaml new file mode 100644 index 000000000..e6285f32e --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/input.yaml @@ -0,0 +1,25 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph one + - kind: block + type: paragraph + nodes: + - kind: block + type: list + nodes: + - kind: text + text: list one + - kind: block + type: list + nodes: + - kind: text + text: list two + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph three diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/output.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/output.yaml new file mode 100644 index 000000000..0d73e1afb --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-nested-non-contiguous/output.yaml @@ -0,0 +1,25 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: textparagraph one + - kind: block + type: paragraph + nodes: + - kind: block + type: list + nodes: + - kind: text + text: textlist one + - kind: block + type: list + nodes: + - kind: text + text: textlist two + - kind: block + type: paragraph + nodes: + - kind: text + text: paragraph three diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/index.js b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/index.js new file mode 100644 index 000000000..0fc68e841 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/index.js @@ -0,0 +1,26 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.get(0) + const second = texts.get(1) + + const next = state + .transform() + .collapseToStartOf(first) + .insertText('text') + .apply() + + .transform() + .collapseToStartOf(second) + .insertText('text') + .apply() + + .transform() + .undo() + .apply() + + return next +} diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/input.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/input.yaml new file mode 100644 index 000000000..ee05966e8 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/input.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: one + - kind: block + type: paragraph + nodes: + - kind: text + text: two diff --git a/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/output.yaml b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/output.yaml new file mode 100644 index 000000000..1618b4e9c --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text-many-non-contiguous/output.yaml @@ -0,0 +1,12 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: textone + - kind: block + type: paragraph + nodes: + - kind: text + text: two diff --git a/test/transforms/fixtures/on-history/undo/insert-text/index.js b/test/transforms/fixtures/on-history/undo/insert-text/index.js new file mode 100644 index 000000000..3c7a21170 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text/index.js @@ -0,0 +1,27 @@ + +import assert from 'assert' + +export default function (state) { + const { document, selection } = state + const texts = document.getTexts() + const first = texts.first() + const range = selection.merge({ + anchorKey: first.key, + anchorOffset: 0, + focusKey: first.key, + focusOffset: 0 + }) + + const next = state + .transform() + .moveTo(range) + .insertText('text') + .apply() + + .transform() + .undo() + .apply() + + assert.deepEqual(next.selection.toJS(), selection.toJS()) + return next +} diff --git a/test/transforms/fixtures/on-history/undo/insert-text/input.yaml b/test/transforms/fixtures/on-history/undo/insert-text/input.yaml new file mode 100644 index 000000000..27f668fe2 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text/input.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: word diff --git a/test/transforms/fixtures/on-history/undo/insert-text/output.yaml b/test/transforms/fixtures/on-history/undo/insert-text/output.yaml new file mode 100644 index 000000000..27f668fe2 --- /dev/null +++ b/test/transforms/fixtures/on-history/undo/insert-text/output.yaml @@ -0,0 +1,7 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + text: word diff --git a/test/transforms/index.js b/test/transforms/index.js index 086e0a529..3a65e4092 100644 --- a/test/transforms/index.js +++ b/test/transforms/index.js @@ -127,4 +127,34 @@ describe('transforms', () => { }) } }) + + describe('on-history', () => { + const dir = resolve(__dirname, './fixtures/on-history') + const transforms = fs.readdirSync(dir) + + for (const transform of transforms) { + if (transform[0] == '.') continue + + describe(`${toCamel(transform)}()`, () => { + const transformDir = resolve(__dirname, './fixtures/on-history', transform) + const tests = fs.readdirSync(transformDir) + + for (const test of tests) { + if (test[0] == '.') continue + + it(test, () => { + const testDir = resolve(transformDir, test) + const fn = require(testDir).default + const input = readMetadata.sync(resolve(testDir, 'input.yaml')) + const expected = readMetadata.sync(resolve(testDir, 'output.yaml')) + + let state = Raw.deserialize(input, { terse: true }) + state = fn(state) + const output = Raw.serialize(state, { terse: true }) + strictEqual(strip(output), strip(expected)) + }) + } + }) + } + }) })