From 367a22c39ae73f85228841b5f90472b3bb25b38b Mon Sep 17 00:00:00 2001 From: Ian Storm Taylor Date: Tue, 5 Sep 2017 18:24:21 -0700 Subject: [PATCH] fix mark operation range creation --- src/changes/by-key.js | 86 +++++++++++++++---- .../index.js | 4 +- .../input.yaml | 0 .../output.yaml | 0 .../index.js | 4 +- .../input.yaml | 0 .../output.yaml | 0 .../toggle-mark/add-partially-marked/index.js | 4 +- .../add-partially-marked/input.yaml | 0 .../add-partially-marked/output.yaml | 0 .../index.js | 4 +- .../input.yaml | 0 .../output.yaml | 0 .../undo/add-mark-partially-exists/index.js | 18 ++++ .../undo/add-mark-partially-exists/input.yaml | 8 ++ .../add-mark-partially-exists/output.yaml | 8 ++ .../on-history/undo/add-mark/index.js | 2 +- .../on-history/undo/add-mark/input.yaml | 9 +- .../on-history/undo/add-mark/output.yaml | 8 +- .../remove-mark-partially-exists/index.js | 18 ++++ .../remove-mark-partially-exists/input.yaml | 8 ++ .../remove-mark-partially-exists/output.yaml | 8 ++ 22 files changed, 157 insertions(+), 32 deletions(-) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js (93%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/input.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/output.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js (93%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/input.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/output.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js (93%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-partially-marked/input.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/add-partially-marked/output.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js (94%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/input.yaml (100%) rename test/{transforms => changes}/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/output.yaml (100%) create mode 100644 test/changes/fixtures/on-history/undo/add-mark-partially-exists/index.js create mode 100644 test/changes/fixtures/on-history/undo/add-mark-partially-exists/input.yaml create mode 100644 test/changes/fixtures/on-history/undo/add-mark-partially-exists/output.yaml create mode 100644 test/changes/fixtures/on-history/undo/remove-mark-partially-exists/index.js create mode 100644 test/changes/fixtures/on-history/undo/remove-mark-partially-exists/input.yaml create mode 100644 test/changes/fixtures/on-history/undo/remove-mark-partially-exists/output.yaml diff --git a/src/changes/by-key.js b/src/changes/by-key.js index a9759d531..5433cda32 100644 --- a/src/changes/by-key.js +++ b/src/changes/by-key.js @@ -28,15 +28,41 @@ Changes.addMarkByKey = (change, key, offset, length, mark, options = {}) => { const { state } = change const { document } = state const path = document.getPath(key) + const node = document.getNode(key) + const ranges = node.getRanges() - change.applyOperation({ - type: 'add_mark', - path, - offset, - length, - mark, + const operations = [] + const bx = offset + const by = offset + length + let o = 0 + + ranges.forEach((range) => { + const ax = o + const ay = ax + range.text.length + + o += range.text.length + + // If the range doesn't overlap with the operation, continue on. + if (ay < bx || by < ax) return + + // If the range already has the mark, continue on. + if (range.marks.has(mark)) return + + // Otherwise, determine which offset and characters overlap. + const start = Math.max(ax, bx) + const end = Math.min(ay, by) + + operations.push({ + type: 'add_mark', + path, + offset: start, + length: end - start, + mark, + }) }) + change.applyOperations(operations) + if (normalize) { const parent = document.getParent(key) change.normalizeNodeByKey(parent.key, SCHEMA) @@ -211,15 +237,41 @@ Changes.removeMarkByKey = (change, key, offset, length, mark, options = {}) => { const { state } = change const { document } = state const path = document.getPath(key) + const node = document.getNode(key) + const ranges = node.getRanges() - change.applyOperation({ - type: 'remove_mark', - path, - offset, - length, - mark, + const operations = [] + const bx = offset + const by = offset + length + let o = 0 + + ranges.forEach((range) => { + const ax = o + const ay = ax + range.text.length + + o += range.text.length + + // If the range doesn't overlap with the operation, continue on. + if (ay < bx || by < ax) return + + // If the range already has the mark, continue on. + if (!range.marks.has(mark)) return + + // Otherwise, determine which offset and characters overlap. + const start = Math.max(ax, bx) + const end = Math.min(ay, by) + + operations.push({ + type: 'remove_mark', + path, + offset: start, + length: end - start, + mark, + }) }) + change.applyOperations(operations) + if (normalize) { const parent = document.getParent(key) change.normalizeNodeByKey(parent.key, SCHEMA) @@ -280,7 +332,6 @@ Changes.removeTextByKey = (change, key, offset, length, options = {}) => { let o = 0 ranges.forEach((range) => { - const { marks } = range const ax = o const ay = ax + range.text.length @@ -299,15 +350,12 @@ Changes.removeTextByKey = (change, key, offset, length, options = {}) => { path, offset: start, text: string, - marks, + marks: range.marks, }) }) - // Apply the removals in reverse order, so that subsequent removals aren't - // impacted by previous ones. - removals.reverse().forEach((op) => { - change.applyOperation(op) - }) + // Apply in reverse order, so subsequent removals don't impact previous ones. + change.applyOperations(removals.reverse()) if (normalize) { const block = document.getClosestBlock(key) diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js b/test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js similarity index 93% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js rename to test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js index a0cb8f6c6..12e75a2af 100644 --- a/test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js +++ b/test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/index.js @@ -13,11 +13,11 @@ export default function (state) { }) const next = state - .transform() + .change() .select(range) .toggleMark('bold') .insertText('a') - .apply() + .state assert.deepEqual(next.selection.toJS(), range.move(1).toJS()) diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/input.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/input.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/input.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/input.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/output.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/output.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/output.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-collapsed-selection-beginning/output.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js b/test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js similarity index 93% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js rename to test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js index 8d16ffc48..4f006d539 100644 --- a/test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js +++ b/test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/index.js @@ -13,10 +13,10 @@ export default function (state) { }) const next = state - .transform() + .change() .select(range) .toggleMark('bold') - .apply() + .state assert.deepEqual(next.selection.toJS(), range.toJS()) diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/input.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/input.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/input.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/input.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/output.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/output.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/output.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-existing-marks-partially-marked/output.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js b/test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js similarity index 93% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js rename to test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js index d62f3b970..fb443109f 100644 --- a/test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js +++ b/test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/index.js @@ -12,10 +12,10 @@ export default function (state) { }) const next = state - .transform() + .change() .select(range) .toggleMark('bold') - .apply() + .state assert.deepEqual(next.selection.toJS(), range.toJS()) diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/input.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/input.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/input.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/input.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/output.yaml b/test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/output.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/add-partially-marked/output.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/add-partially-marked/output.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js b/test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js similarity index 94% rename from test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js rename to test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js index 825758ebe..fd72cc6aa 100644 --- a/test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js +++ b/test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/index.js @@ -13,12 +13,12 @@ export default function (state) { }) const next = state - .transform() + .change() .select(range) .toggleMark('bold') .toggleMark('bold') .insertText('a') - .apply() + .state assert.deepEqual(next.selection.toJS(), range.move(1).toJS()) diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/input.yaml b/test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/input.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/input.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/input.yaml diff --git a/test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/output.yaml b/test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/output.yaml similarity index 100% rename from test/transforms/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/output.yaml rename to test/changes/fixtures/at-current-range/toggle-mark/remove-collapsed-selection-beginning/output.yaml diff --git a/test/changes/fixtures/on-history/undo/add-mark-partially-exists/index.js b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/index.js new file mode 100644 index 000000000..ebf3bd78f --- /dev/null +++ b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/index.js @@ -0,0 +1,18 @@ + +import assert from 'assert' + +export default function (state) { + const { selection } = state + + const next = state + .change() + .removeMarkByKey('a', 0, 4, 'bold') + .state + + .change() + .undo() + .state + + assert.deepEqual(next.selection.toJS(), selection.toJS()) + return next +} diff --git a/test/changes/fixtures/on-history/undo/add-mark-partially-exists/input.yaml b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/input.yaml new file mode 100644 index 000000000..0bc39d9ce --- /dev/null +++ b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/input.yaml @@ -0,0 +1,8 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + key: a + text: text diff --git a/test/changes/fixtures/on-history/undo/add-mark-partially-exists/output.yaml b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/output.yaml new file mode 100644 index 000000000..0bc39d9ce --- /dev/null +++ b/test/changes/fixtures/on-history/undo/add-mark-partially-exists/output.yaml @@ -0,0 +1,8 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + key: a + text: text diff --git a/test/changes/fixtures/on-history/undo/add-mark/index.js b/test/changes/fixtures/on-history/undo/add-mark/index.js index 956cc7b0e..b99ef34d6 100644 --- a/test/changes/fixtures/on-history/undo/add-mark/index.js +++ b/test/changes/fixtures/on-history/undo/add-mark/index.js @@ -6,7 +6,7 @@ export default function (state) { const next = state .change() - .addMarkByKey('key1', 0, 8, 'marktype') + .addMarkByKey('a', 0, 8, 'marktype') .state .change() diff --git a/test/changes/fixtures/on-history/undo/add-mark/input.yaml b/test/changes/fixtures/on-history/undo/add-mark/input.yaml index 4089e48f0..2dbcff6a6 100644 --- a/test/changes/fixtures/on-history/undo/add-mark/input.yaml +++ b/test/changes/fixtures/on-history/undo/add-mark/input.yaml @@ -4,5 +4,10 @@ nodes: type: paragraph nodes: - kind: text - key: 'key1' - text: The text + key: a + ranges: + - text: te + marks: [] + - text: xt + marks: + - type: bold diff --git a/test/changes/fixtures/on-history/undo/add-mark/output.yaml b/test/changes/fixtures/on-history/undo/add-mark/output.yaml index 4089e48f0..8808e2422 100644 --- a/test/changes/fixtures/on-history/undo/add-mark/output.yaml +++ b/test/changes/fixtures/on-history/undo/add-mark/output.yaml @@ -4,5 +4,9 @@ nodes: type: paragraph nodes: - kind: text - key: 'key1' - text: The text + key: a + ranges: + - text: te + - text: xt + marks: + - type: bold diff --git a/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/index.js b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/index.js new file mode 100644 index 000000000..63d99e6f5 --- /dev/null +++ b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/index.js @@ -0,0 +1,18 @@ + +import assert from 'assert' + +export default function (state) { + const { selection } = state + + const next = state + .change() + .addMarkByKey('a', 0, 4, 'bold') + .state + + .change() + .undo() + .state + + assert.deepEqual(next.selection.toJS(), selection.toJS()) + return next +} diff --git a/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/input.yaml b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/input.yaml new file mode 100644 index 000000000..0bc39d9ce --- /dev/null +++ b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/input.yaml @@ -0,0 +1,8 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + key: a + text: text diff --git a/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/output.yaml b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/output.yaml new file mode 100644 index 000000000..0bc39d9ce --- /dev/null +++ b/test/changes/fixtures/on-history/undo/remove-mark-partially-exists/output.yaml @@ -0,0 +1,8 @@ + +nodes: + - kind: block + type: paragraph + nodes: + - kind: text + key: a + text: text