Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a crash in MathFirstMarkdownViewRenderer when rendering Markdown that contains both fenced code blocks and block (display) math by avoiding range invalidation during in-place string mutation.
Changes:
- Iterate parsable segments in reverse order so later replacements don’t invalidate earlier segment ranges.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for range in extractor.parsableRanges(in: rawText).reversed() { | ||
| let segment = rawText[range] | ||
| let segmentParser = MathParser(text: segment) |
There was a problem hiding this comment.
Even with iterating ranges in reverse, this loop still reuses precomputed Range<String.Index> values after mutating rawText (via replaceSubrange). Swift String mutations can invalidate existing indices/ranges, which can still lead to crashes in edge cases. Consider capturing ranges as integer offsets (e.g., UTF-16 offsets) and recreating fresh String.Index values on the current rawText before slicing/replacing, or perform replacements on a copied String segment and then splice the segment back into rawText.
| var extractor = ParsingRangesExtractor() | ||
| extractor.visit(content.parse(options: ParseOptions().union(.parseBlockDirectives))) | ||
| for range in extractor.parsableRanges(in: rawText) { | ||
| for range in extractor.parsableRanges(in: rawText).reversed() { |
There was a problem hiding this comment.
This change fixes a crash scenario (math blocks + code blocks). Please add a regression test that renders markdown containing both a fenced code block and a display-math block to ensure MathFirstMarkdownViewRenderer no longer crashes when processing multiple parsable segments.
Closes #141