ReviewOS

also looking at this

stacks/bunpress

fix: name the document a refused Markdown link was authored in

#94
Closed glennmichael123 wants to merge fix/name-the-document-a-refused-link-is-in into main
2 files +49 -2
packages/bunpress/src/markdown-pages.tsmodified+18-2
Changes to packages/bunpress/src/markdown-pages.ts
@@ -40,6 +40,22 @@ function routeFor(path: string): string {
4040 return `/${encodePath(route)}`
4141}
4242
43/**
44 * The document a link was authored in, for a message somebody has to act on.
45 *
46 * Both link failures below name the destination, which is not enough to find
47 * the link: the destination is what is missing, and the file holding it is
48 * what has to be edited. Relative to the documentation root, because an
49 * absolute temporary path is noise in a build log.
50 *
51 * Under an include, `resolveLink` is handed the innermost origin rather than
52 * the host page, so this names the file the link was actually written in.
53 */
54function authoredIn(page: MarkdownPage, root: string): string {
55 const within = relative(root, page.sourceFile)
56 return within && !within.startsWith('..') && !isAbsolute(within) ? within.split(sep).join('/') : page.sourceFile
57}
58
4359function routeKey(path: string): string {
4460 const encoded = path.split('/').map(segment => encodeURIComponent(decodeURIComponent(segment))).join('/')
4561 return encoded.replace(/\/+$/, '') || '/'
@@ -124,11 +140,11 @@ export function createMarkdownPageCatalog(markdownFiles: readonly string[], docs
124140 return destination
125141 const source = sources.get(target)
126142 if (!source)
127 throw new Error(`Markdown link has no documentation source: ${rawPath}`)
143 throw new Error(`${authoredIn(page, root)}: Markdown link has no documentation source: ${rawPath}`)
128144 const locale = source.explicitLocale ? source.locale : page.locale
129145 const selected = byPage.get(`${locale}\0${source.path}`)
130146 if (!selected)
131 throw new Error(`Markdown link has no rendered page: ${rawPath}`)
147 throw new Error(`${authoredIn(page, root)}: Markdown link has no rendered page: ${rawPath}`)
132148 return `${localizeUrl(language, locale, routeFor(selected.relativePath))}${suffix}`
133149 },
134150 })
test/markdown-source-links.test.tsmodified+31-0
Changes to test/markdown-source-links.test.ts
@@ -4,6 +4,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
44import { tmpdir } from 'node:os'
55import { dirname, join } from 'node:path'
66import { buildDocs } from '../packages/bunpress/bin/cli'
7import { createMarkdownPageCatalog, findMarkdownFiles } from '../packages/bunpress/src/markdown-pages'
78import { decodeHtmlEntities, startServer } from '../packages/bunpress/src/serve'
89
910const temporary: string[] = []
@@ -131,6 +132,36 @@ test('a missing linked source refuses rebuild without replacing the previous com
131132 finally { stop() }
132133}, 30000)
133134
135test('a refused link names the document it was authored in', async () => {
136 const f = await fixture()
137 await writeFile(join(f.docs, 'guides/start.md'), '# Start\n\n[gone](../adr/absent.md)\n')
138 const files = await findMarkdownFiles(f.docs)
139 const catalog = createMarkdownPageCatalog(files, f.docs, { enabled: false, locales: ['en'], defaultLocale: 'en' } as never)
140 const page = catalog.findPage('en', '/guides/start')!
141 expect(page).toBeDefined()
142
143 // The destination is the part that is missing, so naming it alone leaves the
144 // reader grepping for whichever of their documents holds the link. The path
145 // is relative to the documentation root: an absolute temporary directory is
146 // noise in a build log.
147 expect(() => catalog.resolveLink(page, '../adr/absent.md')).toThrow('guides/start.md: Markdown link has no documentation source: ../adr/absent.md')
148}, 30000)
149
150test('a refused link inside an include names the included file, not its host', async () => {
151 const f = await fixture()
152 await mkdir(join(f.docs, 'parts'), { recursive: true })
153 await writeFile(join(f.docs, 'parts/snippet.md'), '[gone](./absent.md)\n')
154 const files = await findMarkdownFiles(f.docs)
155 const catalog = createMarkdownPageCatalog(files, f.docs, { enabled: false, locales: ['en'], defaultLocale: 'en' } as never)
156 const included = catalog.findPage('en', '/parts/snippet')!
157
158 // `rewriteMarkdownSourceLinks` hands `resolveLink` the innermost origin on
159 // its include stack, so the file named here is where the link was written
160 // rather than whichever document pulled it in. That is the one a person has
161 // to open, and the two differ exactly when the message matters most.
162 expect(() => catalog.resolveLink(included, './absent.md')).toThrow('parts/snippet.md: Markdown link has no documentation source: ./absent.md')
163}, 30000)
164
134165test('canonical output collisions refuse publication', async () => {
135166 const f = await fixture()
136167 await writeFile(join(f.docs, 'guides.md'), '# Ambiguous guides\n')