Skip to content

Commit d33522b

Browse files
authored
fix: preserve Markdown formatting and allow source-mode repairs (#1255 fixes #1254)
2 parents 748022d + 281d126 commit d33522b

6 files changed

Lines changed: 137 additions & 14 deletions

File tree

‎docs/specs/dor-tools-builtin.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ Source of truth: `saveText` / `StorePath` in `dor-tools-builtin/src/editor-route
4444
`.md` and `.markdown` open in the bundled MDXEditor page; `.mdx` and other text stay in Monaco. Saving, reloading, and dirty reports follow [Editing files](#editing-files).
4545

4646
- **Must resolve the viewer's direct `lexical` and `@lexical/link` imports to the same installed modules MDXEditor uses** (rationale).
47-
- **Must support GFM tables, task lists, frontmatter, code blocks, fenced `mermaid` diagrams, and a whole-document source mode**, which opens with the error when the rich editor cannot parse a file. Mermaid stays on 11 (rationale).
48-
- **Must keep HTML comments verbatim and save with one final newline, the file's majority bullet and thematic-break marker, and unpadded tables**; the rest follows MDXEditor's serialization. Its normalization of loaded text is not an edit (rationale).
47+
- **Must support GFM tables, task lists, frontmatter, code blocks, fenced `mermaid` diagrams, and a whole-document source mode**, which opens with the error when the rich editor cannot parse a file. **Must let source edits remain dirty and saveable after a parse error, and allow repaired source back into rich text.** Mermaid stays on 11 (rationale).
48+
- **Must keep HTML comments verbatim and save with one final newline, the file's majority bullet and thematic-break marker, and unpadded tables**. **Must retain simple standalone bracket labels, encode literal angle brackets in text as entities, and preserve recognized headings' existing blank-line choice**, with new or renamed headings following the majority style; ambiguous Markdown keeps the serializer's escapes. The rest follows MDXEditor's serialization. Its normalization of loaded text is not an edit (rationale).
4949
- **Never let document HTML reach the live DOM outside an allowlist** of tags and attributes, `href` limited to `http(s):`, `mailto:`, fragments, and scheme-less paths; saving keeps every original attribute, escaped, and HTML images serialize in an inert document. Mermaid diagrams are the one other path, under mermaid's own sanitizer (`docs/specs/security-local.md` → "Local-file viewer"; rationale).
5050
- **Must serve images on request from regular image-format files at or under the document's canonical directory**, by realpath, under a `sandbox` CSP. Nothing else loads, external URLs included.
5151
- **Never show an image at a size the document does not state**: unsized, it shows pixel for pixel, within the column. A pasted or dropped PNG or JPEG with a recorded scale goes in at its pixel width over that scale (rationale).

‎docs/specs/dor-tools-builtin.rationale.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ MDXEditor 4.3.3 depends on Lexical `^0.48.0`; independent direct bumps to 0.52 i
2222

2323
MDXEditor 4.3 parses `<!-- … -->` and discards it, and trims and re-serializes the whole document on export; without the comment node and style detection, saving a typical README after a one-word edit deleted its comments and rewrote every `-` bullet as `*` (measured 2026-10).
2424

25+
Issue #1254 (2026-10) exposed unconditional bracket escaping, literal angle brackets changing spelling across rich/source imports, and blank lines inserted after every heading. Serializer options keep simple labels and per-heading spacing without retaining a parallel source tree, while entity-encoding text brackets survives MDX JSX parsing. Code and HTML still use their own serializers. MDXEditor also leaves its initial-normalization flag set after a failed import; treating source changes as edits after that failure permits repair and save.
26+
2527
Image sizes live in the document because GitHub shows an `<img>` without a `width` pixel for pixel: a Retina screenshot shown here at its recorded scale but left unsized in the file came out twice as large on GitHub. Writing the width on paste keeps the two renderings alike (2026-10).
2628

2729
Image URLs carry a per-load generation because the browser keeps decoded images by URL: a reload that only remounted the editor still showed an image's old pixels after the file changed on disk (observed live, 2026-10).
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import assert from 'node:assert/strict';
2+
import { createRequire } from 'node:module';
3+
import { test } from 'node:test';
4+
import { load } from './bundle.mjs';
5+
6+
// Exercise the exact parser/serializer versions used by the installed editor.
7+
const viewer = createRequire(new URL('../viewer/markdown.tsx', import.meta.url));
8+
const editor = createRequire(viewer.resolve('@mdxeditor/editor'));
9+
const dependency = name => import(editor.resolve(name));
10+
const { fromMarkdown } = await dependency('mdast-util-from-markdown');
11+
const { toMarkdown } = await dependency('mdast-util-to-markdown');
12+
const { mdxJsx } = await dependency('micromark-extension-mdx-jsx');
13+
const { mdxJsxFromMarkdown, mdxJsxToMarkdown } = await dependency('mdast-util-mdx-jsx');
14+
const { markdownFormat } = await load('../viewer/markdown-format.ts');
15+
const parse = text => fromMarkdown(text, { extensions: [mdxJsx()], mdastExtensions: [mdxJsxFromMarkdown()] });
16+
const serialize = (tree, original) => toMarkdown(tree, { extensions: [mdxJsxToMarkdown()], ...markdownFormat(original) });
17+
const roundtrip = text => serialize(parse(text), text);
18+
const semantic = tree => JSON.parse(JSON.stringify(tree, (key, value) => key === 'position' ? undefined : value));
19+
20+
for (const source of [
21+
'## [Unreleased]\n\n## [1.3.0] - 2026-10-20\n',
22+
'Use &lt;your relay&gt; here.\n',
23+
'## Tight\nbody\n\n## Loose\n\nbody\n',
24+
'## First\n## Second\nbody\n',
25+
]) {
26+
test(`preserves issue 1254 formatting: ${JSON.stringify(source)}`, () => {
27+
assert.equal(roundtrip(source), source);
28+
});
29+
}
30+
31+
test('a real edit keeps surrounding labels, entities, and heading spacing', () => {
32+
const source = '## [Unreleased]\nold &lt;your relay&gt;\n\n## [1.3.0] - 2026-10-20\n\nprevious\n';
33+
const tree = parse(source);
34+
tree.children[1].children[0].value = 'new <your relay>';
35+
const saved = serialize(tree, source);
36+
assert.equal(saved, source.replace('old', 'new'));
37+
assert.deepEqual(semantic(parse(saved)), semantic(tree));
38+
});
39+
40+
for (const source of [
41+
'\\[literal](url)\n',
42+
'\\[literal][ref]\n\n[ref]: /target\n',
43+
'\\[literal]\n\n[literal]: /target\n',
44+
'\\[literal]: /target\n',
45+
'[link \\[label\\]](/target)\n',
46+
'- \\[x] plain list text\n',
47+
'\\[outer \\[inner] end]\n',
48+
'\\\\[path]\n',
49+
'![image](image.png)\n',
50+
'`<your relay> [literal]`\n',
51+
'```md\n## Example\nbody [literal] <your relay>\n```\n',
52+
'<span title="<literal>">safe</span>\n',
53+
'Use \\<your relay> and &lt;https://example.com&gt;.\n',
54+
'\\<script>alert(1)\\</script>\n',
55+
'Write \\&#x3C; and \\&#x3E; literally, but \\\\&lt;.\n',
56+
'A \\[label] **bold** and \\[label]\\(url).\n',
57+
]) {
58+
test(`retains semantics: ${JSON.stringify(source)}`, () => {
59+
const before = parse(source);
60+
const saved = serialize(before, source);
61+
assert.deepEqual(semantic(parse(saved)), semantic(before));
62+
assert.equal(roundtrip(saved), saved);
63+
});
64+
}
65+
66+
test('new headings follow the majority spacing, ignoring fenced examples', () => {
67+
const source = '## One\ntext\n\n```md\n## Example\n\ntext\n```\n';
68+
assert.equal(serialize(parse('## New\n\ntext\n'), source), '## New\ntext\n');
69+
});
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import type { MDXEditorProps } from '@mdxeditor/editor';
2+
3+
type Options = NonNullable<MDXEditorProps['toMarkdownOptions']>;
4+
5+
/** Keep the document's inexpensive style choices without retaining a second
6+
* editable syntax tree. Ambiguous Markdown still uses the stock serializer. */
7+
export function markdownFormat(markdown: string): Options {
8+
const prose = markdown.replace(/^(`{3,}|~{3,})[^]*?^\1[^\n]*$/gm, '');
9+
const count = (pattern: RegExp) => prose.match(pattern)?.length ?? 0;
10+
const bullets = (['-', '*', '+'] as const).map(b => [b, count(new RegExp(`^[ \\t]*\\${b}[ \\t]+\\S`, 'gm'))] as const);
11+
const bullet = bullets.reduce((best, next) => next[1] > best[1] ? next : best)[0];
12+
// Per-heading choices preserve mixed files too. Newly added/renamed headings
13+
// use the majority; ties retain the serializer's blank line.
14+
const headings = new Map<string, number>();
15+
let tight = 0, loose = 0;
16+
for (const match of prose.matchAll(/^ {0,3}(#{1,6})[ \t]+([^\n]+)\n(?=([^\n]*))/gm)) {
17+
const gap = match[3].trim() ? 0 : 1;
18+
headings.set(`${match[1].length}:${match[2].trim()}`, gap);
19+
if (gap) loose++; else tight++;
20+
}
21+
let references = false;
22+
return {
23+
bullet,
24+
rule: count(/^\*{3,}[ \t]*$/gm) > count(/^-{3,}[ \t]*$/gm) ? '*' : '-',
25+
// A literal angle bracket must not turn into JSX when source mode imports
26+
// the saved text again. Only text nodes use these entity escapes.
27+
unsafe: [{ character: '<', inConstruct: 'phrasing' }, { character: '>', inConstruct: 'phrasing' }],
28+
handlers: {
29+
root(node, _parent, state, info) {
30+
references = hasDefinition(node);
31+
return state.containerFlow(node, info);
32+
},
33+
text(node, _parent, state, info) {
34+
const safe = state.safe(node.value, { ...info, encode: ['<', '>'] })
35+
.replace(/\\[^]|&#x3([CE]);/g, (match, hex: string | undefined) => hex ? (hex === 'C' ? '&lt;' : '&gt;') : match);
36+
// Restore only simple, complete labels. Keep escapes inside link labels,
37+
// list items (task syntax), and documents with reference definitions.
38+
if (references || state.stack.some(kind => ['label', 'reference', 'listItem'].includes(kind))) return safe;
39+
return safe.replace(/\\(?:\\|\[([\w .-]+)\])/g, (escaped, label: string | undefined, offset: number) => {
40+
if (!label) return escaped; // A literal backslash is not an escape to remove.
41+
const after = safe.slice(offset + escaped.length) || info.after;
42+
if (/^\s*[\[(:\]]/.test(after) || safe[offset - 1] === '!') return escaped;
43+
return `[${label}]`;
44+
});
45+
},
46+
},
47+
join: [left => {
48+
if (left.type !== 'heading') return;
49+
const text = left.children.map(child => 'value' in child ? child.value : '').join('');
50+
return headings.get(`${left.depth}:${text}`) ?? (tight > loose ? 0 : 1);
51+
}],
52+
};
53+
}
54+
55+
function hasDefinition(node: { type: string; children?: { type: string }[] }): boolean {
56+
return node.type === 'definition' || !!node.children?.some(hasDefinition);
57+
}

‎dor-tools-builtin/viewer/markdown.tsx‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { classHighlighter } from '@lezer/highlight';
77
import {
88
$isImageNode, createActiveEditorSubscription$, editImageToolbarComponent$, iconComponentFor$, ImageNode, INSERT_IMAGE_COMMAND, useCellValue,
99
BlockTypeSelect, BoldItalicUnderlineToggles, codeBlockPlugin, codeMirrorPlugin, CodeToggle, CreateLink, diffSourcePlugin,
10-
DiffSourceToggleWrapper, frontmatterPlugin, headingsPlugin, imagePlugin, InsertCodeBlock, InsertTable, InsertThematicBreak,
10+
DiffSourceToggleWrapper, frontmatterPlugin, headingsPlugin, imagePlugin, initialMarkdownNormalize$, InsertCodeBlock, InsertTable, InsertThematicBreak,
1111
linkDialogPlugin, linkDialogState$, openLinkEditDialog$, linkPlugin, listsPlugin, ListsToggle, markdownShortcutPlugin, MDXEditor, quotePlugin, realmPlugin,
1212
rootEditor$, Separator, viewMode$, tablePlugin, thematicBreakPlugin, toolbarPlugin, UndoRedo, type MDXEditorMethods, type MDXEditorProps, type Realm,
1313
} from '@mdxeditor/editor';
@@ -16,6 +16,7 @@ import { checked, documentSession, loadingState, request, stateLabel, textSource
1616
import { editableImage, handOffImageEdits, imageSources, imageUrl, ImagesPanel, localImagePath, pathRoute } from './images-panel';
1717
import { ImageResizeHandles } from './image-resize';
1818
import { commentsPlugin } from './markdown-comments';
19+
import { markdownFormat } from './markdown-format';
1920
import { markdownSafetyPlugin } from './markdown-safety';
2021
import { pageTheme, subscribeTheme } from './page-theme';
2122
import { mermaidDescriptor } from './mermaid-block';
@@ -246,26 +247,20 @@ const plugins = [
246247
toolbarPlugin({ toolbarContents: Toolbar }),
247248
];
248249

249-
/** The list bullet and thematic break the file uses most, so a save keeps its style. */
250-
function markdownStyle(markdown: string) {
251-
const prose = markdown.replace(/^(```|~~~)[^]*?^\1/gm, '');
252-
const count = (pattern: RegExp) => prose.match(pattern)?.length ?? 0;
253-
const bullets = (['-', '*', '+'] as const).map(b => [b, count(new RegExp(`^[ \\t]*\\${b}[ \\t]+\\S`, 'gm'))] as const);
254-
const bullet = bullets.reduce((best, next) => next[1] > best[1] ? next : best)[0];
255-
return { bullet, rule: count(/^\*{3,}[ \t]*$/gm) > count(/^-{3,}[ \t]*$/gm) ? '*' as const : '-' as const };
256-
}
257-
258250
// The editor's own reformatting of loaded text is not an edit.
259251
const onEditorChange: MDXEditorProps['onChange'] = (_, normalize) => { if (normalize) session.rebase(); else session.changed(); };
260252
const onEditorError: MDXEditorProps['onError'] = ({ error: reason }) => {
253+
// MDXEditor clears its normalize flag only in a rich-text export, which a
254+
// parse error suppresses, so every later source edit would arrive as normalization.
255+
realm?.pub(initialMarkdownNormalize$, false);
261256
realm?.pub(viewMode$, 'source');
262257
session.fail(`Showing source: the rich editor cannot parse this file (${reason}).`);
263258
};
264259

265260
/** The rich editor; it follows only the theme, so page state changes leave it be. */
266261
function Editor({ markdown }: { markdown: string }) {
267262
const theme = useSyncExternalStore(subscribeTheme, pageTheme);
268-
const style = useMemo(() => markdownStyle(markdown), [markdown]);
263+
const style = useMemo(() => markdownFormat(markdown), [markdown]);
269264
return (
270265
<MDXEditor ref={editorRef} markdown={markdown} plugins={plugins} toMarkdownOptions={style}
271266
className={theme.dark ? 'dark-theme' : 'light-theme'} contentEditableClassName="prose" spellCheck

‎scripts/spec-word-budgets.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"docs/specs/dor-browser.md": 5800,
1010
"docs/specs/dor-cli.md": 5150,
1111
"docs/specs/dor-tool.md": 6550,
12-
"docs/specs/dor-tools-builtin.md": 1700,
12+
"docs/specs/dor-tools-builtin.md": 1750,
1313
"docs/specs/dor-tools-lib.md": 350,
1414
"docs/specs/glossary.md": 2950,
1515
"docs/specs/hosted.md": 4450,

0 commit comments

Comments
 (0)