fix(markdown): details body with nested code fences#629
Conversation
WalkthroughDetails-block preprocessing now sizes ChangesREADME Processing and Cache Invalidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt (1)
394-397:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the comment to reflect the v3 bump.
The comment references "v2" but the code now uses "v3". Update the comment to explain why the version was bumped from v2 to v3 (dynamic fence handling for details blocks with nested code fences).
📝 Proposed fix to update the comment
- // v2 — bumped after markdown preprocessor overhaul (alerts, - // emoji, details, image-row). Forces re-fetch so users get a - // properly-processed readme instead of waiting for the stale - // v1 entry to expire. + // v3 — bumped after fixing details blocks with nested code fences. + // Forces re-fetch so users get properly-processed readmes with + // dynamic fence delimiters instead of waiting for stale v2 entries + // to expire. val cacheKey = "details:readme:v3:$owner/$repo"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 394 - 397, The comment above the readme processing version bump still says "v2" but the code uses v3; update that comment in DetailsRepositoryImpl (the comment block near the readme/markdown preprocessor in DetailsRepositoryImpl.kt) to say "v3" and briefly state the reason: bumped from v2 to v3 to support dynamic fence handling for details blocks with nested code fences (ensures proper processing of nested fenced code in details blocks), keeping the surrounding explanatory lines about forcing a re-fetch so users get the newly processed readme.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt`:
- Around line 394-397: The comment above the readme processing version bump
still says "v2" but the code uses v3; update that comment in
DetailsRepositoryImpl (the comment block near the readme/markdown preprocessor
in DetailsRepositoryImpl.kt) to say "v3" and briefly state the reason: bumped
from v2 to v3 to support dynamic fence handling for details blocks with nested
code fences (ensures proper processing of nested fenced code in details blocks),
keeping the surrounding explanatory lines about forcing a re-fetch so users get
the newly processed readme.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7c6d3ed-9906-43a1-9148-def2d13b7c88
📒 Files selected for processing (2)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.ktfeature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/utils/preprocessMarkdown.kt
Greptile SummaryThis PR fixes a fence-collision bug where a nested
Confidence Score: 5/5Safe to merge — the fence-sizing and body-extraction changes are logically correct, and the cache key bump will invalidate stale entries for all existing users. The fence-delimiter fix correctly prevents any inner backtick run from closing the outer wrapper. The extractFenceBody refactor eliminates the double-newline bug without introducing new edge cases — the raw-string slice naturally preserves inter-line newlines through EOL tokens that fall between the first and last CODE_FENCE_CONTENT offsets. Wiring githubStoreMarkdownComponents into the inner Markdown is the right call for nested rendering. No functional regressions were identified in the changed paths. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["preprocessMarkdown(rawReadme)"] --> B["Detect details block"]
B --> C["Extract summary + body"]
C --> D["longestBacktickRun(body)"]
D --> E["fence = repeat(max(4, longestRun + 1))"]
E --> F["Emit: fence + ghs-details|encodedSummary + body + fence"]
F --> G["Markdown parser tokenises outer fence as CODE_FENCE"]
G --> H["githubStoreMarkdownComponents.codeFence handler"]
H --> I{"infoStringForDetails?"}
I -- yes --> J["ExpandableDetails composable"]
I -- no --> K["Default code block renderer"]
J --> L["extractFenceBody(model)"]
L --> M["contentNodes = filter CODE_FENCE_CONTENT children"]
M --> N["content.substring(first.start, last.end).trim()"]
N --> O["Inner Markdown with githubStoreMarkdownComponents"]
O --> P["Nested alerts / details / code blocks render correctly"]
Reviews (2): Last reviewed commit: "fix(markdown): preserve body line layout..." | Re-trigger Greptile |
| private fun longestBacktickRun(text: String): Int { | ||
| var max = 0 | ||
| var current = 0 | ||
| for (c in text) { | ||
| if (c == '`') { | ||
| current++ | ||
| if (current > max) max = current | ||
| } else { | ||
| current = 0 | ||
| } | ||
| } | ||
| return max | ||
| } |
There was a problem hiding this comment.
longestBacktickRun is not line-aware
Per CommonMark, only a backtick run that occupies a whole line (optionally followed by spaces, nothing else) can act as a closing fence delimiter. Inline runs like two backticks or ````four backticks```` embedded in surrounding text cannot close a fence. The current implementation counts these inline sequences the same as genuine fence lines, so the computed outer fence can end up longer than strictly necessary. This is harmless in practice — the fence is always safe — but a body containing 5 consecutive inline backticks would produce a 6-backtick wrapper when 3 would suffice. Consider restricting the scan to backtick runs that start at a line boundary to match CommonMark's actual fence-close rule.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt (1)
394-398:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale comment references wrong cache version.
The comment on lines 394-397 describes this as "v2" but the actual cache key is
v4. Update the comment to reflect the current version and the reason for this bump (nested fence handling fix).📝 Suggested fix
- // v2 — bumped after markdown preprocessor overhaul (alerts, - // emoji, details, image-row). Forces re-fetch so users get a - // properly-processed readme instead of waiting for the stale - // v1 entry to expire. + // v4 — bumped after fixing nested code-fence handling in + // <details> blocks. Forces re-fetch so users get properly- + // processed readmes instead of waiting for stale entries to + // expire. val cacheKey = "details:readme:v4:$owner/$repo"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 394 - 398, The inline comment above the cacheKey declaration in DetailsRepositoryImpl.kt is outdated (it says "v2" while the cacheKey is "details:readme:v4:$owner/$repo"); update that comment to state "v4" and mention the actual reason for this bump (fix for nested fence handling in the markdown preprocessor), so the comment matches the cacheKey variable (cacheKey) and explains the version bump and context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt`:
- Around line 394-398: The inline comment above the cacheKey declaration in
DetailsRepositoryImpl.kt is outdated (it says "v2" while the cacheKey is
"details:readme:v4:$owner/$repo"); update that comment to state "v4" and mention
the actual reason for this bump (fix for nested fence handling in the markdown
preprocessor), so the comment matches the cacheKey variable (cacheKey) and
explains the version bump and context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e8dc67e-b08d-4820-986b-2f46aabb5767
📒 Files selected for processing (2)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/markdown/ExpandableDetails.kt
Summary
GHS README's "Show full setup guide"
<details>contained aproperties code fence. The innerterminated our 3-backtickghs-detailswrapper early, so body content leaked outside the collapsible and remainder of README rendered monospace.Fix: size the wrapper fence to
max(4, longestRun + 1)of backticks in body. Nested fences no longer close it. Bumps readme cache key v2→v3 to invalidate stale entries.Test plan
</details>(Wiki & Resources section) renders as normal markdown, not monospace.Summary by CodeRabbit
New Features
Bug Fixes