Repository navigation
📖🔧:point the handbook's cross-links at pages - #1874
Conversation
Six link definitions in the style handbook named the file beside them, `./capitalization.md` and the like. On the built site a page is a directory, so those resolved under the page doing the linking -- `/docs/handbook/style/colons/capitalization.md` -- and returned 404 in production. Every one of them was broken for a reader. They were written that way for a reason: `remark-validate-links` resolves a link against the filesystem, so a site URL fails the check that a neighboring file passes. Absolute URLs satisfy both, since the validator does not follow anything off the disk, and they stay correct when the handbook is read on GitHub rather than on the site. The cost is that a link followed from a deploy preview lands on the published site instead of the preview. Rewriting these at build time would avoid that, at the price of machinery that has to know how a source path becomes a URL. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
✅ Deploy Preview for gh-pages-openinf ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughTwo style handbook pages now use absolute documentation URLs for internal reference links. ChangesDocumentation link normalization
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized documentation change corrects broken handbook cross-links, and no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 markdownlint-cli2 (0.23.2)collections/_docs/handbook/style/code-syntax.mdmarkdownlint-cli2 v0.23.2 (markdownlint v0.41.1) ... [truncated 1267 characters] ... Resolution (node:internal/modules/esm/resolve:271:11) collections/_docs/handbook/style/colons.mdmarkdownlint-cli2 v0.23.2 (markdownlint v0.41.1) ... [truncated 1267 characters] ... Resolution (node:internal/modules/esm/resolve:271:11) Comment |
Six links in the style handbook are broken in production. Confirmed
against the live site:
They were written as
./capitalization.md— the file sitting besidethe source. On the built site a page is a directory, so a relative
.mdtarget resolves under the page doing the linking and hitsnothing.
Affected: five definitions in
colons.md, one incode-syntax.md.Why not root-relative URLs
That was my first fix, and it fails the build.
remark-validate-linksresolves every link against the filesystem, so
/docs/handbook/style/ capitalization/is looked for at../../../../docs/handbook/style/ capitalizationand reported missing —nps testgoes red. The.mdform exists to satisfy that check, and it also keeps the handbook
readable on GitHub.
Absolute URLs satisfy both: the validator does not follow anything off
the disk, and the links stay correct whether the handbook is read on
the site or in the repository. It matches what the handbook already
does elsewhere —
[docs-site]: https://open.inf.is.The trade-off
A cross-reference followed from a deploy preview lands on the published
site rather than on the preview. Rewriting
.mdtargets to URLs atbuild time would avoid that and keep the source relative, but it needs
machinery that knows how a source path becomes a URL. Say the word and
I will build it instead — this fix is the small one.
Checked
nps test— all verify tasks pass,verify-mdincluded.exists in that page's HTML; all 7 confirmed.
broken, down from six.
Summary by CodeRabbit