Render default 404 pages correctly in docs - #1479
Conversation
…. Assisted-by: Claude Sonnet 4.6
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1479 +/- ##
==========================================
- Coverage 89.78% 89.78% -0.01%
==========================================
Files 60 60
Lines 8439 8445 +6
Branches 8439 8445 +6
==========================================
+ Hits 7577 7582 +5
Misses 545 545
- Partials 317 318 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the documentation build “move dev docs into book/dev” step to also generate a root-level book/404.html derived from book/dev/404.html, rewriting paths so the 404 page can load its assets correctly when served from GitHub Pages project URLs (fixing #1390).
Changes:
- Added
write_root_404()to post-processbook/dev/404.htmlintobook/404.html. - Rewrites
<base href>and assethref/srcpaths to ensure CSS/JS resolve correctly from the root 404 page. - Hooks the new 404 generation into the existing
move_to_dev()docs build flow.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| def write_root_404(bookdir: Path) -> None: | ||
| """Generate book/404.html from book/dev/404.html with corrected asset paths.""" | ||
| content = (bookdir / "dev" / "404.html").read_text(encoding="utf-8") |
There was a problem hiding this comment.
Probably not necessary
| content = content.replace( | ||
| 'const path_to_root = "";', 'const path_to_root = "dev/";' | ||
| ) | ||
| content = content.replace('var pathToRoot = "";', 'var pathToRoot = "dev/";') |
There was a problem hiding this comment.
I'm sure if the 404 pages break again, we can fix them later.
| # Create a fix for the 404 routing | ||
| write_root_404(bookdir) |
There was a problem hiding this comment.
These changes are related to the documentation pages, not the tool itself. probably not necessary
| content = re.sub( | ||
| r'((?:href|src)=")((?!(?:https?:|//|#|/)))', | ||
| r"\1dev/\2", | ||
| content, | ||
| ) |
There was a problem hiding this comment.
Unsure what this means tbh
| """Generate book/404.html from book/dev/404.html with corrected asset paths.""" | ||
| content = (bookdir / "dev" / "404.html").read_text(encoding="utf-8") | ||
|
|
||
| # Replace base href with the GitHub Pages project root |
There was a problem hiding this comment.
| # Replace base href with the GitHub Pages project root | |
| # Replace base href with the repository root |
I guess technically it's the repository root, which the GH pages uses in the URL.
And so the repository would need to be cloned into a "MUSE2" directory for this to work locally, right?
There was a problem hiding this comment.
So it's kind of the other way around. The GitHub pages project adds this extra "MUSE2/" path, which is not known at build time. To make this work locally, you'd need to put everything inside the bookdir another layer deeper, under a 'MUSE2' folder.
There was a problem hiding this comment.
Ah yes, of course. Fair enough!
tsmbland
left a comment
tsmbland
left a comment
There was a problem hiding this comment.
I don't necessarily understand what this is doing, but I followed the instructions to reproduce and it worked, so all good
dc2917
left a comment
dc2917
left a comment
There was a problem hiding this comment.
Seems logical to me, and seems to be working too. Just wondering if the project root can be bookdir.parent or something.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Description
This PR should fix the fact the 404 pages are not displaying at all now (and were not rendering properly before). It will use
It's hard to test locally, the best approach I found was to do the following (order is important!):
mdbook serve -ojust build-docs all_with_oldbook/404.htmlto<base href="/">(it is how it is to be compatible with the GitHub Pages deployment)http://localhost:3000/dev/bad-path/bad.html)Fixes #1390
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks