fix history to only merge contiguous inserts/removals, fixes #312

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) ||
+        isOnlySelections(operations) ||
-        (onlyInserts && prevOnlyInserts) ||
+        isContiguousInserts(operations, previous) ||
-        (onlyRemoves && prevOnlyRemoves)
+        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.
  *

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
+}

test/transforms/fixtures/on-history/undo/delete-text/input.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

test/transforms/fixtures/on-history/undo/delete-text/output.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

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
+}

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

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

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
+}

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

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

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
+}

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

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

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
+}

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

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

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
+}

test/transforms/fixtures/on-history/undo/insert-text/input.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

test/transforms/fixtures/on-history/undo/insert-text/output.yaml

@@ -0,0 +1,7 @@
+
+nodes:
+  - kind: block
+    type: paragraph
+    nodes:
+      - kind: text
+        text: word

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))
+          })
+        }
+      })
+    }
+  })
 })