Skip to content

Add Mermaid Support in Fenced Code Blocks - #169

Open
Sam Clark (28pins) wants to merge 5 commits into
microsoft:mainfrom
28pins:28pins-mermaid-feature
Open

Sam Clark (28pins) wants to merge 5 commits into
microsoft:mainfrom
28pins:28pins-mermaid-feature

Conversation

@28pins

Copy link
Copy Markdown

Summary

  • Added mermaid support using beautiful-mermaid-swift
  • For compatibility, rendering can be disabled using MermaidConfig.disabled
  • MermaidConfig offers theme customization options (inherits directly from package, VS Code based)

Validation

Add tests, check demos, stress test kitchen sink demo

OSS readiness

  • No secrets, internal URLs, private identifiers, or product-only service names were added.
  • Public docs, fixtures, or notices were updated if behavior or dependencies changed.
  • Third-party dependency changes (adds, removes, version bumps) are intentional and reviewed.
  • Streaming/incomplete markdown behavior remains covered by fixtures or tests.

@28pins
Sam Clark (28pins) requested review from a team and a lite review from Copilot August 15, 2026 22:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

Comment thread Sources/MarkdownText/UI/MermaidStreamDebouncer.swift
Comment thread Sources/MarkdownText/UI/MermaidStreamDebouncer.swift
Comment thread Sources/MarkdownText/Models/MarkdownRenderConfig+Builders.swift
@28pins

Copy link
Copy Markdown
Author

Copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

Sam Clark (28pins) and others added 3 commits September 1, 2026 09:07
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@28pins Sam Clark (28pins) changed the title 28pins mermaid feature Add Mermaid Support in Fenced Code Blocks Sep 1, 2026
@28pins

Copy link
Copy Markdown
Author

Jun Yan (@junyan72) would you mind allowing the workflow runs? These are my first PRs on this repo, so let me know if I need to do anything else.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical compile errors and unresolved Mermaid view issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

Sources/MarkdownText/Models/MarkdownRenderConfig+Builders.swift:28

  • The added argument lines contain trailing spaces, and .swiftlint.yml leaves the trailing_whitespace rule enabled while CI runs SwiftLint in strict mode. Remove the spaces from these lines and the repeated builder arguments so lint does not fail after the syntax errors are corrected.

Examples/SwiftStreamingMarkdownSample/SwiftStreamingMarkdownSample/Resources/Fixtures/kitchen-sink.md:224

  • This fixture also tells users that Mermaid fences are interactive, but the integrated native drawing view adds no interaction handlers. Change the wording to match the actual rendered behavior or add the promised interactions.
Mermaid fences render as interactive diagrams. Configure them via `MermaidConfig`; setting `.disabled` falls back to a readable fenced code block.

README.md:107

  • This adds BeautifulMermaid and its elk-swift dependency, but the README's Binary Size section still states the old ~1 MB estimate and lists only the previous dependencies. Please update that public size disclosure after measuring the new renderer's impact.
- [x] Mermaid diagrams — rendered as interactive diagrams for `mermaid`-tagged fenced code blocks; theme and opt-out via `MermaidConfig` (`withMermaidConfig`, `.disabled`)

README.md:107

  • The dependency view used here is a native drawing view and this integration adds no gestures or other interaction handlers, so describing these diagrams as “interactive” overpromises the behavior users receive. Describe them as native/rendered diagrams, or implement the promised interaction.
- [x] Mermaid diagrams — rendered as interactive diagrams for `mermaid`-tagged fenced code blocks; theme and opt-out via `MermaidConfig` (`withMermaidConfig`, `.disabled`)

Sources/MarkdownText/Models/MarkdownRenderConfig+Builders.swift:316

  • These : was missing suffixes are also embedded in the Swift source, making the withImageConfig initializer call invalid syntax and preventing compilation. Remove the annotations rather than leaving them as source text.
      mermaidConfig: mermaidConfig,           : was missing
      blockQuoteAlertStyle: blockQuoteAlertStyle : was missing

Sources/MarkdownText/UI/MermaidBlockView.swift:71

  • Once settled, the raw-code fallback is replaced by a native MermaidDiagramView, but this branch adds no accessibility element, label, or textual value for the diagram. VoiceOver users can therefore lose the Mermaid block entirely; expose an accessible diagram label and/or the source text (and keep it synchronized with settledSource) on the rendered view.
      MermaidDiagramView(
        source: settledSource,
        theme: config.mermaidConfig.theme.diagramTheme(for: colorScheme),
        parseError: $parseError
      )
  • Files reviewed: 15/16 changed files
  • Comments generated: 6
  • Review effort level: Lite

Comment on lines +26 to +28
mermaidConfig: mermaidConfig,
imageConfig: imageConfig,
blockQuoteAlertStyle: blockQuoteAlertStyle
Comment on lines +248 to +249
imageConfig: imageConfig, : was missing
blockQuoteAlertStyle: blockQuoteAlertStyle : was missing
Comment on lines +40 to +47
let alertStyle = MarkdownRenderConfig.defaultBlockQuoteAlertStyle

let base = MarkdownRenderConfig(imageConfig: imageConfig, blockQuoteAlertStyle: alertStyle)
let updated = base.withMermaidConfig(.disabled)

XCTAssertEqual(updated.mermaidConfig, .disabled)
XCTAssertEqual(updated.imageConfig, imageConfig)
XCTAssertEqual(updated.blockQuoteAlertStyle, alertStyle)
Comment on lines +38 to +44
let settledSource = State(initialValue: "")
let parseError = State<Error?>(initialValue: nil)
let debouncer = MermaidStreamDebouncer()
debouncer.onCommit = { source in
settledSource.wrappedValue = source
parseError.wrappedValue = nil
}
Comment on lines +53 to +60
.onAppear {
debouncer.schedule(code)
}
.onChange(of: code) { newValue in
debouncer.schedule(newValue)
}
.onDisappear {
debouncer.cancel()
Comment on lines +67 to +70
MermaidDiagramView(
source: settledSource,
theme: config.mermaidConfig.theme.diagramTheme(for: colorScheme),
parseError: $parseError
@28pins

Copy link
Copy Markdown
Author

Copilot Fix the code for all comments in this review thread.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review thread.

@vincent-peng

Copy link
Copy Markdown
Contributor

Reviewed in depth, including checking out beautiful-mermaid-swift at tag 1.0.4 to verify the API. This doesn't currently compile — reworking needed before review:

Blocking

  1. Literal syntax errors. MarkdownRenderConfig+Builders.swift contains : was missing annotations (L248-249, L315-316) — looks like an incomplete conflict resolution in "Fix missing configs". swiftc -parse fails on them.
  2. Undefined symbols. blockQuoteAlertStyle / BlockQuoteAlertStyle are referenced ~17 times including in MermaidConfigTests, but that type exists only in Add GFM Alert Styling #168's branch. This PR silently depends on an unmerged, unlanded PR while README still lists alerts as unimplemented. Either rebase onto Add GFM Alert Styling #168 after it lands with the correct dictionary type, or remove the references.
  3. Lint. ~37 lines with trailing whitespace + missing EOF newline → swiftlint --strict fails.

Design

  1. MermaidConfig enabled by default contradicts the established opt-in precedent (ImageConfig defaults .disabled and is documented experimental). Every consumer's ```mermaid fences silently become diagrams via a new ELK-backed dependency on upgrade. Deliberate choice worth making explicit, or default to disabled.
  2. Layout runs synchronously on the main thread. Verified in the dep: prepareDiagram() (parse + ELK layout) executes inline in the source setter called from updateUIView. Large diagrams block the UI thread, which violates this repo's main-thread rule. The dep offers async MermaidRenderer APIs — worth using or flagging upstream.
  3. Debounce-only gating flickers. Any stream pause >300ms commits partial diagram source; the next token flips back to code. The streamdown convention is code-until-closing-fence — at minimum worth documenting the tradeoff.
  4. Scope creep. withBlockQuoteAlertStyle and the imageConfig builder fix are unrelated to mermaid; both belong elsewhere (the builder fix is separately up as Fix imageConfig being dropped by MarkdownRenderConfig builders #179).
  5. Stale ~1 MB binary-size claim in README — BeautifulMermaid + elk-swift now link unconditionally even when disabled.

Verified fine: MermaidDiagramView(source:theme:parseError:) exists at 1.0.4 (UIViewRepresentable, parseError: Binding<Error?>), iOS 16 floor matches, MIT-licensed pure-Swift dep (no WebKit — better than the alternatives), MermaidStreamDebouncer lifecycle is clean and tested, exact-version pinning matches repo style, and CodeBlock+.swift beside blockmath is the right seam.

Minor: MermaidCodeFallbackView drops code-block chrome (copy button/language label — consider reusing CodeBlockView), diagram view has no accessibility label, and the exact lowercased() == "mermaid" info-string match could use a test for ```mermaid params behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants