diff --git a/packages/super-editor/src/editors/v1/core/helpers/markdown/constants.ts b/packages/super-editor/src/editors/v1/core/helpers/markdown/constants.ts new file mode 100644 index 0000000000..9ce2483259 --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/helpers/markdown/constants.ts @@ -0,0 +1,18 @@ +/** + * Shared constants for the Markdown ↔ ProseMirror conversion pipeline. + */ + +/** + * The font family used to mark monospace ("code") text in both directions of + * the markdown ↔ ProseMirror conversion: + * - `mdastToProseMirror.ts` applies this font (via a `textStyle` mark and/or + * direct `runProperties.fontFamily`) to fenced code blocks (`code`) and + * inline code spans (`inlineCode`). + * - `proseMirrorToMdast.ts` detects this font (on either the mark or the + * direct run properties) to convert monospace runs back into `inlineCode` + * mdast nodes. + * + * Keeping this as a single shared constant avoids the two directions + * silently drifting out of sync if the monospace font ever changes. + */ +export const MARKDOWN_MONOSPACE_FONT = 'Courier New'; diff --git a/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.test.ts b/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.test.ts new file mode 100644 index 0000000000..fbf121693d --- /dev/null +++ b/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from 'vitest'; +import { Schema } from 'prosemirror-model'; +import { convertMdastToBlocks } from './mdastToProseMirror.js'; +import { MARKDOWN_MONOSPACE_FONT } from './constants.js'; +import type { MdastConversionContext } from './types.js'; +import type { Root, Code, Paragraph, InlineCode } from 'mdast'; + +// --------------------------------------------------------------------------- +// Minimal schema mirroring SuperEditor's node/mark types (enough for +// mdastToProseMirror's JSON output to round-trip through nodeFromJSON if +// a test wants to materialize it — most assertions here work directly on +// the raw JSON returned by convertMdastToBlocks). +// --------------------------------------------------------------------------- + +const schema = new Schema({ + nodes: { + doc: { content: 'block+' }, + paragraph: { + group: 'block', + content: 'inline*', + attrs: { + paragraphProperties: { default: null }, + numberingProperties: { default: null }, + }, + }, + run: { + inline: true, + group: 'inline', + content: 'inline*', + attrs: { runProperties: { default: null } }, + }, + text: { group: 'inline' }, + lineBreak: { inline: true, group: 'inline' }, + }, + marks: { + bold: {}, + italic: {}, + strike: {}, + textStyle: { attrs: { fontFamily: { default: null } } }, + }, +}); + +function createMockEditor(): any { + return { + converter: { numbering: { definitions: {}, abstracts: {} } }, + state: { doc: { descendants: () => {} } }, + }; +} + +function makeCtx(): MdastConversionContext { + return { + editor: createMockEditor(), + schema, + diagnostics: [], + options: { dryRun: true }, + }; +} + +/** Build a minimal mdast Root wrapping a single `code` node. */ +function codeRoot(value: string, lang: string | null = null): Root { + const code: Code = { type: 'code', lang, value }; + return { type: 'root', children: [code] }; +} + +/** Build a minimal mdast Root wrapping a paragraph containing an inlineCode span. */ +function inlineCodeRoot(value: string): Root { + const inlineCode: InlineCode = { type: 'inlineCode', value }; + const paragraph: Paragraph = { type: 'paragraph', children: [inlineCode] }; + return { type: 'root', children: [paragraph] }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('convertMdastToBlocks — code block (fenced)', () => { + it('wraps a single-line code block in a paragraph of one run', () => { + const blocks = convertMdastToBlocks(codeRoot('const x = 1;'), makeCtx()); + + expect(blocks).toHaveLength(1); + const [paragraph] = blocks; + expect(paragraph.type).toBe('paragraph'); + expect(paragraph.content).toHaveLength(1); + + const [run] = paragraph.content!; + expect(run.type).toBe('run'); + expect(run.content).toHaveLength(1); + expect(run.content![0]).toMatchObject({ type: 'text', text: 'const x = 1;' }); + }); + + it('sets both the textStyle mark and the direct runProperties.fontFamily attr', () => { + const blocks = convertMdastToBlocks(codeRoot('code'), makeCtx()); + const run = blocks[0].content![0]; + + // Mark (consumed by calculateInlineRunPropertiesPlugin's mark-based recalculation) + expect(run.content![0].marks).toEqual([{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }]); + + // Direct attrs (consumed by callers that never dispatch through a live Editor) + expect(run.attrs).toEqual({ + runProperties: { + fontFamily: { + ascii: MARKDOWN_MONOSPACE_FONT, + eastAsia: MARKDOWN_MONOSPACE_FONT, + hAnsi: MARKDOWN_MONOSPACE_FONT, + cs: MARKDOWN_MONOSPACE_FONT, + }, + }, + }); + }); + + it('uses the OOXML rFonts key ("fontFamily"), never a bare "rFonts" key', () => { + const blocks = convertMdastToBlocks(codeRoot('code'), makeCtx()); + const run = blocks[0].content![0]; + + expect(run.attrs!.runProperties).toHaveProperty('fontFamily'); + expect(run.attrs!.runProperties).not.toHaveProperty('rFonts'); + }); + + it('joins multiple lines with lineBreak nodes, one run per non-empty line', () => { + const blocks = convertMdastToBlocks(codeRoot('line one\nline two\nline three'), makeCtx()); + const content = blocks[0].content!; + + // run, lineBreak, run, lineBreak, run + expect(content.map((n) => n.type)).toEqual(['run', 'lineBreak', 'run', 'lineBreak', 'run']); + expect(content[0].content![0].text).toBe('line one'); + expect(content[2].content![0].text).toBe('line two'); + expect(content[4].content![0].text).toBe('line three'); + }); + + it('emits a lineBreak but no run for a blank line within the code block', () => { + const blocks = convertMdastToBlocks(codeRoot('line one\n\nline three'), makeCtx()); + const content = blocks[0].content!; + + // run, lineBreak, lineBreak, run — no run is emitted for the empty middle line + expect(content.map((n) => n.type)).toEqual(['run', 'lineBreak', 'lineBreak', 'run']); + }); + + it('produces an empty paragraph for an empty code block', () => { + const blocks = convertMdastToBlocks(codeRoot(''), makeCtx()); + + expect(blocks).toHaveLength(1); + expect(blocks[0].type).toBe('paragraph'); + expect(blocks[0].content).toBeUndefined(); + }); +}); + +describe('convertMdastToBlocks — inline code', () => { + it('wraps inline code in a run with only the textStyle mark (no direct runProperties)', () => { + const blocks = convertMdastToBlocks(inlineCodeRoot('inline'), makeCtx()); + const run = blocks[0].content![0]; + + expect(run.type).toBe('run'); + expect(run.content![0]).toMatchObject({ + type: 'text', + text: 'inline', + marks: [{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }], + }); + + // Unlike the fenced code-block case, inlineCode relies solely on the mark — + // see the comment on convertCodeBlock for why these two paths differ. + expect(run.attrs).toBeUndefined(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.ts b/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.ts index 480808b2d1..1aaaa833a1 100644 --- a/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.ts +++ b/packages/super-editor/src/editors/v1/core/helpers/markdown/mdastToProseMirror.ts @@ -37,6 +37,7 @@ import { ListHelpers } from '../list-numbering-helpers.js'; import { generateDocxRandomId } from '../generateDocxRandomId.js'; import { readImageDimensionsFromDataUri } from '../../super-converter/image-dimensions.js'; import type { MdastConversionContext, MarkdownDiagnostic } from './types.js'; +import { MARKDOWN_MONOSPACE_FONT } from './constants.js'; // --------------------------------------------------------------------------- // Public entry point @@ -274,9 +275,44 @@ function convertCodeBlock(node: MdastCode, ctx: MdastConversionContext): JsonNod content.push({ type: 'lineBreak' }); } if (lines[i].length > 0) { - content.push(makeRun(lines[i], [], { rFonts: { ascii: 'Courier New', hAnsi: 'Courier New' } })); + // Use a `textStyle` mark (not just a direct `runProperties.fontFamily` + // attr) so `calculateInlineRunPropertiesPlugin`'s mark-based + // recalculation (which treats `fontFamily` as mark-derived) has a mark + // to decode the font from. A bare `runProperties.fontFamily` with no + // matching mark gets dropped on the first `editor.dispatch(tr)`, since + // that plugin recomputes inline run properties from marks/styles and + // `fontFamily` is not preserved via the existing-props fallback for + // mark-derived keys. + // + // Unlike the `inlineCode` case below, we ALSO set the direct + // `runProperties.fontFamily` attr here (belt-and-suspenders, not a true + // mirror of `inlineCode`). That's deliberate: code-block JSON can be + // consumed by callers that never dispatch a transaction through a live + // `Editor` (e.g. inspecting the converter's raw JSON output directly), + // in which case `calculateInlineRunPropertiesPlugin` never runs and the + // mark alone wouldn't populate `runProperties`. `inlineCode` spans are + // always produced within content destined for an editor dispatch, so a + // mark alone is sufficient there. Once dispatched, the plugin + // recomputes `runProperties.fontFamily` from the mark via + // `decodeRPrFromMarks` anyway (see styles.js). That decoder always sets + // all four rFonts slots (ascii/eastAsia/hAnsi/cs) to the same value, so + // the direct attrs set here match that 4-key shape upfront — otherwise + // the 2-key shape would be silently reshaped by the plugin on first + // dispatch, and any pre-dispatch consumer of the raw JSON would see a + // different (incomplete) shape than a post-dispatch consumer. + content.push( + makeRun(lines[i], [{ type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }], { + fontFamily: { + ascii: MARKDOWN_MONOSPACE_FONT, + eastAsia: MARKDOWN_MONOSPACE_FONT, + hAnsi: MARKDOWN_MONOSPACE_FONT, + cs: MARKDOWN_MONOSPACE_FONT, + }, + }), + ); } } + return makeParagraph(content); } @@ -443,7 +479,7 @@ function convertInlineNode(node: PhrasingContent, ctx: MdastConversionContext, p return [ makeRun((node as MdastInlineCode).value, [ ...parentMarks, - { type: 'textStyle', attrs: { fontFamily: 'Courier New' } }, + { type: 'textStyle', attrs: { fontFamily: MARKDOWN_MONOSPACE_FONT } }, ]), ]; diff --git a/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.test.ts b/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.test.ts index d696033bed..108131241a 100644 --- a/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.test.ts +++ b/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.test.ts @@ -287,7 +287,7 @@ describe('proseMirrorDocToMdast', () => { }); describe('inline code via run properties', () => { - it('converts Courier New rFonts in runProperties to inlineCode', () => { + it('converts Courier New fontFamily in runProperties to inlineCode', () => { const doc = buildDoc({ type: 'doc', content: [ @@ -296,7 +296,7 @@ describe('proseMirrorDocToMdast', () => { content: [ { type: 'run', - attrs: { runProperties: { rFonts: { ascii: 'Courier New', hAnsi: 'Courier New' } } }, + attrs: { runProperties: { fontFamily: { ascii: 'Courier New', hAnsi: 'Courier New' } } }, content: [{ type: 'text', text: 'monospace' }], }, ], diff --git a/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.ts b/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.ts index a6e997cb46..70d027d8d5 100644 --- a/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.ts +++ b/packages/super-editor/src/editors/v1/core/helpers/markdown/proseMirrorToMdast.ts @@ -39,6 +39,7 @@ import type { } from 'mdast'; import { ListHelpers } from '../list-numbering-helpers.js'; import type { Editor } from '../../Editor.js'; +import { MARKDOWN_MONOSPACE_FONT } from './constants.js'; // --------------------------------------------------------------------------- // Public entry point @@ -350,9 +351,11 @@ function convertRunNode(node: PmNode, editor: Editor): PhrasingContent[] { node.forEach((child) => { if (child.type.name === 'text') { const text = child.text ?? ''; - // Check for inline code via run properties (Courier New font) + // Check for inline code via run properties (Courier New font) or via + // the `textStyle` mark (see `isMonospaceSignal` — the two detectors + // are unified so both directions of the monospace signal stay in sync). const runProps = node.attrs.runProperties as Record | undefined; - if (isMonospaceRun(runProps)) { + if (isMonospaceSignal(runProps, child.marks)) { const code: InlineCode = { type: 'inlineCode', value: text }; result.push(wrapWithMarks(code, child.marks)); } else { @@ -365,11 +368,36 @@ function convertRunNode(node: PmNode, editor: Editor): PhrasingContent[] { return result; } +/** + * True when the direct `runProperties.fontFamily` attr is set to the + * monospace font used by `mdastToProseMirror.ts` for code blocks/spans. + */ function isMonospaceRun(runProps: Record | undefined): boolean { if (!runProps) return false; - const rFonts = runProps.rFonts as Record | undefined; - if (!rFonts) return false; - return rFonts.ascii === 'Courier New' || rFonts.hAnsi === 'Courier New'; + const fontFamily = runProps.fontFamily as Record | undefined; + if (!fontFamily) return false; + return fontFamily.ascii === MARKDOWN_MONOSPACE_FONT || fontFamily.hAnsi === MARKDOWN_MONOSPACE_FONT; +} + +/** + * True when the `textStyle` mark's `fontFamily` attr is set to the + * monospace font used by `mdastToProseMirror.ts` for code blocks/spans. + */ +function isMonospaceMark(marks: readonly Mark[] | undefined): boolean { + if (!marks) return false; + const textStyleMark = marks.find((mark) => mark.type.name === 'textStyle'); + return textStyleMark?.attrs?.fontFamily === MARKDOWN_MONOSPACE_FONT; +} + +/** + * Single shared "is this monospace/code text" predicate. Combines the two + * independent signals a run can carry — the direct `runProperties.fontFamily` + * attr (checked by `convertRunNode`) and the `textStyle` mark (checked by + * `applyMark`) — so both call sites agree on what counts as code, and any + * future change to the detection logic only needs to happen in one place. + */ +function isMonospaceSignal(runProps: Record | undefined, marks: readonly Mark[] | undefined): boolean { + return isMonospaceRun(runProps) || isMonospaceMark(marks); } // --------------------------------------------------------------------------- @@ -437,9 +465,11 @@ function applyMark(mark: Mark, child: PhrasingContent): PhrasingContent { children: [child], } as Link; case 'textStyle': { - // Courier New fontFamily → inlineCode (only if child is text) + // Monospace fontFamily → inlineCode (only if child is text). Uses the + // same `isMonospaceMark` predicate as `convertRunNode` via the shared + // `MARKDOWN_MONOSPACE_FONT` constant (see `isMonospaceSignal`). const fontFamily = mark.attrs.fontFamily as string | undefined; - if (fontFamily === 'Courier New' && child.type === 'text') { + if (fontFamily === MARKDOWN_MONOSPACE_FONT && child.type === 'text') { return { type: 'inlineCode', value: (child as Text).value } as InlineCode; } return child; diff --git a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js index 2abc8885e6..9f896a9bca 100644 --- a/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js +++ b/packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.test.js @@ -92,6 +92,11 @@ const makeSchema = () => toDOM: () => ['em', 0], parseDOM: [{ tag: 'em' }], }, + textStyle: { + attrs: { fontFamily: { default: null } }, + toDOM: (mark) => ['span', { style: mark.attrs.fontFamily ? `font-family: ${mark.attrs.fontFamily}` : '' }, 0], + parseDOM: [{ tag: 'span' }], + }, }, }); @@ -782,12 +787,68 @@ describe('calculateInlineRunPropertiesPlugin', () => { expect(runNode?.attrs.runProperties).toEqual({ rsidR: 'r1' }); }); + // Regression coverage for the markdown code-block import scenario (see + // mdastToProseMirror.ts `convertCodeBlock`): a run is seeded with BOTH a + // `textStyle` mark carrying `fontFamily: 'Courier New'` AND a matching direct + // `runProperties.fontFamily` (4-key shape). No `sdPreserveRunPropertiesKeys` + // meta is set on the import transaction — this plugin's own appendTransaction + // must not drop the font on the very first doc-changing dispatch after import. + it('keeps mark-decoded fontFamily on a freshly-imported code-block-shaped run (no preserve meta set)', () => { + const codeFont = { ascii: 'Courier New', eastAsia: 'Courier New', hAnsi: 'Courier New', cs: 'Courier New' }; + decodeRPrFromMarksMock.mockImplementation((marks) => { + const textStyleMark = marks.find((mark) => mark.type.name === 'textStyle'); + if (textStyleMark?.attrs?.fontFamily === 'Courier New') { + return { fontFamily: { ...codeFont } }; + } + return {}; + }); + resolveRunPropertiesMock.mockImplementation(() => ({})); + // Encoder mirrors the real one closely enough for this scenario: it derives a + // concrete `attrs.fontFamily` value from whatever fontFamily object/string it is + // given, rather than returning a hardcoded constant. A constant-return mock would + // make the marks-derived and style-derived encodings always compare equal, hiding + // whether the plugin's mark-vs-style divergence gate behaves correctly. + encodeMarksFromRPrMock.mockImplementation((props) => { + const ff = props?.fontFamily; + if (!ff) return []; + const value = typeof ff === 'string' ? ff : ff.ascii; + if (!value) return []; + return [{ attrs: { fontFamily: value } }]; + }); + + const schema = makeSchema(); + const textStyleMark = schema.marks.textStyle.create({ fontFamily: 'Courier New' }); + const doc = paragraphDoc( + schema, + { + runProperties: { fontFamily: { ...codeFont } }, + runPropertiesInlineKeys: null, + runPropertiesStyleKeys: null, + runPropertiesOverrideKeys: null, + }, + [textStyleMark], + 'const x = 1;', + ); + const state = createState(schema, doc); + + // Simulate the first doc-changing transaction after import (e.g. a benign no-op + // insert/delete elsewhere triggering appendTransaction), mirroring the fact that + // the markdown importer sets no `sdPreserveRunPropertiesKeys` meta at all. + const textPos = runPos(state.doc) + 1; + const tr = state.tr.insertText('X', textPos).delete(textPos, textPos + 1); + const { state: nextState } = state.applyTransaction(tr); + + const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0); + expect(runNode?.attrs.runProperties?.fontFamily).toEqual(codeFont); + }); + it('maps changed ranges through later transactions', () => { const schema = makeSchema(); const doc = schema.node('doc', null, [ schema.node('paragraph', null, [schema.node('run', null, schema.text('AAA'))]), schema.node('paragraph', null, [schema.node('run', null, schema.text('Hello'))]), ]); + const state = createState(schema, doc); const [firstRunPos, secondRunPos] = runPositions(state.doc); const { from, to } = runTextRangeAtPos(secondRunPos, 0, 2); // "He"