Address issue #541: render Mermaid/Graphviz in HTML export when highlighting is off - #542
Open
schuyler wants to merge 2 commits into
Open
Address issue #541: render Mermaid/Graphviz in HTML export when highlighting is off#542schuyler wants to merge 2 commits into
schuyler wants to merge 2 commits into
Conversation
Mermaid and Graphviz script inclusion in -[MPRenderer HTMLForExportWithStyles:highlighting:] was nested inside the `if (withHighlighting)` gate, which also drives scriptsOption. With "include highlighting" off, the diagram scripts were never added and scriptsOption stayed MPAssetNone, so exported HTML left mermaid/graphviz fences as plain code blocks. Hoist the Mermaid and Graphviz blocks out of the highlighting gate, each promoting scriptsOption to MPAssetEmbedded (mirroring the MathJax branch). Prism remains gated on highlighting. This is the export-path counterpart to the preview-path fix from #540. Strengthen the two weak export edge-case tests to assert the diagram scripts are embedded (and Prism absent) with highlighting off, and add coverage for both diagrams together, no double-inclusion when highlighting is on, disabled diagrams, withStyles independence, and MathJax coexistence. Related to #541
Contributor
Code Coverage ReportCurrent Coverage: 64.62% Coverage Details (Summary) |
Self-review follow-ups for the HTML-export diagram fix: - Assert the opposite diagram's script is absent in the single-diagram export tests, guarding against a transposed-array regression in the two structurally identical Mermaid/Graphviz blocks. - Add a Graphviz withStyles:NO test mirroring the Mermaid one, confirming diagram scripts embed independently of the styles option. Related to #541
Owner
Author
Maintainer notes (from self-review — observed, deliberately not addressed here)
Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the export-path counterpart of #533/#540: Mermaid and Graphviz diagrams now render in exported HTML regardless of the export panel's "include highlighting" option.
In
-[MPRenderer HTMLForExportWithStyles:highlighting:], diagram script inclusion was nested inside theif (withHighlighting)gate — which also drivesscriptsOption. With highlighting off, the diagram scripts were never added andscriptsOptionstayedMPAssetNone(soMPGetHTMLwould drop them anyway), leaving exported diagram fences as plain code blocks.The fix:
withHighlightinggate into their own top-levelifblocks.scriptsOption = MPAssetEmbeddedwhen either diagram type is enabled, mirroring the existing MathJax branch (both parts are required — hoisting alone would leave the scripts silently dropped).withHighlighting, and leavesstylesOption, the MathJax branch, and the live-preview-scriptspath untouched.Related Issue
Related to #541 (interpretation posted here: #541 (comment))
Judgment Calls
scriptsOptionpromotion is load-bearing: without it, hoisted scripts render to nothing (MPAssetNone→ nil → skipped). Mirroring the MathJax branch keeps the idiom consistent.MPAssetEmbedded), keeping exported files self-contained — consistent with existing export behavior and MathJax.mermaid.initialize(andfunction doGraphviz(engine) {(each verified to occur exactly once in the init scripts, absent from the bundled libraries). Filename substrings only work on the linked preview path.testRendererWithMermaidEnabled/testRendererWithGraphvizEnabledpreviously asserted onlyXCTAssertNotNil— which is why the bug shipped undetected — so they were upgraded rather than left alongside new tests.No open questions block this change.
Testing
Automated (headless, via the mock renderer delegate; the final test gate is the macOS CI run on this branch):
testRendererWithMermaidEnabled/testRendererWithGraphvizEnabled: diagram scripts embedded and Prism absent with highlighting off.withStyles:NOindependence; MathJax coexistence.Generated by Claude Code