From a3615d00ce6f54e1b055f13a3954df09243b10ba Mon Sep 17 00:00:00 2001 From: Ricardo Costeira Date: Thu, 29 Oct 2026 18:47:58 +0100 Subject: [PATCH] Honor Markdown preview layouts and highlight precise changes --- public/app.js | 105 ++++++++++++++++++++++++--- public/monaco/app.mjs | 27 +++++-- public/monaco/styles.css | 2 + public/styles.css | 2 + scripts/check-monaco.ts | 2 +- scripts/markdown-preview-browser.mjs | 88 ++++++++++++++++++++++ scripts/monaco-fixture.ts | 6 ++ tests/ui-markdown-preview.test.ts | 2 +- tests/ui-markdown-unified.test.ts | 70 ++++++++++++++++++ 9 files changed, 285 insertions(+), 19 deletions(-) create mode 100644 scripts/markdown-preview-browser.mjs create mode 100644 tests/ui-markdown-unified.test.ts diff --git a/public/app.js b/public/app.js index 4ca25d9..697efb8 100644 --- a/public/app.js +++ b/public/app.js @@ -329,10 +329,16 @@ function markdownCodeBlock(fence) { return `
${escapeHtml(fence.lines.join("\n"))}
`; } -function markdownSourceBlock(html, startLine, endLine, source) { +function markdownSourceBlock(html, startLine, endLine, source, highlight = true, tag = "section") { if (!source) return html; const changed = [...source.changedLines].some((line) => line >= startLine && line <= endLine); - return `
${html}
`; + source.blocks?.push({ html, startLine, endLine, changed, highlight }); + return `<${tag} class="markdown-source-block${changed && highlight ? ` ${source.side === "old" ? "deletion" : "addition"}` : ""}" data-markdown-path="${escapeHtml(source.path)}" data-markdown-side="${source.side}" data-source-start="${startLine}" data-source-end="${endLine}">${html}`; +} + +function markdownItemClass(startLine, endLine, source) { + const changed = source && [...source.changedLines].some(line => line >= startLine && line <= endLine); + return changed ? ` class="markdown-source-item ${source.side === "old" ? "deletion" : "addition"}"` : ""; } function markdownInlineSource(value, line, source, sourceLine) { @@ -348,7 +354,15 @@ function renderMarkdown(value, headingOffset = 1, source = null) { let listStart = null; let fence = null; const closeList = () => { - if (listType) output.push(markdownSourceBlock(`<${listType}>${listItems.map((item) => item.html).join("")}`, listStart, listItems.at(-1)?.line ?? listStart, source)); + if (listType) { + // Match individual items in Unified so editing one item does not repeat + // the retained list. Split keeps its list wrapper and highlights items. + for (const item of listItems) source?.blocks?.push({ + html: item.html, startLine: item.line, endLine: item.endLine, listType, highlight: true, + changed: [...source.changedLines].some(line => line >= item.line && line <= item.endLine), + }); + output.push(markdownSourceBlock(`<${listType}>${listItems.map(item => `${item.html}`).join("")}`, listStart, listItems.at(-1)?.endLine ?? listStart, source ? { ...source, blocks: undefined } : null, false)); + } listType = null; listItems = []; listStart = null; @@ -385,7 +399,7 @@ function renderMarkdown(value, headingOffset = 1, source = null) { index += 1; } index -= 1; - output.push(markdownSourceBlock(`
${headers.map((header, cellIndex) => ``).join("")}${tableRows.map((row) => `${row.cells.map((cell, cellIndex) => ``).join("")}`).join("")}
${markdownInlineSource(header, startLine, source, line)}
${markdownInlineSource(cell, row.line, source, row.sourceLine)}
`, startLine, index + 1, source)); + output.push(markdownSourceBlock(`
${headers.map((header, cellIndex) => ``).join("")}${tableRows.map((row) => `${row.cells.map((cell, cellIndex) => ``).join("")}`).join("")}
${markdownInlineSource(header, startLine, source, line)}
${markdownInlineSource(cell, row.line, source, row.sourceLine)}
`, startLine, index + 1, source, false)); continue; } const heading = /^\s*(#{1,6})\s+(.+?)\s*$/.exec(line); @@ -403,7 +417,13 @@ function renderMarkdown(value, headingOffset = 1, source = null) { listType = nextListType; listStart = index + 1; } - listItems.push({ line: index + 1, html: `
  • ${markdownInlineSource(item[2], index + 1, source, line)}
    • ` }); + listItems.push({ line: index + 1, endLine: index + 1, html: markdownInlineSource(item[2], index + 1, source, line) }); + continue; + } + if (listType && line.trim()) { + const previous = listItems.at(-1); + previous.html += ` ${markdownInlineSource(line.trim(), index + 1, source, line)}`; + previous.endLine = index + 1; continue; } closeList(); @@ -1818,17 +1838,68 @@ function markdownChangedRanges(file, side) { } function renderMarkdownDiff(file) { - const sides = markdownSides(file); + const sides = state.diffLayout === "unified" && markdownSides(file).length === 2 ? [["unified", "Changes"]] : markdownSides(file); return `
    ${sides.map(([side, label]) => `
    ${label}

    Loading Markdown preview…

    `).join("")}
    `; } +function renderUnifiedMarkdownDiff(file, before, after) { + const blocks = { old: [], new: [] }; + const changedLines = { old: markdownChangedLines(file, "old"), new: markdownChangedLines(file, "new") }; + for (const [side, content] of [["old", before], ["new", after]]) { + renderMarkdown(content, 0, { path: file.path, side, blocks: blocks[side], changedLines: changedLines[side], changedRanges: markdownChangedRanges(file, side) }); + } + // Removing the changed lines from each captured source leaves the same + // unchanged sequence, including context outside the diff's visible hunks. + const oldLines = before.split("\n"), newLines = after.split("\n"); + const correspondingLines = new Map(); + let oldLine = 1, newLine = 1; + while (oldLine <= oldLines.length && newLine <= newLines.length) { + if (changedLines.old.has(oldLine)) { oldLine++; continue; } + if (changedLines.new.has(newLine)) { newLine++; continue; } + if (oldLines[oldLine - 1] === newLines[newLine - 1]) correspondingLines.set(oldLine, newLine); + oldLine++; newLine++; + } + const newBlocks = new Map(blocks.new.map((block, index) => [block.startLine, { block, index }])); + const output = []; + let listType; + const append = (block, side, changed) => { + if (listType !== block.listType) { + if (listType) output.push(``); + listType = block.listType; + if (listType) output.push(`<${listType}>`); + } + output.push(markdownSourceBlock(block.html, block.startLine, block.endLine, { + path: file.path, side, changedLines: new Set(changed && block.changed ? [block.startLine] : []), + }, block.highlight, listType ? "li" : "section")); + }; + let oldIndex = 0, newIndex = 0; + for (let index = 0; index < blocks.old.length; index++) { + const oldBlock = blocks.old[index]; + const match = newBlocks.get(correspondingLines.get(oldBlock.startLine)); + if (oldBlock.changed || !match || match.index < newIndex || match.block.changed + || correspondingLines.get(oldBlock.endLine) !== match.block.endLine || oldBlock.html !== match.block.html || oldBlock.listType !== match.block.listType) continue; + // Keep tables and fences whole. Unmatched blocks are replaced as a + // unit, so deleting a delimiter cannot produce broken Markdown fragments. + while (oldIndex < index) append(blocks.old[oldIndex++], "old", true); + while (newIndex < match.index) append(blocks.new[newIndex++], "new", true); + append(match.block, "new", false); + oldIndex = index + 1; newIndex = match.index + 1; + } + while (oldIndex < blocks.old.length) append(blocks.old[oldIndex++], "old", true); + while (newIndex < blocks.new.length) append(blocks.new[newIndex++], "new", true); + if (listType) output.push(``); + return output.join(""); +} + async function hydrateMarkdownPreviews(files) { const requestId = ++state.markdownPreviewRequestId; const revision = state.session.revision; const previews = files.filter((file) => isMarkdownFile(file) && state.markdownPreviewPaths.has(file.path)); for (const file of previews) { + const unified = elements.diff.querySelector(`article[data-markdown-path="${CSS.escape(file.path)}"][data-markdown-side="unified"]`); + const contents = {}; for (const [side] of markdownSides(file)) { - const target = elements.diff.querySelector(`[data-markdown-path="${CSS.escape(file.path)}"][data-markdown-side="${side}"]`); + const target = unified ?? elements.diff.querySelector(`article[data-markdown-path="${CSS.escape(file.path)}"][data-markdown-side="${side}"]`); if (!target) continue; const key = `${revision}:${file.path}:${side}`; try { @@ -1839,11 +1910,15 @@ async function hydrateMarkdownPreviews(files) { content = result.content; state.markdownPreviewCache.set(key, content); } - target.innerHTML = renderMarkdown(content, 0, { path: file.path, side, changedLines: markdownChangedLines(file, side), changedRanges: markdownChangedRanges(file, side) }) || '

    This Markdown file is empty.

    '; + contents[side] = content; + if (!unified) target.innerHTML = renderMarkdown(content, 0, { path: file.path, side, changedLines: markdownChangedLines(file, side), changedRanges: markdownChangedRanges(file, side) }) || '

    This Markdown file is empty.

    '; } catch (error) { if (requestId === state.markdownPreviewRequestId && target.isConnected) target.innerHTML = `

    ${escapeHtml(error.message)}

    `; } } + if (unified?.isConnected && requestId === state.markdownPreviewRequestId && contents.old !== undefined && contents.new !== undefined) { + unified.innerHTML = renderUnifiedMarkdownDiff(file, contents.old, contents.new) || '

    This Markdown file is empty.

    '; + } } if (requestId === state.markdownPreviewRequestId) renderTourAnnotation(); } @@ -5639,7 +5714,14 @@ function renderSession() { ? `${comparison.base} → since last review` : `${comparison.base} → working tree`; elements["comparison-view"].value = state.session.comparisonView ?? "full"; - elements["comparison-view"].disabled = !state.session.checkpoints?.length || Boolean(state.tourMode); + elements["comparison-view"].disabled = state.busy; + const comparisonRangeReason = state.tourMode ? "Exit the tour to compare changes since your last review." + : !state.session.checkpoints?.length ? "Create a review checkpoint to compare later changes against it." : ""; + const sinceLastReview = elements["comparison-view"].querySelector('[value="since-last-review"]'); + sinceLastReview.disabled = Boolean(comparisonRangeReason); + sinceLastReview.textContent = state.tourMode ? "Since last review (exit tour first)" + : !state.session.checkpoints?.length ? "Since last review (create checkpoint first)" : "Since last review"; + elements["comparison-view"].title = comparisonRangeReason || "Choose which changes to review."; elements["revision-summary"].textContent = state.session.revisionSummary?.text ?? ""; elements["checkpoint-history"].innerHTML = (state.session.checkpoints ?? []).slice().reverse().map((checkpoint, index) => { const title = checkpoint.label ?? (checkpoint.kind === "decision" ? (checkpoint.decision ?? checkpoint.outcome ?? "Decision") : "Manual checkpoint"); @@ -8356,6 +8438,11 @@ state.monacoRenderer = createMonacoRenderer({ if (!file || !["old", "new"].includes(side)) throw Error("Captured Markdown source is unavailable."); return renderMarkdown(content, 0, { path, side, changedLines: markdownChangedLines(file, side), changedRanges: markdownChangedRanges(file, side) }); }, + renderUnifiedMarkdownPreview(path, before, after) { + const file = state.session.comparison.files.find(file => file.path === path); + if (!file) throw Error("Captured Markdown source is unavailable."); + return renderUnifiedMarkdownDiff(file, before, after); + }, }, }); diff --git a/public/monaco/app.mjs b/public/monaco/app.mjs index faec92f..98741f6 100644 --- a/public/monaco/app.mjs +++ b/public/monaco/app.mjs @@ -335,6 +335,7 @@ const mobileDecorations = {}; let annotationSignature = null; let sourceHadAnchor = false, copyPending = false; let markdownPane = null, markdownPreviewButton = null; +let syncMarkdownPreviewLayout = () => {}; const markdownPreviewOpen = () => Boolean(markdownPane && !markdownPane.hidden); function closeMarkdownPreview(restoreFocus = false) { if (!markdownPreviewOpen()) return; @@ -1299,14 +1300,23 @@ if (host && selectedFile.language === "markdown") { // Status is Git's, not guessed from empty content. Existing empty files // must retain their Before side, and added files must not invent one. const sides = selectedFile.status === "added" ? ["new"] : selectedFile.status === "deleted" ? ["old"] : ["old", "new"]; - for (const side of sides) { - const column = document.createElement("section"); column.dataset.markdownSide = side; - const label = document.createElement("h2"); label.textContent = side === "old" ? "Before" : "After"; - const content = document.createElement("div"); content.className = "review-rendered-markdown"; - content.innerHTML = host.renderMarkdownPreview(selectedFile.path, side, selectedFile[side]); - if (!selectedFile[side]) content.textContent = "Empty file"; - column.append(label, content); columns.append(column); - } + syncMarkdownPreviewLayout = () => { + if (columns.dataset.layout === activeLayout) return; + columns.dataset.layout = activeLayout; + columns.replaceChildren(); + for (const side of activeLayout === "unified" && sides.length === 2 ? ["unified"] : sides) { + const column = document.createElement("section"); column.dataset.markdownSide = side; + const label = document.createElement("h2"); label.textContent = side === "unified" ? "Changes" : side === "old" ? "Before" : "After"; + const content = document.createElement("div"); content.className = "review-rendered-markdown"; + content.innerHTML = side === "unified" + ? host.renderUnifiedMarkdownPreview(selectedFile.path, selectedFile.old, selectedFile.new) + : host.renderMarkdownPreview(selectedFile.path, side, selectedFile[side]); + if (!content.innerHTML) content.textContent = "Empty file"; + column.append(label, content); columns.append(column); + } + scheduleDetailHeight(); + }; + syncMarkdownPreviewLayout(); markdownPane.append(heading, columns); fileDetail.append(markdownPane); markdownPreviewButton = button("Preview", () => { if (composer) { fileToolStatus("Save or cancel the draft before opening Markdown preview."); return; } @@ -1437,6 +1447,7 @@ return { focusTour(id) { closeMarkdownPreview(); if (!$("source-pane").hidden) $("return").click(); return tourAnnotations.focus(id); }, layout() { host?.clearMobileSelection?.(); + syncMarkdownPreviewLayout(); $("editor").classList.toggle("review-split", activeLayout === "split"); deletedSelection.reset(); currentAnchor = null; currentSourceRange = null; selectionError = ""; updateFileTools(); diff --git a/public/monaco/styles.css b/public/monaco/styles.css index e3b4f88..c06e594 100644 --- a/public/monaco/styles.css +++ b/public/monaco/styles.css @@ -121,6 +121,8 @@ .review-rendered-markdown .markdown-source-block{border-left:3px solid transparent;padding:1px 10px} .review-rendered-markdown .markdown-source-block.addition{border-left-color:#9acb9c;background:#90bb7320} .review-rendered-markdown .markdown-source-block.deletion{border-left-color:#dc8686;background:#ef6e6820} +.review-rendered-markdown .markdown-source-item.addition{background:#90bb7320;box-shadow:inset 3px 0 #9acb9c} +.review-rendered-markdown .markdown-source-item.deletion{background:#ef6e6820;box-shadow:inset 3px 0 #dc8686} .review-rendered-markdown .addition .inline-change{background:#8bad6155}.review-rendered-markdown .deletion .inline-change{background:#d4767255} @media(max-width:700px){.review-markdown-columns{display:block}.review-markdown-columns>section+section{border-left:0;border-top:1px solid var(--border)}} :root[data-hide-markers] .review-change-gutter > * { visibility: hidden; } diff --git a/public/styles.css b/public/styles.css index 29325b3..083b141 100644 --- a/public/styles.css +++ b/public/styles.css @@ -693,6 +693,8 @@ kbd { display: inline-block; min-width: 1.5em; border: 1px solid var(--border-ho .markdown-source-block { position: relative; margin: 0 -10px; border-left: 3px solid transparent; padding: 1px 10px; } .markdown-source-block.addition { border-left-color: var(--addition-line); background: var(--addition); } .markdown-source-block.deletion { border-left-color: var(--deletion-line); background: var(--deletion); } +.markdown-source-item.addition { background: var(--addition); box-shadow: inset 3px 0 var(--addition-line); } +.markdown-source-item.deletion { background: var(--deletion); box-shadow: inset 3px 0 var(--deletion-line); } .markdown-source-block.tour-anchor { border-left-color: var(--accent-strong); background-image: linear-gradient(90deg, var(--tour-highlight), var(--accent-soft)); box-shadow: inset -2px 0 var(--accent-border); filter: saturate(1.18) brightness(1.08); } .markdown-source-block.tour-anchor-start { border-top: 2px solid var(--accent-strong); } .markdown-source-block.tour-anchor-end { border-bottom: 2px solid var(--accent-strong); } diff --git a/scripts/check-monaco.ts b/scripts/check-monaco.ts index c88bc9b..44a3c7c 100644 --- a/scripts/check-monaco.ts +++ b/scripts/check-monaco.ts @@ -23,7 +23,7 @@ try { } const groups = { briefing: { samplePr: false, scripts: ["briefing-entry", "briefing-source", "briefing", "briefing-code", "briefing-diagrams"] }, - local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "monaco-gutter", "monaco-wrapping", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] }, + local: { samplePr: false, scripts: ["monaco-foundation", "monaco-scroll", "file-header", "monaco-edits", "monaco-edits-syntax", "monaco-themes", "monaco-gutter", "monaco-wrapping", "markdown-preview", "monaco-semantic", "monaco-mount", "monaco-paged-draft"] }, navigation: { samplePr: true, scripts: ["monaco-phase", "monaco-phase-scroll", "monaco-upward-tour", "monaco-reviewed-scroll", "monaco-sticky", "monaco-file-explorer"] }, discussions: { samplePr: true, scripts: ["renderer-comments", "monaco-context", "monaco-edits", "monaco-edits-syntax", "monaco-discussions"] }, before: { samplePr: true, beforeTour: true, scripts: ["monaco-before-tour"] }, diff --git a/scripts/markdown-preview-browser.mjs b/scripts/markdown-preview-browser.mjs new file mode 100644 index 0000000..91e5fc5 --- /dev/null +++ b/scripts/markdown-preview-browser.mjs @@ -0,0 +1,88 @@ +// Run against the disposable local scripts/monaco-fixture.ts review. +import assert from 'node:assert/strict'; +import { pathToFileURL } from 'node:url'; +const [playwrightPath, url] = process.argv.slice(2); +const { chromium } = await import(pathToFileURL(playwrightPath).href); +const browser = await chromium.launch({ channel: process.env.CI ? undefined : 'chrome', headless: true }); +const page = await browser.newPage({ viewport: { width: 1800, height: 1100 }, reducedMotion: 'reduce', colorScheme: 'dark' }); +page.setDefaultTimeout(20000); +const errors = []; +page.on('pageerror', error => errors.push(error.message)); +const path = 'docs/preview-layout.md'; +async function preview(renderer, path) { + await page.locator(`#file-tree .file-button[data-file="${path}"]`).click(); + if (renderer === 'current') { + const card = page.locator(`#diff > [data-path="${path}"]`); + const toggle = card.locator('[data-markdown-preview]'); + if (await toggle.textContent() === 'Preview') await toggle.click(); + await card.locator('.markdown-source-block').first().waitFor(); + return card.locator('.markdown-diff'); + } + await page.evaluate(path => diffCompassProductionHost.reveal(path, 'new', 1), path); + await page.waitForFunction(path => [...document.querySelectorAll('.monaco-review-frame')].some(node => node.dataset.monacoPath === path && node.contentWindow?.diffLab?.views.has(path)), path); + const frame = page.frames().find(frame => new URL(frame.url(), url).searchParams.get('file') === path); + await frame.locator('[data-control="preview"]').click(); + await frame.locator('[data-control="markdown-preview"]').waitFor(); + return frame.locator('.review-markdown-columns'); +} +try { + await page.route('**/api/update', route => route.fulfill({ json: { updateAvailable: false } })); + await page.goto(url); + await page.locator('#step-count').filter({ hasText: 'Step ' }).waitFor({ state: 'attached' }); + if (await page.locator('#tour-mode-toggle').textContent() === 'Exit tour') await page.locator('#tour-mode-toggle').click(); + await page.locator('#review-filter').selectOption(''); + const range = page.locator('#comparison-view'); + assert.equal(await range.isEnabled(), true, 'Comparison menu opens without a checkpoint'); + assert.equal(await range.locator('[value="since-last-review"]').evaluate(option => option.disabled), true); + assert.match(await range.locator('[value="since-last-review"]').textContent(), /create checkpoint first/); + await range.focus(); await page.keyboard.press('ArrowDown'); await page.keyboard.press('Escape'); + assert.equal(await range.inputValue(), 'full'); + for (const renderer of ['current', 'monaco']) { + await page.locator('#diff-renderer').selectOption(renderer); + await page.locator('[data-diff-layout="split"]').click(); + const content = await preview(renderer, path); + for (const layout of ['split', 'unified', 'split', 'unified']) { + await page.locator(`[data-diff-layout="${layout}"]`).click(); + await content.locator('.markdown-source-block').first().waitFor(); + const expected = layout === 'split' ? 2 : 1; + await page.waitForFunction(({ renderer, path, expected }) => { + const root = renderer === 'current' ? document.querySelector(`#diff > [data-path="${path}"] .markdown-diff`) + : [...document.querySelectorAll('.monaco-review-frame')].find(node => node.dataset.monacoPath === path)?.contentDocument.querySelector('.review-markdown-columns'); + return root?.children.length === expected && root.querySelectorAll('h1').length === expected; + }, { renderer, path, expected }); + assert.equal(await content.locator('h1').count(), expected, 'Unchanged heading appears once per column'); + assert.equal(await content.locator('li.deletion').count(), 1, 'Only the removed item is red'); + assert.equal(await content.locator('li.deletion').textContent(), 'Which workflow it follows. A team that already uses one can keep its practices.'); + assert.equal(await content.locator('li').filter({ hasText: 'Which harness to use.' }).count(), expected, 'Unified keeps unchanged list items once'); + assert.equal(await content.locator('.markdown-source-block.deletion ul').count(), 0, 'Shared list items never inherit a removal background'); + assert.equal(await content.locator('li.addition').count(), 0, 'Retained list items are not additions'); + assert.equal(await content.locator('.markdown-source-block.addition').count(), 1); + const columns = await content.locator(':scope > section').evaluateAll(nodes => nodes.map(node => ({ x: node.getBoundingClientRect().x, width: node.getBoundingClientRect().width }))); + if (layout === 'split') assert.ok(columns[1].x > columns[0].x + columns[0].width - 2, 'Split is side by side'); + else assert.ok(columns[0].width > 700, 'Unified uses the full content width'); + } + await page.screenshot({ path: `/tmp/dc-markdown-${renderer}-unified.png` }); + for (const otherPath of ['docs/preview.md', 'docs/emptied.md']) { + const other = await preview(renderer, otherPath); + assert.equal(await other.locator(':scope > section').count(), 1, 'Added or empty-sided preview opens directly in Unified'); + if (otherPath === 'docs/preview.md') assert.equal(await other.locator('[data-markdown-side="old"]').count(), 0, 'Added file never invents Before content'); + else assert.equal(await other.locator('.deletion').count(), 1, 'Empty After retains removed Before content'); + } + console.log(`PASS ${renderer}: live Split/Unified, precise list highlighting, added and emptied Markdown`); + } + // The range selector becomes useful once a real checkpoint exists. + await page.locator('#checkpoint-button').click(); + await page.locator('#checkpoint-label').fill('Markdown preview regression'); + await page.locator('#checkpoint-form button[type="submit"]').click(); + await page.waitForFunction(() => !document.querySelector('#comparison-view [value="since-last-review"]').disabled && !document.querySelector('#comparison-view').disabled); + await range.selectOption('since-last-review'); + await page.waitForFunction(() => document.getElementById('comparison-label').textContent.includes('since last review') && !document.getElementById('comparison-view').disabled); + assert.equal(await page.locator('#diff > [data-path]').count(), 0, 'Fresh checkpoint has no subsequent changes'); + await range.selectOption('full'); + await page.locator(`#diff > [data-path="${path}"]`).waitFor({ state: 'attached' }); + assert.deepEqual(errors, []); + console.log('PASS comparison range explains unavailable checkpoint and switches both ways after saving one'); +} catch (error) { + await page.screenshot({ path: '/tmp/dc-markdown-preview-failure.png' }); + throw error; +} finally { await browser.close(); } diff --git a/scripts/monaco-fixture.ts b/scripts/monaco-fixture.ts index 514aac4..b54238e 100644 --- a/scripts/monaco-fixture.ts +++ b/scripts/monaco-fixture.ts @@ -30,6 +30,12 @@ if (resumeFile) { "", "End of decisions.", "", ].join("\n"); extraFiles["docs/wrapping.md"] = { before: wrappingTable("Previous"), after: wrappingTable("Revised") }; + const previewList = "# Team choices\n\nKeep these choices.\n\n- Which harness to use.\n- How to split its work.\n"; + extraFiles["docs/preview-layout.md"] = { + before: previewList + "- Which workflow it follows. A team that already uses one\n can keep its practices.\n\n## Shared ending\n\nKeep the ending.\n", + after: previewList.replace("Keep these choices.", "Keep these choices.\n\nThe team agrees the change before writing code.") + "\n## Shared ending\n\nKeep the ending.\n", + }; + extraFiles["docs/emptied.md"] = { before: "# Removed contents\n", after: "" }; if (samplePr || beforeTour) extraFiles[contextFixtureFile.path] = contextFixtureFile; for (let i = 0; i < 12; i++) extraFiles[`src/lazy-${i}.ts`] = { before: "// Unchanged context\n".repeat(80) + "export const value = 1;\n", after: "// Unchanged context\n".repeat(80) + "export const value = 2;\n" }; extraFiles["src/whitespace.py"] = { before: "if ready:\n run()\n", after: "if ready:\n run()\n" }; diff --git a/tests/ui-markdown-preview.test.ts b/tests/ui-markdown-preview.test.ts index 6b8c6bc..8fbf64e 100644 --- a/tests/ui-markdown-preview.test.ts +++ b/tests/ui-markdown-preview.test.ts @@ -12,7 +12,7 @@ test("Markdown files expose rendered previews without replacing the diff permane expect(app).toContain("function markdownChangedLines(file, side)"); expect(app).toContain("for (const [side] of markdownSides(file))"); expect(app).toContain("changedRanges: markdownChangedRanges(file, side)"); - expect(app).toContain('class="markdown-source-block${changed ? ` ${source.side === "old" ? "deletion" : "addition"}` : ""}"'); + expect(app).toContain('class="markdown-source-block${changed && highlight ? ` ${source.side === "old" ? "deletion" : "addition"}` : ""}"'); expect(app).toContain('.markdown-source-block[data-markdown-path="${CSS.escape(path)}"]'); expect(app).toContain("function markdownTableAlignments(line, columns)"); expect(app).toContain("function markdownFenceStart(line)"); diff --git a/tests/ui-markdown-unified.test.ts b/tests/ui-markdown-unified.test.ts new file mode 100644 index 0000000..fec5dea --- /dev/null +++ b/tests/ui-markdown-unified.test.ts @@ -0,0 +1,70 @@ +import { expect, test } from "bun:test"; +import { readFile } from "node:fs/promises"; +import { parseUnifiedDiff } from "../src/diff.ts"; + +const app = await readFile(new URL("../public/app.js", import.meta.url), "utf8"); +const extract = (start: string, end: string) => app.slice(app.indexOf(start), app.indexOf(end)); +const escapeHtml = (value: unknown) => String(value).replaceAll("&", "&").replaceAll("<", "<").replaceAll(">", ">").replaceAll('"', """); +const { renderUnifiedMarkdownDiff } = new Function("escapeHtml", [ + extract("function renderInlineChangeText", "function pullRequestDescription"), + extract("function inlineChangedRanges", "function diffCodeHtml"), + extract("function splitRows", "function renderSplitSide"), + extract("function markdownSides", "async function hydrateMarkdownPreviews"), + "return { renderUnifiedMarkdownDiff };", +].join("\n"))(escapeHtml); + +function render(before: string, after: string, hunks: string) { + const [file] = parseUnifiedDiff(`diff --git a/guide.md b/guide.md\n--- a/guide.md\n+++ b/guide.md\n${hunks}`); + return renderUnifiedMarkdownDiff(file, before, after) as string; +} + +const blocks = (html: string) => [...html.matchAll(/<(section|li) class="markdown-source-block([^"]*)"[^>]*data-markdown-side="([^"]+)"[^>]*data-source-start="(\d+)"[^>]*>(.*?)<\/\1>/gs)] + .map((match) => ({ kind: match[2]!.trim(), side: match[3], line: Number(match[4]), html: match[5] })); + +test("unified Markdown interleaves replacements and keeps context beyond sparse hunks once", () => { + const html = render("# Guide\n\nUse old mode.\n\nSame paragraph.\n\nRemove me.\n\nEnd.\n", + "# Guide\n\nUse new mode.\nAn extra paragraph.\n\nSame paragraph.\n\nEnd.\n", + "@@ -3 +3,2 @@\n-Use old mode.\n+Use new mode.\n+An extra paragraph.\n@@ -7,2 +7,0 @@\n-Remove me.\n-\n"); + const result = blocks(html); + expect(result.map(({ kind, side, line }) => [kind, side, line])).toEqual([ + ["", "new", 1], ["deletion", "old", 3], ["addition", "new", 3], ["addition", "new", 4], + ["", "new", 6], ["deletion", "old", 7], ["", "new", 8], + ]); + expect(result[1]!.html).toContain('old'); + expect(result[2]!.html).toContain('new'); +}); + +test("unified Markdown preserves tables and fences while merging changed list items", () => { + const before = "# Guide\n\n| Key | Value |\n| --- | --- |\n| Mode | Old |\n\n## List\n\n- Keep\n- Old item\n\n## Code\n\n```js\nold();\n```\n\nEnd.\n"; + const after = before.replace("| Old |", "| New |").replace("- Old item", "- New item").replace("old();", "new();"); + const result = blocks(render(before, after, + "@@ -5 +5 @@\n-| Mode | Old |\n+| Mode | New |\n@@ -10 +10 @@\n-- Old item\n+- New item\n@@ -15 +15 @@\n-old();\n+new();\n")); + expect(result.map(block => block.kind)).toEqual(["", "", "", "", "", "deletion", "addition", "", "deletion", "addition", ""]); + for (const [index, tag] of [[1, "table"], [8, "pre"]] as const) { + expect(result[index]!.html).toContain(`<${tag}>`); + expect(result[index + 1]!.html).toContain(``); + } + expect(result.at(-1)!.html).toBe("

    End.

    "); + expect(result[1]!.html).toContain(''); + expect(result[2]!.html).toContain(''); + expect(result.filter(block => block.html === "Keep")).toHaveLength(1); + expect(result[5]!.html).toContain('Old'); + expect(result[6]!.html).toContain('New'); +}); + +test("blank-line edits preserve list items without repeating or falsely highlighting them", () => { + const result = blocks(render("# Guide\n\n- One\n\n- Two\n\nEnd.\n", "# Guide\n\n- One\n- Two\n\nEnd.\n", + "@@ -4 +3,0 @@\n-\n")); + expect(result.map(block => block.kind)).toEqual(["", "", "", ""]); + expect(result.map(block => block.html)).toEqual(["

    Guide

    ", "One", "Two", "

    End.

    "]); +}); + +test("empty versions, insertion at the start, and repeated unchanged text retain their source identity", () => { + expect(blocks(render("", "# Added\n", "@@ -0,0 +1 @@\n+# Added\n")).map(block => [block.kind, block.side])).toEqual([["addition", "new"]]); + expect(blocks(render("# Removed\n", "", "@@ -1 +0,0 @@\n-# Removed\n")).map(block => [block.kind, block.side])).toEqual([["deletion", "old"]]); + const result = blocks(render("Same.\nOld.\nSame.\n", "Intro.\nSame.\nNew.\nSame.\n", + "@@ -0,0 +1 @@\n+Intro.\n@@ -2 +3 @@\n-Old.\n+New.\n")); + expect(result.map(({ kind, side, line }) => [kind, side, line])).toEqual([ + ["addition", "new", 1], ["", "new", 2], ["deletion", "old", 2], ["addition", "new", 3], ["", "new", 4], + ]); +});