Fix nested blockquote parsing, drawn quote bars, and phantom list-item spacing#7
Merged
Merged
Conversation
…yled list items Nested blockquotes: the tokenizer treated '>', '>>', and '> > >' identically — it consumed the whole marker prefix as one flat count and kept appending to the same open blockquote, so every nesting level collapsed into a single flat quote. Now detectBlockquote counts markers and deeper lines push child blockquote contexts (the same open-stack pattern nested list items use), giving the assembler real parentID/depth. Shallower-marked lines lazily continue the deepest open quote per CommonMark; blank lines close the whole quote stack. The renderer draws one '│ ' bar per depth level and collapses the inter-paragraph gap between adjacent quote blocks. List spacing: a finalized list line stores a trailing standalone newline run. It normally coalesces into the plain text and is sanitized away, but when an item ends in a styled span (inline code, bold, link) the runs cannot merge, and the surviving newline plus the item's own terminator rendered as an empty paragraph — a full blank line after bullets like 'ending with `code`'. renderListItem now trims trailing newlines from the body before appending its terminator. Golden tokenizer tests for both nesting behaviors and renderer regression tests for depth bars and tight styled-ending items. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
A '>' line with no content is a blank line inside a blockquote: it ends the open paragraph. Previously it just appended a newline that was later sanitized into spaces, so '> p1\n>\n> p2' merged into one line, and after '> > nested' a '> back to first' line lazily continued the nested quote instead of returning to level one (the classic Markdown.pl sample in TEST.md). Now finalizeLine marks the deepest quote context when it sees a marker-only line, and the next quote line closes the open quote stack and reopens fresh blocks at its own marker depth. Rendered result for the TEST.md sample matches GitHub: level 1, nested level 2 (with a level-1 connector bar line), back to level 1. Adjacent quote blocks already collapse their spacing, so paragraph splits inside one quote read as a single quote with paragraph gaps. Golden tokenizer tests and a renderer shape test cover both the paragraph split and the outer-level return. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
… text The quote bar was a literal '│ ' string prepended to every quoted line: it read as a pipe character, broke across wrapped lines, and was included in text selection and copy. Now renderBlockquote marks the block's whole range (including the trailing newline) with .picoBlockquoteLevel/.picoBlockquoteBarColor and indents the text past a leading gutter; the text views draw one rounded vertical bar per nesting level there: - BlockquoteBarLayoutManager (NSLayoutManager.drawBackground override) covers the iOS TextKit 1 view and both macOS views (the macOS 'TextKit 2' view already runs on NSLayoutManager). - BlockquoteBarTextLayoutFragment + NSTextLayoutManagerDelegate covers the real TextKit 2 path on iOS 16+. Because the attribute spans block-trailing newlines, adjacent quote blocks (paragraph splits, nested levels) merge into one continuous bar — matching GitHub's rendering — and copied quote text is now clean. Bar color stays theme-driven (blockquoteColor) and resolves dynamically for light/dark at draw time. Includes a round-trip test guarding that the custom attributes survive the AttributedString conversion the pipeline performs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
![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 adds support for nested blockquotes in PicoMarkdownView by parsing marker depths and rendering vertical bars dynamically via custom TextKit 1 and TextKit 2 layout hooks, rather than baking characters into the text. It also fixes an issue where list items ending in styled spans rendered with extra blank lines. The review feedback focuses on improving robustness and performance, including dynamically retrieving the correct text container in multi-container layouts, avoiding unnecessary rendering bounds expansion for non-blockquote paragraphs, optimizing trailing newline trimming to prevent string allocations, and simplifying boolean assignment logic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
CI confirmed the round-trip test failure: AttributedString's plain conversion initializers silently drop custom NSAttributedString.Keys, so .picoBlockquoteLevel/.picoBlockquoteBarColor never reached NSTextStorage and no bars were drawn (the paragraph indent survived, being a standard attribute — exactly the symptom seen on macOS). Adds AttributeScopes.PicoMarkdownAttributes with typed keys for the two attributes (nesting the platform + Foundation scopes so standard attributes convert too) and pico-scoped conversion helpers, used at the pipeline seams: renderBlockquote's NSAttributedString→AttributedString conversion and the backend's three AttributedString→NSAttributedString sites. Also applies review feedback: resolve the text container per glyph range instead of textContainers.first, skip rendering-surface expansion for non-quote fragments, trim trailing newlines without per-iteration string allocation, and simplify the follows-blockquote check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
The streaming replacement engine holds back a trailing '.' (potential
'...' → '…' replacement) and flushes it as a separate run, so
'First para.' tokenizes as [plain("First para"), plain("."), …] and
the event-shape assertions mismatched. Every other quote golden already
uses period-free text; align these two.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adc1f07560
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…parators - Mark the NSTextLayoutManagerDelegate fragment method nonisolated: the protocol is not main-actor bound (TextKit 2 may lay out off-main) and the method only inspects the text element. Fixes the iOS build error. - Close the quote stack immediately when a marker-only '>' line finalizes, instead of deferring via a context flag. Simpler, and it fixes a real bug the deferred approach had: an *unquoted* line after the separator lazily continued the old quote instead of starting a normal paragraph. Event sequences for quoted continuations are unchanged; new golden covers the unquoted case. - Container-only quote parents (e.g. the implicit level-1 block that '>> nested' opens) now render nothing: children draw the bars for all enclosing levels, so the parent's newline showed as a stray blank quote line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cee78e0095
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ndings Bar continuity (user-reported): each quote block's bars were drawn from its own bounding rect, so the shared level-1 bar broke at every block boundary (nested quotes, paragraph splits). The TextKit 1 layout manager now collects adjacent bar segments (scanning one attribute run past the drawn region) and extends bars shared with a text-adjacent neighbor down to the neighbor's top — one uninterrupted bar per level, GitHub-style. Bars are plain rects now so joined segments meet seamlessly. Codex review findings: - Image-only quote parents (empty alt text) were suppressed as container-only, dropping the attachment: atomic payloads (image, math) now count as content. - The container-only suppression went stale when a nested marker split across chunks: inserting a child block now refreshes its parent's cached render. - Whitespace emitted by marker-only separator lines (and trailing newline runs surviving styled spans) rendered as blank quoted lines: the quote body now trims trailing whitespace before its terminator. Regression tests for all three plus the styled-span quote ending. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ce527f4b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex follow-up on the parent-refresh fix: the renderer's cache was refreshed, but the emitted diff only mentions the new child, and the backend updates storage strictly by diff IDs — so the parent's stale blank quoted line survived in the view until the quote closed. The backend's blockStarted handling now also syncs the inserted block's parent record (no-op when content is unchanged). Backend regression test replays the split-marker streaming scenario against NSTextStorage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1681f6dcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t-safe trim
Third Codex review round, all three accepted:
- detectBlockquote now allows the marker's optional space plus up to
three spaces of indentation before a nested '>' (CommonMark), so
'> > nested' deepens instead of rendering a literal '> nested'.
Five or more spaces keep the inner marker as content.
- Marker-only quote lines ('>', '> ') no longer leak their leftover
whitespace or a spurious hard-break newline into the event stream:
appendDeltaIfNeeded defers emission while a quote line is marker-only
(deferral, not swallowing — '> continuation' content is unchanged).
- With separators clean at the source, renderBlockquote's trailing trim
is newline-only again, so meaningful trailing spaces inside quoted
code spans survive rendering and copy.
Goldens for the padded-nesting and clean-separator event shapes, and a
renderer test pinning the code-span trailing space.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
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 three rendering inconsistencies reported against the demo app (comparing
markdown-it.mdandTEST.mdfixtures with GitHub's rendering):Nested blockquotes rendered flat. The tokenizer consumed
>,>>, and> > >as one flat prefix and appended everything to a single quote block. NowdetectBlockquotecounts markers and deeper lines push child blockquote blocks (the same open-stack pattern nested list items use), giving the assembler realparentID/depth. Shallower-marked lines lazily continue the deepest open quote per CommonMark.Marker-only
>lines were merged into paragraphs. A>line with no content is a blank line inside the quote: it now ends the open paragraph, so> p1/>/> p2renders as two paragraphs, and> Back to the first level.after a nested quote returns to level 1 (the classic Markdown.pl sample inTEST.md) instead of continuing the nested quote.Phantom blank line after list items ending in styled spans. A finalized list line stores a trailing
"\n"run; when the item ends in inline code/bold/link the run can't coalesce into plain text, survived sanitization, and combined with the item's own terminator into an empty paragraph.renderListItemnow trims trailing newlines.Quote bars are now drawn, not typed. The old
"│ "glyphs read as pipe characters, broke across wrapped lines, and were included in selection/copy.renderBlockquotenow marks ranges with.picoBlockquoteLevel/.picoBlockquoteBarColorand reserves an indent gutter; the text views draw one continuous rounded bar per nesting level:BlockquoteBarLayoutManager(NSLayoutManager.drawBackgroundoverride) for the iOS TextKit 1 view and both macOS viewsBlockquoteBarTextLayoutFragment+NSTextLayoutManagerDelegatefor the real TextKit 2 path on iOS 16+Because the attributes span block-trailing newlines, adjacent quote blocks merge into one uninterrupted bar, matching GitHub.
Tests
>>,> > >), lazy continuation, marker-only paragraph splits, outer-level returnAttributedString↔NSAttributedStringconversionsKnown follow-up (out of scope here)
Block elements inside blockquotes (
> ## heading,> 1. list, quoted code blocks) still render as plain quote text — that requires making blockquotes container blocks in the tokenizer and is planned as a separate PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01NRUDZwowe1T836zcYLwQoo
Generated by Claude Code