feat: wire mdbook-mermaid into mdbook build and pages#19
feat: wire mdbook-mermaid into mdbook build and pages#19Chris-godz merged 1 commit intoRethinkCraft:mainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates Mermaid diagram rendering into the mdBook documentation build process. It configures the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates mdbook-mermaid to render diagrams in the documentation. The changes in book.toml correctly configure the preprocessor and add the necessary JavaScript files. The new book/mermaid-init.js script handles theme detection for Mermaid diagrams and reloads the page on theme changes to ensure diagrams are re-rendered correctly.
My review focuses on the new JavaScript file, where I've suggested a few improvements for conciseness and robustness. Specifically, I've pointed out a way to simplify the theme detection logic and how to prevent potential runtime errors when setting up event listeners for theme switching.
book/mermaid-init.js
Outdated
| let lastThemeWasLight = true; | ||
| for (const cssClass of classList) { | ||
| if (darkThemes.includes(cssClass)) { | ||
| lastThemeWasLight = false; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop to determine the current theme can be made more concise and readable by using the Array.prototype.some() method. This simplifies seven lines of code into one.
| let lastThemeWasLight = true; | |
| for (const cssClass of classList) { | |
| if (darkThemes.includes(cssClass)) { | |
| lastThemeWasLight = false; | |
| break; | |
| } | |
| } | |
| const lastThemeWasLight = !darkThemes.some(theme => classList.contains(theme)); |
book/mermaid-init.js
Outdated
| document.getElementById(darkTheme).addEventListener('click', () => { | ||
| if (lastThemeWasLight) { | ||
| window.location.reload(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
To make the script more robust, consider using optional chaining (?.). This will prevent a runtime error if an element with a given theme ID is not found in the DOM. document.getElementById returns null for non-existent elements, and calling addEventListener on null would crash the script.
| document.getElementById(darkTheme).addEventListener('click', () => { | |
| if (lastThemeWasLight) { | |
| window.location.reload(); | |
| } | |
| }); | |
| document.getElementById(darkTheme)?.addEventListener('click', () => { | |
| if (lastThemeWasLight) { | |
| window.location.reload(); | |
| } | |
| }); |
book/mermaid-init.js
Outdated
| document.getElementById(lightTheme).addEventListener('click', () => { | ||
| if (!lastThemeWasLight) { | ||
| window.location.reload(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Similarly, using optional chaining here will make the script more robust against missing theme-switcher elements. This prevents a potential runtime error if an element with a given theme ID is not found.
| document.getElementById(lightTheme).addEventListener('click', () => { | |
| if (!lastThemeWasLight) { | |
| window.location.reload(); | |
| } | |
| }); | |
| document.getElementById(lightTheme)?.addEventListener('click', () => { | |
| if (!lastThemeWasLight) { | |
| window.location.reload(); | |
| } | |
| }); |
Chris-godz
force-pushed
the
feat/mdbook-mermaid-pages
branch
from
0263110 to
d7363cd
Compare
- feat(book): enable the mermaid preprocessor and keep the readable init bootstrap without tracking the minified vendor blob - ci(pages): install mdbook-mermaid 0.17.0, pin mdbook 0.5.2, and generate Mermaid assets before the Pages build - ci(docs-validate): mirror the mdbook-mermaid setup and mdbook pin in docs validation - chore(gitignore): ignore regenerated book/mermaid.min.js copies from local installs
Chris-godz
force-pushed
the
feat/mdbook-mermaid-pages
branch
from
d7363cd to
6446f51
Compare
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
1 participant
Summary
mdbook-mermaidpreprocessor inbook/book.tomland keep the readablebook/mermaid-init.jsbootstrap for the existing Mermaid diagramsmdbook-mermaid0.17.0 in both Pages and docs validation workflows, and runmdbook-mermaid install bookbefore each build so the runtime asset is generated there instead of being tracked in gitmdbookto 0.5.2 in CI so Pages and validation use the same toolchain when building Mermaid-enabled docsbook/mermaid.min.jscopies locally so the minified vendor blob does not keep showing up in git diffsNote
Medium Risk
Medium risk because it changes GitHub Actions build steps and bumps/pins
mdbook, which could break docs validation or Pages deployment if the toolchain install or preprocessor behavior differs across runners.Overview
Mermaid rendering is now built into the mdBook pipeline. The book config enables the
mdbook-mermaidpreprocessor and loads Mermaid runtime/init JS viaadditional-js.CI and Pages workflows now install Rust,
mdbook-mermaid(pinned to0.17.0), and runmdbook-mermaid install bookbefore building;mdbookis pinned to0.5.2to keep builds consistent. The generatedbook/mermaid.min.jsis added to.gitignoreso the vendored asset isn’t tracked.Written by Cursor Bugbot for commit 6446f51. This will update automatically on new commits. Configure here.