Skip to content

Blank lines between blocks vanish when CLI-posted content is edited in Basecamp - #637

Open
jorgemanrubia wants to merge 2 commits into
mainfrom
fix-paragraph-separator-roundtrip
Open

Blank lines between blocks vanish when CLI-posted content is edited in Basecamp#637
jorgemanrubia wants to merge 2 commits into
mainfrom
fix-paragraph-separator-roundtrip

Conversation

@jorgemanrubia

@jorgemanrubia jorgemanrubia commented Aug 17, 2026

Copy link
Copy Markdown
Member

Content posted through the CLI displays correctly, then loses every blank line between blocks the first time someone opens it in Basecamp, makes any edit and saves. Paragraphs, headings and lists all end up flush against each other, and the spacing is gone for good.

The CLI spelled that blank line as a bare top-level <br>: the Markdown pipeline emitted one for each blank line between blocks. A bare top-level <br> is not a block in the editor's document model, so it can never be a child of the root — the editor drops it on import, deliberately, in lexxy src/elements/editor.js:

  // Whitespace-only text nodes (e.g. "\n" between block elements like <div>) and stray line break
  // nodes are formatting artifacts from the HTML source. They can't be appended to the root node
  // and have no semantic meaning, so we strip them during import.
  #isNotWhitespaceOnlyNode(node) {
    if ($isLineBreakNode(node)) return false

The editor's own blank line is an empty paragraph, <p><br></p>. This emits that instead, so the separator round-trips. Inside a blockquote the break stays a <br> — there it is inline content, which the editor preserves. The raw-HTML passthrough from #527 now inserts the same durable separator between adjacent paragraphs.

Evidence

Driven through lexxy v0.9.29 (the version bc3 pins), using its real Lexical import/export, on the same document each time — separator count before vs. after one edit:

  • <p>Para one.</p><br><p>Para two.</p><br><h2>Heading</h2>… (what the CLI stored before this change) — 4 separators, 0 after the edit.
  • <p>Para one.</p><p><br></p><p>Para two.</p><p><br></p><h2>Heading</h2>… (what it stores now) — 4 separators, 4 after the edit, and byte-identical on a second round trip.

Same harness for the blockquote cases: <blockquote>A<br>B</blockquote> and <blockquote><p>A</p><br><p>B</p></blockquote> both survive untouched, which is why nested breaks are left alone.

Scope

Separators the caller supplied in raw HTML are left exactly as they came. Only genuinely adjacent paragraphs get a separator inserted. The paragraph matching here is regex-based and not nesting-aware, so rewriting existing <br>s would reach into blockquotes and replace inline content that the editor already preserves. Normalizing caller-supplied HTML would need a real parser and is deliberately not part of this change: what this fixes is the CLI emitting a separator the editor cannot keep.

bc3 #11986 does not cover this

Markdown-in/markdown-out for the v1 API (basecamp/bc3#11986) looks adjacent but leaves this bug where it is:

  • It touches no client-side code — no JS, no assets, no lexxy pin — so the import filter that drops the separators is byte-identical on that branch.
  • Its conversion only fires when a request submits the new rich_text_format: markdown hint. The CLI submits HTML, so what gets stored is unchanged.
  • If the CLI were rewired to markdown-in, its Commonmarker.to_html invocation emits adjacent <p> with no separator at all, and bc3's .formatted_content sets p, div { margin: 0 } — so the blank lines would be lost at write time instead of at edit time. Different mechanism, same missing spacing.

The Markdown pipeline separated top-level blocks with a bare <br>, and
the raw-HTML passthrough inserted one between adjacent paragraphs. A bare
top-level <br> is not a block in Basecamp's editor document model, so the
editor discards it on import: the content displays correctly until someone
opens it, makes any edit and saves, at which point every blank line is
gone for good.

Emit the editor's own separator instead — an empty paragraph — which
survives the round trip untouched. Inside a blockquote the break stays a
<br>: there it is inline content, which the editor keeps. Bare <br>
separators arriving through the HTML passthrough are rewritten to the
durable form for the same reason.

Verified against lexxy v0.9.29 (the editor bc3 ships): a document with
four separators comes back with zero after an edit before this change,
and with all four after it.
Copilot AI balanced review requested due to automatic review settings August 17, 2026 07:23
@github-actions github-actions Bot added the tests Tests (unit and e2e) label Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates rich-text separators so blank lines survive Basecamp editor round-trips.

Changes:

  • Emits durable empty paragraphs for top-level Markdown separators.
  • Rewrites raw-HTML paragraph separators and expands regression tests.

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.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/richtext/richtext.go Implements durable separator rendering and rewriting.
internal/richtext/richtext_test.go Updates expectations and adds separator regressions.
Suppressed comments (1)

internal/richtext/richtext.go:436

  • The paragraph regex is not aware of HTML nesting, so this also rewrites valid nested breaks that the new renderer deliberately preserves. For example, <blockquote><p>A</p><br><p>B</p></blockquote> becomes <blockquote><p>A</p><p><br></p><p>B</p></blockquote>, although a <br> inside a blockquote is legal inline content and should survive editing. Restrict separator rewriting to direct children of the document root (preferably via an HTML parser) and leave nested boundaries unchanged.
			b.WriteString(reBR.ReplaceAllString(gap, ""))
			b.WriteString(paragraphSeparator)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/richtext/richtext.go Outdated
if !empty[i] && !empty[i+1] && strings.TrimSpace(gap) == "" {
b.WriteString(gap)
b.WriteString("<br>")
if !empty[i] && !empty[i+1] && isSeparatorGap(gap) {
Rewriting a bare <br> boundary reached into blockquotes: the paragraph
matching is regex-based, not nesting-aware, and a <br> between two
paragraphs inside a blockquote is inline content the editor keeps
untouched (verified against lexxy v0.9.29). Insert the durable separator
only where paragraphs are genuinely adjacent, and leave separators the
caller supplied as they came.
Copilot AI review requested due to automatic review settings August 17, 2026 07:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@robzolkos robzolkos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated this against the Basecamp card and Lexxy v0.9.29. The existing top-level <br> separators collapse from 4 to 0 on an editor round trip; the <p><br></p> separators emitted here survive 4 to 4, while nested blockquote <br> remains intact. The focused rich-text tests and full bin/ci pass. The documented caller-supplied raw HTML limitation does not block this fix. Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants