also looking at this
fix: name the document a refused Markdown link was authored in
#94
2 files
+49
-2
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -40,6 +40,22 @@ function routeFor(path: string): string { | ||
| 40 | 40 | return `/${encodePath(route)}` |
| 41 | 41 | } |
| 42 | 42 | |
| 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 | */ | |
| 54 | function 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 | ||
| 43 | 59 | function routeKey(path: string): string { |
| 44 | 60 | const encoded = path.split('/').map(segment => encodeURIComponent(decodeURIComponent(segment))).join('/') |
| 45 | 61 | return encoded.replace(/\/+$/, '') || '/' |
| @@ -124,11 +140,11 @@ export function createMarkdownPageCatalog(markdownFiles: readonly string[], docs | ||
| 124 | 140 | return destination |
| 125 | 141 | const source = sources.get(target) |
| 126 | 142 | 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}`) | |
| 128 | 144 | const locale = source.explicitLocale ? source.locale : page.locale |
| 129 | 145 | const selected = byPage.get(`${locale}\0${source.path}`) |
| 130 | 146 | 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}`) | |
| 132 | 148 | return `${localizeUrl(language, locale, routeFor(selected.relativePath))}${suffix}` |
| 133 | 149 | }, |
| 134 | 150 | }) |
| @@ -4,6 +4,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' | ||
| 4 | 4 | import { tmpdir } from 'node:os' |
| 5 | 5 | import { dirname, join } from 'node:path' |
| 6 | 6 | import { buildDocs } from '../packages/bunpress/bin/cli' |
| 7 | import { createMarkdownPageCatalog, findMarkdownFiles } from '../packages/bunpress/src/markdown-pages' | |
| 7 | 8 | import { decodeHtmlEntities, startServer } from '../packages/bunpress/src/serve' |
| 8 | 9 | |
| 9 | 10 | const temporary: string[] = [] |
| @@ -131,6 +132,36 @@ test('a missing linked source refuses rebuild without replacing the previous com | ||
| 131 | 132 | finally { stop() } |
| 132 | 133 | }, 30000) |
| 133 | 134 | |
| 135 | test('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 | ||
| 150 | test('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 | ||
| 134 | 165 | test('canonical output collisions refuse publication', async () => { |
| 135 | 166 | const f = await fixture() |
| 136 | 167 | await writeFile(join(f.docs, 'guides.md'), '# Ambiguous guides\n') |