From 65796c8ba0bc21eb5532f686c3817ffab31cec36 Mon Sep 17 00:00:00 2001
From: Stan Gurenkov <klausflex@gmail.com>
Date: Mon, 19 Aug 2019 09:23:46 -0700
Subject: [PATCH] insertTextByPath should replace marks on the inserted text
 with the once that are provided (#2936)

It were trying to add those marks that are passed to the function to the existing once,
but it should replace them instead.

Example error behavior was:
* put cursor at the end of the marked text
* toggle marks
* enter text

expected:
text is being added without toggled marks

actual:
text is added with those marks applied
---
 packages/slate/src/commands/by-path.js        | 62 ++++++++++++++++++-
 .../different-set-of-marks.js                 | 30 +++++++++
 .../by-key/replace-marks-by-key/empty-set.js  | 30 +++++++++
 .../same-marks-different-data.js              | 32 ++++++++++
 .../replace-with-active-marks-with-data.js    |  5 +-
 .../replace-with-mark-and-active-mark.js      |  5 +-
 6 files changed, 153 insertions(+), 11 deletions(-)
 create mode 100644 packages/slate/test/commands/by-key/replace-marks-by-key/different-set-of-marks.js
 create mode 100644 packages/slate/test/commands/by-key/replace-marks-by-key/empty-set.js
 create mode 100644 packages/slate/test/commands/by-key/replace-marks-by-key/same-marks-different-data.js

diff --git a/packages/slate/src/commands/by-path.js b/packages/slate/src/commands/by-path.js
index 4d08472b9..79184c808 100644
--- a/packages/slate/src/commands/by-path.js
+++ b/packages/slate/src/commands/by-path.js
@@ -64,6 +64,62 @@ Commands.addMarksByPath = (editor, path, offset, length, marks) => {
   })
 }
 
+/**
+ * Sets specific set of marks on the path
+ * @param {Editor} editor
+ * @param {Array} path
+ * @param {Number} offset
+ * @param {Number} length
+ * @param {Array<Object|Mark>} marks
+ */
+
+Commands.replaceMarksByPath = (editor, path, offset, length, marks) => {
+  const marksSet = Mark.createSet(marks)
+
+  const { value } = editor
+  const { document } = value
+  const node = document.assertNode(path)
+
+  if (node.marks.equals(marksSet)) {
+    return
+  }
+
+  editor.withoutNormalizing(() => {
+    // If it ends before the end of the node, we'll need to split to create a new
+    // text with different marks.
+    if (offset + length < node.text.length) {
+      editor.splitNodeByPath(path, offset + length)
+    }
+
+    // Same thing if it starts after the start. But in that case, we need to
+    // update our path and offset to point to the new start.
+    if (offset > 0) {
+      editor.splitNodeByPath(path, offset)
+      path = PathUtils.increment(path)
+      offset = 0
+    }
+
+    const marksToApply = marksSet.subtract(node.marks)
+    const marksToRemove = node.marks.subtract(marksSet)
+
+    marksToRemove.forEach(mark => {
+      editor.applyOperation({
+        type: 'remove_mark',
+        path,
+        mark: Mark.create(mark),
+      })
+    })
+
+    marksToApply.forEach(mark => {
+      editor.applyOperation({
+        type: 'add_mark',
+        path,
+        mark: Mark.create(mark),
+      })
+    })
+  })
+}
+
 /**
  * Insert a `fragment` at `index` in a node by `path`.
  *
@@ -107,7 +163,6 @@ Commands.insertNodeByPath = (editor, path, index, node) => {
  */
 
 Commands.insertTextByPath = (editor, path, offset, text, marks) => {
-  marks = Mark.createSet(marks)
   const { value } = editor
   const { annotations, document } = value
   document.assertNode(path)
@@ -140,8 +195,8 @@ Commands.insertTextByPath = (editor, path, offset, text, marks) => {
       text,
     })
 
-    if (marks.size) {
-      editor.addMarksByPath(path, offset, text.length, marks)
+    if (marks) {
+      editor.replaceMarksByPath(path, offset, text.length, marks)
     }
   })
 }
@@ -726,6 +781,7 @@ const COMMANDS = [
   'removeMark',
   'removeNode',
   'removeText',
+  'replaceMarks',
   'replaceNode',
   'replaceText',
   'setMark',
diff --git a/packages/slate/test/commands/by-key/replace-marks-by-key/different-set-of-marks.js b/packages/slate/test/commands/by-key/replace-marks-by-key/different-set-of-marks.js
new file mode 100644
index 000000000..6561eca72
--- /dev/null
+++ b/packages/slate/test/commands/by-key/replace-marks-by-key/different-set-of-marks.js
@@ -0,0 +1,30 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.replaceMarksByKey('a', 0, 2, [{ type: 'italic' }])
+}
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        <b key="a" thing="value">
+          word
+        </b>
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        <i>wo</i>
+        <b thing="value">rd</b>
+      </paragraph>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/by-key/replace-marks-by-key/empty-set.js b/packages/slate/test/commands/by-key/replace-marks-by-key/empty-set.js
new file mode 100644
index 000000000..b6c0679c9
--- /dev/null
+++ b/packages/slate/test/commands/by-key/replace-marks-by-key/empty-set.js
@@ -0,0 +1,30 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.replaceMarksByKey('a', 0, 2, [])
+}
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        <b key="a" thing="value">
+          word
+        </b>
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        wo
+        <b thing="value">rd</b>
+      </paragraph>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/by-key/replace-marks-by-key/same-marks-different-data.js b/packages/slate/test/commands/by-key/replace-marks-by-key/same-marks-different-data.js
new file mode 100644
index 000000000..8e7c8c6b7
--- /dev/null
+++ b/packages/slate/test/commands/by-key/replace-marks-by-key/same-marks-different-data.js
@@ -0,0 +1,32 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default function(editor) {
+  editor.replaceMarksByKey('a', 0, 2, [
+    { type: 'bold', data: { thing: 'new value' } },
+  ])
+}
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        <b key="a" thing="value">
+          word
+        </b>
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        <b thing="new value">wo</b>
+        <b thing="value">rd</b>
+      </paragraph>
+    </document>
+  </value>
+)
diff --git a/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-active-marks-with-data.js b/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-active-marks-with-data.js
index 37c798eeb..d6316901f 100644
--- a/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-active-marks-with-data.js
+++ b/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-active-marks-with-data.js
@@ -27,10 +27,7 @@ export const output = (
   <value>
     <document>
       <paragraph>
-        Meow,{' '}
-        <fontSize size={16}>
-          <fontSize size={12}>cat is cute</fontSize>
-        </fontSize>
+        Meow, <fontSize size={16}>cat is cute</fontSize>
         <fontSize size={12}>
           <cursor />d.
         </fontSize>
diff --git a/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-mark-and-active-mark.js b/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-mark-and-active-mark.js
index cd889c4ff..fe2a60c07 100644
--- a/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-mark-and-active-mark.js
+++ b/packages/slate/test/commands/by-key/replace-text-by-key/replace-with-mark-and-active-mark.js
@@ -27,10 +27,7 @@ export const output = (
   <value>
     <document>
       <paragraph>
-        Meow,{' '}
-        <i>
-          <b>cat is cute</b>
-        </i>
+        Meow, <i>cat is cute</i>
         <b>
           <cursor />d.
         </b>