use correct expand config when creating marks

Problem: the spans created by the plugin were hard coded to use "both"
or to use the default provided by Automerge. This doesn't always match
what a user might expect as ProseMirror allows configuring how marks
should expand using the "inclusive" attribute on mark specifications.

Solution: use the "inclusive" attribute to set the mark expand
configuration when creating marks in Automerge (including as part of the
updateSpans call).

Author: alexjg

src/pmToAm.ts

@@ -32,7 +32,7 @@ export default function (
   }
 
   for (const step of steps) {
-    //console.log(step)
+    // console.log(JSON.stringify(step))
     const stepId = step.toJSON()["stepType"]
     if (stepId === "addMark") {
       unappliedMarks.push(step as AddMarkStep)
@@ -119,7 +119,7 @@ function replaceStep(
     throw new Error("Could not apply step to document")
   }
   const newSpans = pmNodeToSpans(adapter, applied)
-  automerge.updateSpans(doc, field, newSpans)
+  automerge.updateSpans(doc, field, newSpans, adapter.updateSpansConfig())
 }
 
 function replaceAroundStep(
@@ -135,7 +135,7 @@ function replaceAroundStep(
     throw new Error("Could not apply step to document")
   }
   const newSpans = pmNodeToSpans(adapter, applied)
-  automerge.updateSpans(doc, field, newSpans)
+  automerge.updateSpans(doc, field, newSpans, adapter.updateSpansConfig())
 }
 
 function applyAddMarkSteps(
@@ -254,10 +254,14 @@ function reconcileMarks(
       !currentMarkNames.has(markName) ||
       newMarks[markName] !== currentMarks[markName]
     ) {
+      const expand =
+        (marks.find(m => m.type.name === markName)?.type.spec.inclusive ?? true)
+          ? "both"
+          : "none"
       automerge.mark(
         doc,
         path,
-        { start: index, end: index + length, expand: "both" },
+        { start: index, end: index + length, expand },
         markName,
         newMarks[markName],
       )

src/schema.ts

@@ -11,6 +11,8 @@ import {
 import { next as am } from "@automerge/automerge/slim"
 import { BlockMarker } from "./types.js"
 
+type ExpandConfig = "both" | "none"
+
 export interface MappedSchemaSpec {
   nodes: { [key: string]: MappedNodeSpec }
   marks?: { [key: string]: MappedMarkSpec }
@@ -179,6 +181,28 @@ by setting the automerge.unknownBlock property to true`,
     this.unknownBlock = unknownBlock
     this.schema = schema
   }
+
+  expandConfig(markName: string): ExpandConfig {
+    const mark = this.markMappings.find(
+      m => m.prosemirrorMark.name === markName,
+    )
+    return (mark?.prosemirrorMark.spec.inclusive ?? true) ? "both" : "none"
+  }
+
+  updateSpansConfig(): am.UpdateSpansConfig {
+    const config: am.UpdateSpansConfig = {
+      defaultExpand: "both",
+      perMarkExpand: {},
+    }
+
+    const perMark: { [key: string]: ExpandConfig } = {}
+    for (const mark of this.markMappings) {
+      perMark[mark.automergeMarkName] =
+        (mark.prosemirrorMark.spec.inclusive ?? true) ? "both" : "none"
+    }
+    config.perMarkExpand = perMark
+    return config
+  }
 }
 
 function shallowClone(spec: MappedSchemaSpec): MappedSchemaSpec {
@@ -236,7 +260,7 @@ function addAmgNodeStateAttrs(nodes: { [key: string]: MappedNodeSpec }): {
 export function amMarksFromPmMarks(
   adapter: SchemaAdapter,
   marks: readonly Mark[],
-): am.MarkSet {
+): { [key: string]: am.MarkValue } {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const result: { [key: string]: any } = {}
   marks.forEach(mark => {

test/pmToAm.spec.ts

@@ -1,26 +1,35 @@
 import { Fragment, Slice, Node } from "prosemirror-model"
 import { assertSplitBlock, makeDoc } from "./utils.js"
-import { AddMarkStep, ReplaceStep, Step } from "prosemirror-transform"
+import {
+  AddMarkStep,
+  ReplaceAroundStep,
+  ReplaceStep,
+  Step,
+} from "prosemirror-transform"
 import { default as pmToAm } from "../src/pmToAm.js"
 import { next as am } from "@automerge/automerge"
 import { assert } from "chai"
 import { pmDocFromSpans } from "../src/traversal.js"
 import { EditorState } from "prosemirror-state"
 import { basicSchemaAdapter } from "../src/basicSchema.js"
+import { ImmutableString } from "@automerge/automerge-repo"
 
 const schema = basicSchemaAdapter.schema
 
 function updateDoc(
   amDoc: am.Doc<unknown>,
   pmDoc: Node,
   steps: Step[],
-): am.Patch[] {
+): { diff: am.Patch[]; updatedDoc: am.Doc<unknown> } {
   const heads = am.getHeads(amDoc)
   const spans = am.spans(amDoc, ["text"])
   const updatedDoc = am.change(amDoc, d => {
     pmToAm(basicSchemaAdapter, spans, steps, d, pmDoc, ["text"])
   })
-  return am.diff(amDoc, heads, am.getHeads(updatedDoc))
+  return {
+    diff: am.diff(amDoc, heads, am.getHeads(updatedDoc)),
+    updatedDoc,
+  }
 }
 
 describe("when converting a ReplaceStep to a change", () => {
@@ -32,7 +41,7 @@ describe("when converting a ReplaceStep to a change", () => {
     // am:            0    1  2 3 4  5  6
     //     <doc><ol> <li> <p> i t e m ' ' 1 </p></li></ol></doc>
     // pm:0     0   1    2   3 4 5 6 7   8 9   10   11   12     13
-    const diff = updateDoc(doc, editor.doc, [
+    const { diff } = updateDoc(doc, editor.doc, [
       new ReplaceStep(
         9,
         9,
@@ -63,7 +72,7 @@ describe("when converting a ReplaceStep to a change", () => {
       { type: "ordered-list-item", parents: [], attrs: {} },
       "item 1",
     ])
-    const diff = updateDoc(doc, editor.doc, [
+    const { diff } = updateDoc(doc, editor.doc, [
       new ReplaceStep(
         11,
         11,
@@ -91,7 +100,7 @@ describe("when converting a ReplaceStep to a change", () => {
       { type: "paragraph", parents: ["ordered-list-item"], attrs: {} },
       "item 1",
     ])
-    const diff = updateDoc(doc, editor.doc, [
+    const { diff } = updateDoc(doc, editor.doc, [
       new ReplaceStep(
         9,
         9,
@@ -121,7 +130,7 @@ describe("when converting a ReplaceStep to a change", () => {
     //am:              0       1 2 3 4  5  6
     //     <doc> <ol> <li> <p> i t e m ' ' 1 </p> </li> </ol> </doc>
     //pm: 0     0    1    2   3 4 5 6 7   8 9   10     11    12     13
-    const diff = updateDoc(doc, editor.doc, [
+    const { diff } = updateDoc(doc, editor.doc, [
       new ReplaceStep(
         11,
         11,
@@ -229,6 +238,204 @@ describe("when converting a ReplaceStep to a change", () => {
       strong: [{ start: 5, end: 6, name: "strong", value: true }],
     })
   })
+
+  describe("expand configuration", () => {
+    // These tests all make use of the fact that the basicSchemaAdapter is
+    // configured with bold spans which are inclusive (in prosemirror terms)
+    // and link spans which are not inclusive. These tests check that the
+    // automerge expand configuration is correctly inferred from the schema.
+
+    it("should use the mark expand configuration from the schema", () => {
+      // We use ProseMirror to insert a range which has both a bold and link
+      // mark on it, then use Automerge to splice a character following this
+      // range and check that only the bold mark expands
+      const { editor, doc } = makeDoc(["text"])
+
+      // First make an insertion which adds marks
+      let { updatedDoc } = updateDoc(doc, editor.doc, [
+        new ReplaceStep(
+          1,
+          1,
+          new Slice(
+            Fragment.from(
+              editor.schema.text("1", [
+                editor.schema.marks.strong.create(),
+                editor.schema.marks.link.create({
+                  href: "https://example.com",
+                }),
+              ]),
+            ),
+            0,
+            0,
+          ),
+        ),
+      ])
+
+      // Now, insert into the automerge document after the initial character
+      updatedDoc = am.change(updatedDoc, d =>
+        am.splice(d as am.Doc<unknown>, ["text"], 1, 0, "2"),
+      )
+
+      const spans = am.spans(updatedDoc, ["text"])
+      // Here we expect the bold span to expand, because it's configured to do so
+      // in the schema, and the link span to not expand as it's configured to not
+      // expand
+      assert.deepStrictEqual(spans, [
+        {
+          marks: {
+            link: JSON.stringify({ href: "https://example.com", title: null }),
+            strong: true,
+          },
+          type: "text",
+          value: "1",
+        },
+        {
+          marks: {
+            strong: true,
+          },
+          type: "text",
+          value: "2",
+        },
+        {
+          type: "text",
+          value: "text",
+        },
+      ])
+    })
+  })
+
+  it("should use the correct mark config when performing more complex ReplaceSteps", () => {
+    // In this test we make a slightly more complex change which replaces some existing content
+    // with new blocks. This means that in the implementation we don't use splice but instead
+    // use updateSpans.
+    const { editor, doc } = makeDoc(["item 1"])
+
+    // Use ProseMirror to replace the entire document with a paragraph
+    // containing the text "item one" with both strong and link marks
+    let { updatedDoc } = updateDoc(doc, editor.doc, [
+      new ReplaceStep(
+        0,
+        8,
+        new Slice(
+          Fragment.from(
+            schema.node("ordered_list", null, [
+              schema.node("list_item", null, [
+                schema.node("paragraph", null, [
+                  schema.text("item one", [
+                    editor.schema.marks.strong.create(),
+                    editor.schema.marks.link.create({
+                      href: "https://example.com",
+                    }),
+                  ]),
+                ]),
+              ]),
+            ]),
+          ),
+          0,
+          0,
+        ),
+      ),
+    ])
+
+    // Now use Automerge to splice the text 'two' following the marked text we
+    // just created
+    updatedDoc = am.change(updatedDoc, d => {
+      am.splice(d as any, ["text"], 9, 0, "two")
+    })
+    const spans = am.spans(updatedDoc, ["text"])
+
+    // Here bold should expand as it is inclusive (in prosemirror terms) whilst
+    // the link span should not expand as it is not inclusive
+    assert.deepStrictEqual(spans, [
+      {
+        type: "block",
+        value: {
+          attrs: {},
+          isEmbed: false,
+          parents: [],
+          type: new ImmutableString("ordered-list-item"),
+        },
+      },
+      {
+        marks: {
+          link: '{"href":"https://example.com","title":null}',
+          strong: true,
+        },
+        type: "text",
+        value: "item one",
+      },
+      {
+        marks: {
+          strong: true,
+        },
+        type: "text",
+        value: "two",
+      },
+    ])
+  })
+
+  it("should use the correct mark config when performing ReplaceAroundSteps", () => {
+    // In this test we create a `ReplaceAroundStep` which again fires the updateSpans
+    // logic in the plugin
+    const { editor, doc } = makeDoc(["item one"])
+
+    // Insert <paragraph>intro</paragraph> into the document and move the
+    // existing content to occur after this paragraph. The newly inserted
+    // paragraph has a link and strong mark on it.
+    let { updatedDoc } = updateDoc(doc, editor.doc, [
+      new ReplaceAroundStep(
+        0,
+        10,
+        1,
+        9,
+        new Slice(
+          Fragment.from([
+            schema.node("paragraph", null, [
+              schema.text("intro", [
+                editor.schema.marks.strong.create(),
+                editor.schema.marks.link.create({
+                  href: "https://example.com",
+                }),
+              ]),
+            ]),
+          ]),
+          0,
+          0,
+        ),
+        6,
+      ),
+    ])
+
+    // Now use automerge to insert the text "middle" in the paragraph we just inserted
+    updatedDoc = am.change(updatedDoc, d => {
+      am.splice(d as any, ["text"], 5, 0, "middle")
+    })
+    const spans = am.spans(updatedDoc, ["text"])
+
+    // Here bold should expand as it is inclusive (in prosemirror terms) whilst
+    // the link span should not expand as it is not inclusive
+    assert.deepStrictEqual(spans, [
+      {
+        marks: {
+          link: '{"href":"https://example.com","title":null}',
+          strong: true,
+        },
+        type: "text",
+        value: "intro",
+      },
+      {
+        marks: {
+          strong: true,
+        },
+        type: "text",
+        value: "middle",
+      },
+      {
+        type: "text",
+        value: "item one",
+      },
+    ])
+  })
 })
 
 describe("when converting addMark steps to a change", () => {