Fix markdown formatting issues in TUI streaming: block-level element newline omission and consecutive empty line accumulation #3259

Activity

1. 1716775457damn commented on Jul 7, 2026

   

   1716775457damn

   on Jul 7, 2026

   More actions

   Thanks for the detailed analysis! The ensure_newlines helper is a clean approach. This should fix both the stickiness and empty line accumulation. Adding test cases for various Markdown block combinations would be a great follow-up.
2. Aditaya08 commented on Aug 19, 2026

   

   Aditaya08

   on Aug 19, 2026

   More actions

   Proposed Fix for #3259

   Hi, I'd like to work on this issue.

   Problem

   In render_event() (render.rs), block-level start events (code block, item, blockquote) don't check for existing trailing newlines before writing, causing:

   1. Stickiness: Text immediately followed by a code block renders inline (e.g., 1. Enter directory:╭─ rust)
   2. Extra blank lines: Multiple block-end events unconditionally push \n, stacking 3-4 empty lines

   Solution

   Add an idempotent ensure_newlines helper:

   fn ensure_newlines(output: &mut String, count: usize) {
       if output.is_empty() {
           return;
       }
       let current = output.chars().rev().take_while(|&c| c == '\n').count();
       if current < count {
           output.push_str(&"\n".repeat(count - current));
       }
   }

   Apply it to block start/end handlers:

   Event Handler	Current Behavior	Proposed Change
   Start(Tag::CodeBlock) line 317	No leading newline check	ensure_newlines(output, 1) before start_code_block()
   Start(Tag::Item) line 316	No leading newline check	ensure_newlines(output, 1) before start_item()
   Start(Tag::BlockQuote) line 293	No leading newline check	ensure_newlines(output, 1) before start_quote()
   Start(Tag::Heading) line 289	push('\n') if not empty	Replace with ensure_newlines(output, 2)
   End(TagEnd::Paragraph) line 292	push_str("\n\n") unconditionally	ensure_newlines(output, 2)
   End(TagEnd::Heading) line 298-301	push_str("\n\n") unconditionally	ensure_newlines(output, 2)
   End(TagEnd::List) line 312-314	push('\n') unconditionally	ensure_newlines(output, 1)
   End(TagEnd::Item) line 302-304	push('\n') unconditionally	ensure_newlines(output, 1)
   End(TagEnd::BlockQuote) line 294-297	push('\n') unconditionally	ensure_newlines(output, 1)
   End(TagEnd::Table) line 387-391	push_str("\n\n") unconditionally	ensure_newlines(output, 2)

   Streaming path: MarkdownStreamState::push() uses the same render_markdown() pipeline, so the fix applies automatically.

   Happy to implement once approved.
3. 1716775457damn commented on Aug 27, 2026

   

   1716775457damn

   on Aug 27, 2026

   More actions

   @Aditaya08 方案本质上就是解析器在缺少换行符时补齐的 idempotent 处理，思路没问题。有一点值得注意：ensure_newlines 只处理了"尾部换行不足"，但如果某段输出本来就以"应补 2 个换行"的上下文（例如 heading 后接 code block）结束，count 取 1 时仍然会额外补行，建议为 start/end 事件明确区分语义（block 边界 vs paragraph 边界），并用一个基于事件类型的统一策略而非逐 handler 打补丁。也建议补上覆盖"text + fenced code block""heading + list""连续列表项结束"三种组合的渲染测试。提前开 PR 的话我可以帮忙 review。
4. 1716775457damn commented on Aug 28, 2026

   

   1716775457damn

   on Aug 28, 2026

   More actions

   The ensure_newlines helper approach looks solid — it makes the newline handling idempotent, which is exactly what you need to avoid both sticky inline rendering and the 3-4 blank line stacking. One thought: the paragraph-end handler currently unconditionally pushes "\n\n", so switching it to ensure_newlines(output, 2) will change behavior when the output already ends with a newline (e.g. right after another block element). That's the intended fix, but it's worth adding a couple of unit tests covering (a) block element immediately after text and (b) consecutive block-end events, so the case where current >= count (no push at all) is verified explicitly. That'd lock the behaviour in before touching the TUI renderer.
5. 1716775457damn commented on Aug 28, 2026

   

   1716775457damn

   on Aug 28, 2026

   More actions

   Confirmed on both counts, and the second one has a concrete cause worth writing down.

   The empty-line accumulation is exactly what unconditional pushes predict, because pulldown-cmark emits nested end events — a 3-deep list produces End(Item) → End(Item) → End(List), each appending its own newline. That's why the symptom scales with nesting depth (3–4 blank lines for a 3–4 level list) instead of being a constant offset. Rewriting the append as ensure_newlines is the right shape of fix: it makes each handler idempotent, so the trailing-newline count converges no matter how many end events arrive or in what order. Worth preserving that property even if individual call sites change later.

   Two edge cases to verify with the table rules: End(Table) is reached through End(TableHead) / End(TableRow) / End(TableCell), so check that ensure_newlines(_, 2) on the table close doesn't collapse spacing you actually want after the last row. Same question for a fenced block that is the final child of a list item — End(CodeBlock) then End(Item) then End(List).

   Suggestion: pin it with a snapshot test covering a nested list + fenced code block + blockquote in a single fixture. That combination is where both bugs surfaced, and it's the easiest thing to silently regress the next time render.rs is touched.
6. 1716775457damn commented on Aug 30, 2026

   

   1716775457damn

   on Aug 30, 2026

   More actions

   Two things worth pinning down before this lands, both from the streaming angle, since that is where the helper earns or loses its keep:

   1. The handler table covers Paragraph / Heading / List / Item / BlockQuote, and the write-up mentions table closings, but neither TagEnd::CodeBlock nor Rule (thematic break) appears in the per-handler list. Those are exactly the cases that produce visible stickiness: a horizontal rule rendered straight after a paragraph, and a code block closing directly against the next block. Worth confirming both are routed through ensure_newlines as well, otherwise the accumulation bug just moves one block type over.

   2. For streaming, the property that matters is idempotence across repeated renders, not just within one pass. When the renderer re-parses and re-emits the buffer as tokens arrive, any unconditional push in an End handler compounds once per pass. ensure_newlines only ever tops up to the target count, so it stays correct under repetition, which is the invariant worth locking in. It is also worth writing that into a comment on the helper, because the next person touching it will be tempted to replace the reverse scan with a plain push when output already ends in the right number of newlines.

   On tests: a table-driven case list mapping markdown input to the exact expected output string beats unit-testing the helper in isolation, because the real regression is event ordering, not arithmetic. Include the empty-output case, the helper must stay a no-op there so the first block does not start with a blank line, and a back-to-back code block case.
7. 1716775457damn commented on Sep 7, 2026

   

   1716775457damn

   on Sep 7, 2026

   More actions

   Confirmed on my side — both symptoms share one root cause: pulldown-cmark is fed incremental chunks, and the newline that terminates a chunk is dropped when the next chunk opens a block-level element, so the block boundary never emits its own separator. Suggested shape for the fix: (1) keep a small render state in the streaming renderer (e.g. pending_break / did the last emitted char end a line) rather than relying on the parser to re-emit boundaries, and flush a blank line before a Start(Block) event when the previous output did not end with one; (2) normalise on emit for the accumulation case — collapse runs of more than two consecutive newlines to exactly two and skip a separator that is already at the end of the buffer, which removes the need to track chunk split positions at all; (3) add a test that re-renders one fixed document with the input split at every byte offset and asserts the concatenated output is byte-identical — that catches both the dropped boundary and the accumulation without hand-picked cases.