Separate contiguous paragraphs in HTML passed to the CLI#527
Conversation
Basecamp rich text relies on explicit separator nodes for paragraph spacing, not CSS margins, so contiguous <p>A</p><p>B</p> renders squished. The Markdown pipeline already inserts a separator between blank-line-separated paragraphs via TrixBreak; this brings the raw-HTML passthrough in MarkdownToHTML into line with that behavior. insertParagraphSeparators inserts a bare <br> between directly adjacent, non-empty <p> blocks. It is byte-preserving apart from the inserted separators and idempotent: boundaries that already carry a separator (a bare <br>, or an empty <p><br></p>/<p></p> on either side) are left untouched, so already-separated content (including Basecamp editor output) is a no-op. Only adjacent <p> pairs are separated; a heading, list, or attachment between them already provides its own break.
There was a problem hiding this comment.
Pull request overview
This PR fixes “squished” rendering when users pass raw HTML with contiguous paragraph blocks into the CLI by normalizing adjacent non-empty <p>...</p><p>...</p> boundaries to include a separator (<br>), aligning the HTML passthrough path with the existing Markdown→Trix behavior.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Route
MarkdownToHTML’s HTML-detection branch throughinsertParagraphSeparatorsinstead of returning the input verbatim. - Add paragraph-boundary normalization that inserts
<br>only between directly adjacent, non-empty paragraph blocks (whitespace-only gaps preserved; existing separators left untouched). - Add unit tests covering expected behavior, idempotency, and convergence with the Markdown path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/richtext/richtext.go | Normalizes raw HTML paragraph adjacency by inserting <br> separators and adds helper for detecting “empty” paragraphs. |
| internal/richtext/richtext_test.go | Adds test coverage for separator insertion behavior, idempotency, and equivalence with Markdown paragraph breaks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
2 issues found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- isEmptyParagraph now treats paragraphs containing only non-breaking space entities ( ,  ,  ) as empty, so editor-authored <p> </p> separator lines are recognized and not wrapped in extra <br> separators. Adds tests for the three entity forms. - Update MarkdownToHTML doc comment: HTML input is no longer returned unchanged; a <br> is inserted between directly adjacent paragraphs.
|
Addressed both review findings in 8539d0d:
Tests, `gofmt`, `go vet`, and `golangci-lint` all green. |
Problem
Basecamp rich text relies on explicit separator nodes for paragraph spacing, not CSS margins on
<p>. So contiguous<p>A</p><p>B</p>renders squished — the paragraphs run together and don't look separated. This is common with agent/LLM output, which frequently emits contiguous<p>blocks.The Markdown pipeline already handles this: a blank line between paragraphs inserts a separator (via
TrixBreak). But the raw-HTML passthrough inMarkdownToHTMLsent HTML through verbatim, so contiguous<p>from HTML input rendered squished.Context: internal card "Missing new lines to separate paragraphs in rich text."
main)<p>renders squished<br>separators insertedChange
MarkdownToHTML's HTML-passthrough branch now runsinsertParagraphSeparators, which inserts a bare<br>between directly adjacent, non-empty<p>blocks. This brings the raw-HTML path into line with what the Markdown path already does.Properties:
<br>, or an empty<p><br></p>/<p></p>on either side) is left untouched, so already-separated content, including Basecamp editor output, is a no-op.<p>pairs are separated. Anything between them (a heading, list, or attachment) already provides its own break and is left alone.Why client-side, and why a bare
<br>Basecamp's editor stores a single-Enter break as contiguous
<p>A</p><p>B</p>and a double-Enter break as<p>A</p><p><br></p><p>B</p>. Contiguous<p>is therefore a legitimate, intentional editor state, byte-identical to what an API client sends when it means "separated" — so the server can't safely normalize it without corrupting editor content and breaking round-trips. The CLI can: its input is never editor-authored, and its edit loop already collapses the tight/separated distinction, so treating contiguous HTML paragraphs as separate is consistent with how the CLI already behaves everywhere else.A bare
<br>is used to match what the Markdown pipeline already emits. It renders identically to the editor's canonical<p><br></p>(verified live).Verification
richtext_test.go: behavior + 14 edge cases, idempotency (f(f(x)) == f(x)), and convergence with the Markdown path.richtext,commands,output,presenter,tuipackages pass;gofmt/go vet/golangci-lintclean;check-skill-driftpasses (no CLI surface change).<p>one</p><p>two</p><p>three</p>:<p>one</p><p>two</p><p>three</p>→ renders squished<p>one</p><br><p>two</p><br><p>three</p>→ renders separatedSummary by cubic
Ensures HTML passed to the CLI renders separated paragraphs by inserting
"<br>"between directly adjacent non-empty<p>tags, matching Markdown behavior. Now treats<p> </p>(and / ) as empty separator paragraphs.MarkdownToHTMLnow runsinsertParagraphSeparatorsfor HTML input."<br>"between adjacent non-empty<p>blocks; preserves existing separators (<br>, empty<p>, or<p> </p>/ / ). variants, and Markdown-path convergence.Written for commit 8539d0d. Summary will update on new commits.