Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jorgemanrubia wants to merge 2 commits into
base: main
Choose a base branch
from
+78 −35
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cb57e6d
Blank lines between blocks vanish when CLI content is edited
jorgemanrubia Aug 17, 2026
3965da1
Leave caller-supplied separators alone in the HTML passthrough
jorgemanrubia Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions internal/richtext/richtext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,15 @@ var mdConverter = goldmark.New(
),
)

// TrixBreak is a custom block node that renders as <br>\n for Trix paragraph spacing.
// paragraphSeparator is the blank line Basecamp's editor itself stores between
// two blocks. A bare top-level <br> is not a block in the editor's document
// model, so it is discarded the first time someone edits the content and the
// spacing disappears; an empty paragraph survives the round trip.
const paragraphSeparator = "<p><br></p>"

// TrixBreak is a custom block node that renders the blank line between blocks:
// an empty paragraph at the top level, a <br> inside a block (see
// renderTrixBreak).
type TrixBreak struct{ ast.BaseBlock }

// KindTrixBreak is the node kind for TrixBreak.
Expand Down Expand Up @@ -329,11 +337,18 @@ func (r *trixRenderer) renderFencedCodeBlock(w util.BufWriter, source []byte, no
return ast.WalkContinue, nil
}

func (r *trixRenderer) renderTrixBreak(w util.BufWriter, _ []byte, _ ast.Node, entering bool) (ast.WalkStatus, error) {
// renderTrixBreak emits an empty paragraph for a top-level break and a <br> for
// one inside a block. Only the top level needs a block-level separator:
// a <br> nested in a blockquote is inline content, which survives editing.
func (r *trixRenderer) renderTrixBreak(w util.BufWriter, _ []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
if !entering {
return ast.WalkContinue, nil
}
_, _ = w.WriteString("<br>\n")
if parent := node.Parent(); parent != nil && parent.Kind() == ast.KindDocument {
_, _ = w.WriteString(paragraphSeparator + "\n")
} else {
_, _ = w.WriteString("<br>\n")
}
return ast.WalkContinue, nil
}

Expand All @@ -348,8 +363,8 @@ func (r *trixRenderer) renderEscapedAt(w util.BufWriter, _ []byte, _ ast.Node, e
// MarkdownToHTML converts Markdown text to HTML suitable for Basecamp's rich text fields.
// It uses goldmark with custom AST transformations for Trix editor compatibility.
// If the input already appears to be HTML, it is passed through with existing
// formatting preserved, except that a <br> separator is inserted between
// directly adjacent paragraph blocks (see insertParagraphSeparators).
// formatting preserved, except that a separator is inserted between directly
// adjacent paragraph blocks (see insertParagraphSeparators).
func MarkdownToHTML(md string) string {
if md == "" {
return ""
Expand All @@ -370,9 +385,9 @@ func MarkdownToHTML(md string) string {
return strings.TrimSpace(buf.String())
}

// insertParagraphSeparators inserts a <br> between directly adjacent, non-empty
// paragraph blocks so that HTML supplied to the CLI renders with visible
// paragraph spacing.
// insertParagraphSeparators puts an empty separator paragraph between directly
// adjacent, non-empty paragraph blocks so that HTML supplied to the CLI renders
// with visible paragraph spacing.
//
// Basecamp's rich text relies on explicit separator nodes for paragraph
// spacing, not CSS margins: contiguous <p>A</p><p>B</p> renders squished. The
Expand All @@ -387,10 +402,12 @@ func MarkdownToHTML(md string) string {
// idempotent: a boundary that already carries a separator — a bare <br> between
// the paragraphs, or an empty separator paragraph (<p><br></p> or <p></p>) on
// either side — is left untouched, so running it on already-separated content
// (including Basecamp editor output) is a no-op. Only directly adjacent <p>
// blocks are separated; anything between them (whitespace excepted), such as a
// heading, list, or attachment, already provides its own break and is left
// alone.
// (including Basecamp editor output) is a no-op. Separators the caller supplied
// are left as they came: this matching is not nesting-aware, and a <br> the
// caller put between two paragraphs inside a blockquote is legal inline content
// that survives editing. Only directly adjacent <p> blocks are separated;
// anything else between them, such as a heading, list, or attachment, already
// provides its own break and is left alone.
func insertParagraphSeparators(s string) string {
locs := reP.FindAllStringIndex(s, -1)
if len(locs) < 2 {
Expand Down Expand Up @@ -418,7 +435,7 @@ func insertParagraphSeparators(s string) string {
gap := s[end:nextStart]
if !empty[i] && !empty[i+1] && strings.TrimSpace(gap) == "" {
b.WriteString(gap)
b.WriteString("<br>")
b.WriteString(paragraphSeparator)
cursor = nextStart
}
}
Expand All @@ -430,7 +447,7 @@ func insertParagraphSeparators(s string) string {
// i.e. it is empty or contains only <br> tags and whitespace, including
// non-breaking-space entities (&nbsp;, &#160;, &#xa0;) that rich text editors
// commonly use for blank separator lines. Such paragraphs act as separators, so
// no additional <br> is inserted adjacent to them.
// no additional one is inserted adjacent to them.
func isEmptyParagraph(block string) bool {
m := reP.FindStringSubmatch(block)
if m == nil {
Expand Down
68 changes: 47 additions & 21 deletions internal/richtext/richtext_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "list followed by blank line then paragraph",
input: "- Item 1\n- Item 2\n\nFollowing paragraph.",
expected: "<ul>\n<li>Item 1</li>\n<li>Item 2</li>\n</ul>\n<br>\n<p>Following paragraph.</p>",
expected: "<ul>\n<li>Item 1</li>\n<li>Item 2</li>\n</ul>\n<p><br></p>\n<p>Following paragraph.</p>",
},
{
// CommonMark §5.4: "After" is a lazy continuation of the second list item.
Expand Down Expand Up @@ -128,7 +128,7 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "mixed formatting",
input: "# Title\n\nThis is **bold** and *italic* and `code`.",
expected: "<h1>Title</h1>\n<br>\n<p>This is <strong>bold</strong> and <em>italic</em> and <code>code</code>.</p>",
expected: "<h1>Title</h1>\n<p><br></p>\n<p>This is <strong>bold</strong> and <em>italic</em> and <code>code</code>.</p>",
},
{
name: "escapes HTML",
Expand All @@ -143,12 +143,12 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "paragraph spacing with blank line",
input: "First paragraph\n\nSecond paragraph",
expected: "<p>First paragraph</p>\n<br>\n<p>Second paragraph</p>",
expected: "<p>First paragraph</p>\n<p><br></p>\n<p>Second paragraph</p>",
},
{
name: "multiple blank lines collapse to one break",
input: "First\n\n\n\nSecond",
expected: "<p>First</p>\n<br>\n<p>Second</p>",
expected: "<p>First</p>\n<p><br></p>\n<p>Second</p>",
},
{
name: "consecutive lines join into one paragraph",
Expand All @@ -158,12 +158,12 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "blank line before list",
input: "Intro\n\n- Item 1\n- Item 2",
expected: "<p>Intro</p>\n<br>\n<ul>\n<li>Item 1</li>\n<li>Item 2</li>\n</ul>",
expected: "<p>Intro</p>\n<p><br></p>\n<ul>\n<li>Item 1</li>\n<li>Item 2</li>\n</ul>",
},
{
name: "blank line before code block",
input: "Intro\n\n```\ncode\n```",
expected: "<p>Intro</p>\n<br>\n<pre><code>code\n</code></pre>",
expected: "<p>Intro</p>\n<p><br></p>\n<pre><code>code\n</code></pre>",
},
{
name: "leading blank lines ignored",
Expand All @@ -173,12 +173,12 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "blank line before blockquote",
input: "Intro\n\n> A quote",
expected: "<p>Intro</p>\n<br>\n<blockquote>A quote</blockquote>",
expected: "<p>Intro</p>\n<p><br></p>\n<blockquote>A quote</blockquote>",
},
{
name: "blank line before horizontal rule",
input: "Intro\n\n---",
expected: "<p>Intro</p>\n<br>\n<hr>",
expected: "<p>Intro</p>\n<p><br></p>\n<hr>",
},
{
name: "heading flushes accumulated paragraph",
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestMarkdownToHTML(t *testing.T) {
{
name: "fenced code block containing HTML tags is converted",
input: "intro\n\n```\n<div>hello</div>\n```",
expected: "<p>intro</p>\n<br>\n<pre><code>&lt;div&gt;hello&lt;/div&gt;\n</code></pre>",
expected: "<p>intro</p>\n<p><br></p>\n<pre><code>&lt;div&gt;hello&lt;/div&gt;\n</code></pre>",
},
}

Expand Down Expand Up @@ -2223,33 +2223,41 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) {
{
name: "two contiguous paragraphs get a separator",
input: "<p>Line 1</p><p>Line 2</p>",
expected: "<p>Line 1</p><br><p>Line 2</p>",
expected: "<p>Line 1</p><p><br></p><p>Line 2</p>",
},
{
name: "three contiguous paragraphs get separators between each",
input: "<p>A</p><p>B</p><p>C</p>",
expected: "<p>A</p><br><p>B</p><br><p>C</p>",
expected: "<p>A</p><p><br></p><p>B</p><p><br></p><p>C</p>",
},
{
name: "paragraphs with attributes are separated",
input: `<p dir="auto">A</p><p dir="auto">B</p>`,
expected: `<p dir="auto">A</p><br><p dir="auto">B</p>`,
expected: `<p dir="auto">A</p><p><br></p><p dir="auto">B</p>`,
},
{
name: "whitespace-only gap is preserved and separator added",
input: "<p>A</p>\n<p>B</p>",
expected: "<p>A</p>\n<br><p>B</p>",
expected: "<p>A</p>\n<p><br></p><p>B</p>",
},
{
name: "existing bare br separator is left untouched (idempotent)",
name: "caller-supplied bare br separator is left untouched",
input: "<p>A</p><br><p>B</p>",
expected: "<p>A</p><br><p>B</p>",
},
{
name: "empty separator paragraph is left untouched (Lexxy canonical)",
name: "separator paragraph is left untouched (editor canonical)",
input: "<p>A</p><p><br></p><p>B</p>",
expected: "<p>A</p><p><br></p><p>B</p>",
},
{
// A <br> between paragraphs nested in a blockquote is inline content,
// which the editor keeps — matching is not nesting-aware, so leaving
// caller-supplied separators alone is what protects it.
name: "bare br inside a blockquote is left untouched",
input: "<blockquote><p>A</p><br><p>B</p></blockquote>",
expected: "<blockquote><p>A</p><br><p>B</p></blockquote>",
},
{
name: "empty paragraph separator is left untouched",
input: "<p>A</p><p></p><p>B</p>",
Expand Down Expand Up @@ -2293,7 +2301,7 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) {
{
name: "paragraph with inline br is still non-empty and separated",
input: "<p>A<br>C</p><p>B</p>",
expected: "<p>A<br>C</p><br><p>B</p>",
expected: "<p>A<br>C</p><p><br></p><p>B</p>",
},
{
name: "leading empty paragraph is left alone",
Expand All @@ -2303,7 +2311,7 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) {
{
name: "mix of separated and contiguous only fills the gap that lacks a separator",
input: "<p>A</p><p>B</p><br><p>C</p>",
expected: "<p>A</p><br><p>B</p><br><p>C</p>",
expected: "<p>A</p><p><br></p><p>B</p><br><p>C</p>",
},
}

Expand All @@ -2325,6 +2333,7 @@ func TestMarkdownToHTMLParagraphSeparatorsIdempotent(t *testing.T) {
`<p dir="auto">A</p><p dir="auto">B</p>`,
"<p>A</p>\n<p>B</p>",
"<p>A</p><p><br></p><p>B</p>",
"<p>A</p><br><p>B</p>",
"<p>A</p><h3>H</h3><p>B</p>",
}

Expand All @@ -2345,10 +2354,27 @@ func TestMarkdownToHTMLParagraphSeparatorsMatchMarkdownPath(t *testing.T) {
fromMarkdown := MarkdownToHTML("Line 1\n\nLine 2")
fromHTML := MarkdownToHTML("<p>Line 1</p><p>Line 2</p>")

if !strings.Contains(fromMarkdown, "<br>") {
t.Fatalf("markdown path unexpectedly produced no <br>: %q", fromMarkdown)
if fromMarkdown != "<p>Line 1</p>\n<p><br></p>\n<p>Line 2</p>" {
t.Errorf("markdown path = %q", fromMarkdown)
}
if fromHTML != "<p>Line 1</p><p><br></p><p>Line 2</p>" {
t.Errorf("HTML path = %q, want %q", fromHTML, "<p>Line 1</p><p><br></p><p>Line 2</p>")
}
}

// Basecamp's editor discards a bare top-level <br> the first time the content is
// edited, collapsing the spacing. No blank line between blocks may rely on one.
func TestMarkdownToHTMLEmitsNoBareTopLevelBreaks(t *testing.T) {
markdown := "Para one.\n\nPara two.\n\n## Heading\n\n- a\n- b\n\n> A quote\n\n```\ncode\n```\n\n---\n\nClosing."

html := MarkdownToHTML(markdown)

for _, block := range []string{"<p>", "<h2>", "<ul>", "<blockquote>", "<pre>", "<hr>"} {
if strings.Contains(html, "<br>\n"+block) {
t.Errorf("bare <br> separator before %s in %q", block, html)
}
}
if fromHTML != "<p>Line 1</p><br><p>Line 2</p>" {
t.Errorf("HTML path = %q, want %q", fromHTML, "<p>Line 1</p><br><p>Line 2</p>")
if want := strings.Count(markdown, "\n\n"); strings.Count(html, paragraphSeparator) != want {
t.Errorf("got %d separator paragraphs, want %d in %q", strings.Count(html, paragraphSeparator), want, html)
}
}
Loading