From 5f8677e53f50687e26ca49881dbd0ff5ed3d39f8 Mon Sep 17 00:00:00 2001 From: Divyansh Singh <40380293+brc-dd@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:43:07 +0530 Subject: [PATCH] fix(markdown): give stitched lines no position instead of a wrong one An output line assembled from more than one source (page text on the same line as an include directive) matches no single physical line, so any position on it names the wrong place. Rebasing trusted such lines' file attribution and rewrote the page's own relative links against the included file's directory when a mid-line include came first; columns on splice tail lines were measured against the expanded text. Links and attributes on spliced lines now carry no location at all, and the tail line after a mid-line include is marked spliced too. Also from the adversarial review: omit the column when the inline text appears more than once in its raw line (repeated table cells) instead of guessing the first occurrence; give a fully-elided source an identity line map instead of letting resolve() throw; store segment files posix-style so Windows builds compare and report one separator; offset alert paragraph_open attrs past the removed marker like their inline content; keep the page's line map and dev attrs out of excerpt renders; name the including page in dead-link reports for included files (deadLinks[].via); and harden the dev click handler (relative-base endpoint, fetch failure). Co-Authored-By: Claude Fable 5 --- .../node/markdown/plugins/include.test.ts | 45 ++++++++++++++++--- .../node/markdown/plugins/sourceAttrs.test.ts | 29 ++++++++++++ .../markdown/plugins/sourcePositions.test.ts | 10 +++++ __tests__/unit/node/markdownToVue.test.ts | 9 +++- src/client/app/openInEditor.ts | 9 +++- src/node/markdown/lineMap.ts | 9 ++++ src/node/markdown/plugins/containers.ts | 5 +++ src/node/markdown/plugins/frontmatter.ts | 10 ++++- src/node/markdown/plugins/include.ts | 30 ++++++++++--- src/node/markdown/plugins/sourceAttrs.ts | 12 +++-- src/node/markdown/plugins/sourcePositions.ts | 28 +++++++----- src/node/markdownToVue.ts | 5 ++- src/node/plugin.ts | 7 +-- 13 files changed, 175 insertions(+), 33 deletions(-) diff --git a/__tests__/unit/node/markdown/plugins/include.test.ts b/__tests__/unit/node/markdown/plugins/include.test.ts index 1ca005e3..5a7813ee 100644 --- a/__tests__/unit/node/markdown/plugins/include.test.ts +++ b/__tests__/unit/node/markdown/plugins/include.test.ts @@ -612,22 +612,55 @@ describe('node/markdown/plugins/include', () => { ]) }) - test('inline includes attribute the splice line to the page', async () => { + test('stitched lines of inline includes carry no position', async () => { await write('sub/part.md', 'spliced [s](./s)\nnext [n](./n)\n') const { locs } = await renderLocs( 'before after\n' ) - // the first included line merges into the page's line — it keeps the - // page as its file and gets no column, since the stitched line matches - // neither file's authored text; content on the included file's own - // subsequent lines resolves into it exactly + // the first included line merges into the page's line — a stitched + // line matches neither file's authored text, so links on it get no + // location at all; the included file's own subsequent lines resolve + // exactly expect(locs).toEqual([ - { file: path.join(root, 'index.md'), line: 1 }, { file: path.join(root, 'sub/part.md'), line: 2, column: 6 } ]) }) + test('page text after a mid-line include is not misattributed', async () => { + await write('sub/part.md', 'spliced [s](./s.md)') + + const { html, locs } = await renderLocs( + ' after [a](./x.md)\n' + ) + // the page's own link must not be rebased against the included file's + // directory, and neither link gets a position on the stitched line + expect(html).toContain('href="./x.html"') + expect(html).not.toContain('sub/x') + expect(locs).toEqual([]) + }) + + test('page links after a multi-line inline include carry no position', async () => { + await write('sub/part.md', 'T1\nT2\n') + + const { locs } = await renderLocs( + 'before [l](./t.md)\n' + ) + // the tail line's text matches no single physical line - a column + // computed against the expanded text would be wrong + expect(locs).toEqual([]) + }) + + test('a fully elided source still resolves to the page', async () => { + const { env } = await render('', { + include: { silent: true } + }) + expect(env.lineMap!.resolve(0)).toEqual({ + file: path.join(root, 'index.md'), + line: 0 + }) + }) + test('alert bodies inside includes resolve exactly', async () => { await write('sub/part.md', '> [!NOTE]\n> body [x](./x)\n') diff --git a/__tests__/unit/node/markdown/plugins/sourceAttrs.test.ts b/__tests__/unit/node/markdown/plugins/sourceAttrs.test.ts index 441f4207..9fd7bb00 100644 --- a/__tests__/unit/node/markdown/plugins/sourceAttrs.test.ts +++ b/__tests__/unit/node/markdown/plugins/sourceAttrs.test.ts @@ -98,6 +98,35 @@ describe('node/markdown/plugins/sourceAttrs', () => { ) }) + test('alert body paragraphs point past the removed marker line', async () => { + const { vueSrc, file } = await renderPage({ + 'index.md': 'intro\n\n> [!WARNING]\n> line A\n>\n> line B\n' + }) + const at = (line: number) => `data-v-inspector="${rel(file)}:${line}:1"` + // the alert wrapper spans from the marker; its first paragraph does not + expect(vueSrc).toContain(`github-alert" ${at(3)}`) + expect(vueSrc).toContain(`

line A

`) + expect(vueSrc).toContain(`

line B

`) + }) + + test('excerpt renders stay free of source attributes', async () => { + disposeMdItInstance() + const { createMarkdownRenderer } = await import('node/markdown/markdown') + const md = await createMarkdownRenderer('.', { + highlight: (code) => code, + frontmatter: { grayMatterOptions: { excerpt: true } } + }) + const env = { + path: '/docs/page.md', + relativePath: 'page.md', + cleanUrls: false, + emitSourceLoc: true + } as any + await md.renderAsync('---\nt: 1\n---\nfirst para\n\n---\n\nrest\n', env) + expect(env.excerpt).toContain('first para') + expect(env.excerpt).not.toContain('data-v-inspector') + }) + test('builds render without source attributes', async () => { const { vueSrc } = await renderPage( { 'index.md': '# Head\n\npara\n' }, diff --git a/__tests__/unit/node/markdown/plugins/sourcePositions.test.ts b/__tests__/unit/node/markdown/plugins/sourcePositions.test.ts index a7428b7f..181be815 100644 --- a/__tests__/unit/node/markdown/plugins/sourcePositions.test.ts +++ b/__tests__/unit/node/markdown/plugins/sourcePositions.test.ts @@ -127,6 +127,16 @@ describe('markdown/plugins/sourcePositions', () => { ]) }) + test('repeated cell text keeps the line and omits the column', async () => { + const links = await collect( + '| a | b |\n|---|---|\n| [x](./x) | [x](./x) |\n' + ) + expect(links.map((l) => l.loc)).toEqual([ + { file: '/docs/page.md', line: 3 }, + { file: '/docs/page.md', line: 3 } + ]) + }) + test('github alert bodies report exact positions', async () => { const links = await collect( '> [!TIP]\n> a [one](./one)\n> b [two](./two)\n' diff --git a/__tests__/unit/node/markdownToVue.test.ts b/__tests__/unit/node/markdownToVue.test.ts index 14394a01..add8a2d3 100644 --- a/__tests__/unit/node/markdownToVue.test.ts +++ b/__tests__/unit/node/markdownToVue.test.ts @@ -102,7 +102,14 @@ describe('node/markdownToVue', () => { const result = await render(src, file) expect(result.deadLinks).toEqual([ - { url: './nope', resolved: '/nope', file: partial, line: 5, column: 1 }, + { + url: './nope', + resolved: '/nope', + file: partial, + line: 5, + column: 1, + via: file + }, { url: './a', resolved: '/a', file, line: 9, column: 6 }, { url: './b', resolved: '/b', file, line: 10, column: 5 } ]) diff --git a/src/client/app/openInEditor.ts b/src/client/app/openInEditor.ts index f4ea06eb..4afad53b 100644 --- a/src/client/app/openInEditor.ts +++ b/src/client/app/openInEditor.ts @@ -47,9 +47,14 @@ export function setupOpenInEditor(): void { e.preventDefault() e.stopPropagation() clear() + // a relative site base resolves page-relatively - the endpoint lives + // at the server root + const base = import.meta.env.BASE_URL fetch( - `${import.meta.env.BASE_URL}__open-in-editor?file=${encodeURIComponent(loc)}` - ) + `${base.startsWith('.') ? '/' : base}__open-in-editor?file=${encodeURIComponent(loc)}` + ).catch(() => { + // dev server gone - nothing to do + }) }, true ) diff --git a/src/node/markdown/lineMap.ts b/src/node/markdown/lineMap.ts index 20ec1c17..6a8ac8ff 100644 --- a/src/node/markdown/lineMap.ts +++ b/src/node/markdown/lineMap.ts @@ -100,6 +100,15 @@ export class MappedBuilder { this.midLine = text[text.length - 1] !== '\n' } + /** + * Marks the line currently being written as stitched from more than one + * source — for output lines whose text no longer matches any single + * physical line (e.g. page text following a mid-line include). + */ + markLineSpliced(): void { + this.splicedLines.add(this.outLine) + } + private push(segment: LineMapSegment): void { const prev = this.segments[this.segments.length - 1] if (prev) { diff --git a/src/node/markdown/plugins/containers.ts b/src/node/markdown/plugins/containers.ts index 4f816999..039a2d84 100644 --- a/src/node/markdown/plugins/containers.ts +++ b/src/node/markdown/plugins/containers.ts @@ -239,6 +239,11 @@ export const gitHubAlertsPlugin = ( ...firstContent.meta, vpLineOffset: removedLines } + // the enclosing paragraph's map also spans the removed marker + const paragraph = tokens[tokens.indexOf(firstContent) - 1] + if (paragraph?.type === 'paragraph_open') { + paragraph.meta = { ...paragraph.meta, vpLineOffset: removedLines } + } } open.type = 'github_alert_open' open.tag = 'div' diff --git a/src/node/markdown/plugins/frontmatter.ts b/src/node/markdown/plugins/frontmatter.ts index 6f122882..ae36a612 100644 --- a/src/node/markdown/plugins/frontmatter.ts +++ b/src/node/markdown/plugins/frontmatter.ts @@ -21,8 +21,16 @@ export function frontmatterPlugin( env.content = content env.frontmatter = { ...(env.frontmatter as object), ...data } + // the excerpt's token maps are excerpt-local — keep the page's line map + // and dev source attributes out of its render env.excerpt = - renderExcerpt && excerpt ? md.render(excerpt, { ...env }) : excerpt + renderExcerpt && excerpt + ? md.render(excerpt, { + ...env, + lineMap: undefined, + emitSourceLoc: false + }) + : excerpt // gray-matter only ever removes lines from the top of the file, so the // difference in line-break counts is exactly the removed line count diff --git a/src/node/markdown/plugins/include.ts b/src/node/markdown/plugins/include.ts index 1a577ab5..e9cc4526 100644 --- a/src/node/markdown/plugins/include.ts +++ b/src/node/markdown/plugins/include.ts @@ -80,7 +80,14 @@ export function includePlugin( logger ) mdEnv!.src = expanded.src - mdEnv!.lineMap = new LineMap(expanded.segments, expanded.splicedLines) + mdEnv!.lineMap = new LineMap( + // everything replaced by nothing leaves no segments - fall back to + // the page itself so resolve() stays total + expanded.segments.length + ? expanded.segments + : [{ start: 0, file, line: 0 }], + expanded.splicedLines + ) return renderAsync(expanded.src, env) } @@ -132,9 +139,13 @@ async function processIncludes( const region = regionRE.exec(m1) if (region) m1 = m1.slice(0, region.index) - const includePath = m1.startsWith('@') - ? path.join(srcDir, m1.slice(separatorRE.test(m1[1]) ? 2 : 1)) - : path.join(path.dirname(file), m1) + // posix-style, so segment files match the vite-style ids in env.path / + // env.realPath on every platform + const includePath = slash( + m1.startsWith('@') + ? path.join(srcDir, m1.slice(separatorRE.test(m1[1]) ? 2 : 1)) + : path.join(path.dirname(file), m1) + ) // leave circular includes unexpanded — only repeats along the ancestor // chain are cycles, the same file may still be included by siblings @@ -144,7 +155,7 @@ async function processIncludes( // record the dependency before reading it, so that creating a missing // file is picked up by the watcher - env.includes!.push(slash(includePath)) + env.includes!.push(includePath) let content: string try { @@ -271,6 +282,15 @@ async function processIncludes( out.append(expanded.src, expanded.segments, expanded.splicedLines) cursor = m.index + m[0].length cursorLine += countLineBreaks(m[0]) + // page text following the directive on the same source line lands on + // an output line whose text matches no single physical line + const sourceLineEnd = src.indexOf('\n', cursor) + if ( + cursor < (sourceLineEnd === -1 ? src.length : sourceLineEnd) && + expanded.src + ) { + out.markLineSpliced() + } } } passthrough(src.length) diff --git a/src/node/markdown/plugins/sourceAttrs.ts b/src/node/markdown/plugins/sourceAttrs.ts index 2829a103..4125bc08 100644 --- a/src/node/markdown/plugins/sourceAttrs.ts +++ b/src/node/markdown/plugins/sourceAttrs.ts @@ -70,9 +70,15 @@ export function sourceAttrsPlugin(md: MarkdownItAsync): void { ) { continue } - const { file, line } = env.lineMap - ? env.lineMap.resolve(token.map[0]) - : { file: env.realPath ?? env.path, line: token.map[0] } + // vpLineOffset: lines a core rule removed from the block's content + // (the github-alerts marker) while its map kept spanning them + const mapLine = token.map[0] + (token.meta?.vpLineOffset ?? 0) + const resolved = env.lineMap?.resolve(mapLine) + // a spliced line is stitched from more than one source — skip rather + // than point at the wrong file + if (resolved?.spliced) continue + const file = resolved ? resolved.file : (env.realPath ?? env.path) + const line = resolved ? resolved.line : mapLine if (!file) continue token.attrSet( SOURCE_LOC_ATTR, diff --git a/src/node/markdown/plugins/sourcePositions.ts b/src/node/markdown/plugins/sourcePositions.ts index ce377bf7..5c85c1d2 100644 --- a/src/node/markdown/plugins/sourcePositions.ts +++ b/src/node/markdown/plugins/sourcePositions.ts @@ -206,19 +206,27 @@ function sourceLocs(state: StateCore): void { const pos = child[POS] if (!pos) continue + // the destination as authored (before rebasing and href normalization + // mutate the attr at render time), for dead-link reporting + child.meta ??= {} + const dest = child.attrGet(isImage ? 'src' : 'href') + if (dest != null) child.meta.vpRaw = safeDecodeURI(dest) + // vpLineOffset: lines a pre-inline core rule removed from the token's // content (the github-alerts marker) while its map kept spanning them const line = token.map[0] + (token.meta?.vpLineOffset ?? 0) + pos.dLine const resolved = env.lineMap?.resolve(line) + // a spliced line is stitched from more than one source — a position on + // it would be wrong in whichever file it names, so it gets none + if (resolved?.spliced) continue + const loc: MarkdownSourceLoc = resolved ? { file: resolved.file, line: resolved.line + 1 } : { file: env.realPath ?? env.path, line: line + 1 } // block parsing only ever strips a prefix per line, so the inline line - // is a suffix of the raw source line; re-align to get the true column. - // A spliced line is stitched from more than one source, so no column - // on it is meaningful in any single file. - if (pos.startInLine >= 0 && !resolved?.spliced) { + // is a suffix of the raw source line; re-align to get the true column + if (pos.startInLine >= 0) { srcLineStarts ??= makeLineStarts(state.src) const raw = getLine(state.src, srcLineStarts, line) if (raw !== undefined) { @@ -226,19 +234,17 @@ function sourceLocs(state: StateCore): void { loc.column = raw.length - pos.lineText.length + pos.startInLine + 1 } else { // over-indented content gets spaces prepended instead — fall - // back to searching, and omit the column on ambiguity + // back to searching, and omit the column when the text appears + // more than once (repeated table cells) const at = raw.indexOf(pos.lineText) - if (at >= 0) loc.column = at + pos.startInLine + 1 + if (at >= 0 && raw.indexOf(pos.lineText, at + 1) === -1) { + loc.column = at + pos.startInLine + 1 + } } } } - child.meta ??= {} child.meta.vpLoc = loc - // the destination as authored (before rebasing and href normalization - // mutate the attr at render time), for dead-link reporting - const dest = child.attrGet(isImage ? 'src' : 'href') - if (dest != null) child.meta.vpRaw = safeDecodeURI(dest) } } } diff --git a/src/node/markdownToVue.ts b/src/node/markdownToVue.ts index 287e90d5..f12031e7 100644 --- a/src/node/markdownToVue.ts +++ b/src/node/markdownToVue.ts @@ -65,6 +65,8 @@ export interface DeadLink { /** 1-based position in `file`, when known */ line?: number column?: number + /** the page that pulled the link in, when `file` is an included file */ + via?: string } export function clearCache(relativePath?: string) { @@ -273,7 +275,8 @@ export async function createMarkdownToVueRenderFn( ...(resolvedPath != null && { resolved: resolvedPath }), file: loc?.file ?? fileOrig, ...(loc != null && { line: loc.line }), - ...(loc?.column != null && { column: loc.column }) + ...(loc?.column != null && { column: loc.column }), + ...(loc?.file != null && loc.file !== fileOrig && { via: fileOrig }) }) } } diff --git a/src/node/plugin.ts b/src/node/plugin.ts index 585d38b2..455eaecb 100644 --- a/src/node/plugin.ts +++ b/src/node/plugin.ts @@ -478,20 +478,21 @@ function logDeadLinks( devMode = false ) { const logged = new Set() - deadLinks.forEach(({ url, resolved, file, line, column }, i) => { + deadLinks.forEach(({ url, resolved, file, line, column, via }, i) => { const location = line == null ? file : `${file}:${line}${column == null ? '' : `:${column}`}` - const key = `${location}:::${url}` + const key = `${location}:::${url}:::${via ?? ''}` if (logged.has(key)) return logged.add(key) const prefix = '\n'.repeat(i === 0 ? (devMode ? 1 : 2) : 0) const target = resolved && resolved !== url ? ` (resolves to ${c.cyan(resolved)})` : '' + const includedBy = via ? ` (via ${c.white(c.dim(via))})` : '' logger.warn( c.yellow( - `${prefix}(!) Found dead link ${c.cyan(url)}${target} in file ${c.white(c.dim(location))}` + `${prefix}(!) Found dead link ${c.cyan(url)}${target} in file ${c.white(c.dim(location))}${includedBy}` ) ) })