diff --git a/internal/commands/chat_test.go b/internal/commands/chat_test.go index 8d126f7d5..8631cada2 100644 --- a/internal/commands/chat_test.go +++ b/internal/commands/chat_test.go @@ -1238,7 +1238,8 @@ func TestChatUpdatePlainTextOptOut(t *testing.T) { // TestChatUpdatePlainTextSerializesLiterally is the F2 literal case: text/plain // serializes via richtext.PlainToHTML, so HTML-special characters are escaped -// (rendered as typed, not interpreted) and line breaks are preserved as
. +// (rendered as typed, not interpreted) and line breaks are preserved as
+// inside the paragraph, where Basecamp's editor keeps them. func TestChatUpdatePlainTextSerializesLiterally(t *testing.T) { t.Setenv("BASECAMP_NO_KEYRING", "1") @@ -1255,10 +1256,8 @@ func TestChatUpdatePlainTextSerializesLiterally(t *testing.T) { content, ok := requestBody["content"].(string) require.True(t, ok) - assert.Contains(t, content, "<strong>x</strong>", - "HTML-special characters should be escaped, not interpreted as markup") - assert.Contains(t, content, "
", - "line breaks should be preserved as
") + assert.Contains(t, content, "<strong>x</strong>
line2

", + "HTML-special characters should be escaped and the line break kept inline within the paragraph") assert.NotContains(t, content, "bc-attachment", "text/plain skips mention resolution") } diff --git a/internal/richtext/richtext.go b/internal/richtext/richtext.go index 33359610e..675a67436 100644 --- a/internal/richtext/richtext.go +++ b/internal/richtext/richtext.go @@ -155,7 +155,15 @@ var mdConverter = goldmark.New( ), ) -// TrixBreak is a custom block node that renders as
\n for Trix paragraph spacing. +// paragraphSeparator is the blank line Basecamp's editor itself stores between +// two blocks. A bare top-level
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 = "


" + +// TrixBreak is a custom block node that renders the blank line between blocks: +// an empty paragraph at the top level, a
inside a block (see +// renderTrixBreak). type TrixBreak struct{ ast.BaseBlock } // KindTrixBreak is the node kind for TrixBreak. @@ -350,11 +358,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
for +// one inside a block. Only the top level needs a block-level separator: +// a
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("
\n") + if parent := node.Parent(); parent != nil && parent.Kind() == ast.KindDocument { + _, _ = w.WriteString(paragraphSeparator + "\n") + } else { + _, _ = w.WriteString("
\n") + } return ast.WalkContinue, nil } @@ -369,8 +384,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
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 "" @@ -391,9 +406,9 @@ func MarkdownToHTML(md string) string { return strings.TrimSpace(buf.String()) } -// insertParagraphSeparators inserts a
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

A

B

renders squished. The @@ -408,10 +423,12 @@ func MarkdownToHTML(md string) string { // idempotent: a boundary that already carries a separator — a bare
between // the paragraphs, or an empty separator paragraph (


or

) on // either side — is left untouched, so running it on already-separated content -// (including Basecamp editor output) is a no-op. Only directly adjacent

-// 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
the +// caller put between two paragraphs inside a blockquote is legal inline content +// that survives editing. Only directly adjacent

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 { @@ -439,7 +456,7 @@ func insertParagraphSeparators(s string) string { gap := s[end:nextStart] if !empty[i] && !empty[i+1] && strings.TrimSpace(gap) == "" { b.WriteString(gap) - b.WriteString("
") + b.WriteString(paragraphSeparator) cursor = nextStart } } @@ -451,7 +468,7 @@ func insertParagraphSeparators(s string) string { // i.e. it is empty or contains only
tags and whitespace, including // non-breaking-space entities ( ,  ,  ) that rich text editors // commonly use for blank separator lines. Such paragraphs act as separators, so -// no additional
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 { @@ -462,20 +479,60 @@ func isEmptyParagraph(block string) bool { return strings.TrimSpace(inner) == "" } -// PlainToHTML serializes literal plain text as Basecamp rich text: HTML-special +// PlainToHTML serializes literal plain text as Basecamp rich text. HTML-special // characters are escaped so they render as typed (not interpreted as markup), -// and line breaks are preserved as
so multi-line input keeps its shape. -// Windows CRLF and bare CR are normalized to LF first so a single
is -// emitted per line break. Use this when the caller wants the text delivered -// verbatim to an endpoint that always stores rich text. +// and the line structure is kept in the one shape Basecamp's editor preserves +// across an edit: each run of non-blank lines becomes a

with single line +// breaks as
between its lines, and each blank line between runs becomes an +// empty paragraph (paragraphSeparator). +// +// The editor drops a root-level
on import and keeps a
only when it +// sits between two text runs inside a block, so neither bare
between +// lines nor

for a blank line survives the first edit in Basecamp. +// Leading and trailing blank lines are dropped — they have no paragraphs to +// separate — matching the Markdown path; a whitespace-only line counts as +// blank. Windows CRLF and bare CR are normalized to LF first. Use this when the +// caller wants the text delivered verbatim to an endpoint that always stores +// rich text. func PlainToHTML(s string) string { - if s == "" { - return "" - } s = strings.ReplaceAll(s, "\r\n", "\n") s = strings.ReplaceAll(s, "\r", "\n") - s = escapeHTML(s) - return strings.ReplaceAll(s, "\n", "
") + lines := trimBlankLines(strings.Split(escapeHTML(s), "\n")) + + var b strings.Builder + var run []string + flush := func() { + if len(run) > 0 { + b.WriteString("

" + strings.Join(run, "
") + "

") + run = run[:0] + } + } + for _, line := range lines { + if isBlankLine(line) { + flush() + b.WriteString(paragraphSeparator) + } else { + run = append(run, line) + } + } + flush() + return b.String() +} + +// trimBlankLines drops leading and trailing blank lines. +func trimBlankLines(lines []string) []string { + start, end := 0, len(lines) + for start < end && isBlankLine(lines[start]) { + start++ + } + for end > start && isBlankLine(lines[end-1]) { + end-- + } + return lines[start:end] +} + +func isBlankLine(line string) bool { + return strings.TrimSpace(line) == "" } // escapeHTML escapes special HTML characters. diff --git a/internal/richtext/richtext_test.go b/internal/richtext/richtext_test.go index ac03d2894..007f453b4 100644 --- a/internal/richtext/richtext_test.go +++ b/internal/richtext/richtext_test.go @@ -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: "\n
\n

Following paragraph.

", + expected: "\n


\n

Following paragraph.

", }, { // CommonMark §5.4: "After" is a lazy continuation of the second list item. @@ -128,7 +128,7 @@ func TestMarkdownToHTML(t *testing.T) { { name: "mixed formatting", input: "# Title\n\nThis is **bold** and *italic* and `code`.", - expected: "

Title

\n
\n

This is bold and italic and code.

", + expected: "

Title

\n


\n

This is bold and italic and code.

", }, { name: "escapes HTML", @@ -143,12 +143,12 @@ func TestMarkdownToHTML(t *testing.T) { { name: "paragraph spacing with blank line", input: "First paragraph\n\nSecond paragraph", - expected: "

First paragraph

\n
\n

Second paragraph

", + expected: "

First paragraph

\n


\n

Second paragraph

", }, { name: "multiple blank lines collapse to one break", input: "First\n\n\n\nSecond", - expected: "

First

\n
\n

Second

", + expected: "

First

\n


\n

Second

", }, { name: "consecutive lines join into one paragraph", @@ -158,12 +158,12 @@ func TestMarkdownToHTML(t *testing.T) { { name: "blank line before list", input: "Intro\n\n- Item 1\n- Item 2", - expected: "

Intro

\n
\n", + expected: "

Intro

\n


\n", }, { name: "blank line before code block", input: "Intro\n\n```\ncode\n```", - expected: "

Intro

\n
\n
code\n
", + expected: "

Intro

\n


\n
code\n
", }, { name: "leading blank lines ignored", @@ -173,12 +173,12 @@ func TestMarkdownToHTML(t *testing.T) { { name: "blank line before blockquote", input: "Intro\n\n> A quote", - expected: "

Intro

\n
\n
A quote
", + expected: "

Intro

\n


\n
A quote
", }, { name: "blank line before horizontal rule", input: "Intro\n\n---", - expected: "

Intro

\n
\n
", + expected: "

Intro

\n


\n
", }, { name: "heading flushes accumulated paragraph", @@ -214,7 +214,7 @@ func TestMarkdownToHTML(t *testing.T) { { name: "fenced code block containing HTML tags is converted", input: "intro\n\n```\n
hello
\n```", - expected: "

intro

\n
\n
<div>hello</div>\n
", + expected: "

intro

\n


\n
<div>hello</div>\n
", }, { // Issue #405: a GFM table renders as a bare (BC3's @@ -2313,33 +2313,53 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) { { name: "two contiguous paragraphs get a separator", input: "

Line 1

Line 2

", - expected: "

Line 1


Line 2

", + expected: "

Line 1


Line 2

", }, { name: "three contiguous paragraphs get separators between each", input: "

A

B

C

", - expected: "

A


B


C

", + expected: "

A


B


C

", }, { name: "paragraphs with attributes are separated", input: `

A

B

`, - expected: `

A


B

`, + expected: `

A


B

`, }, { name: "whitespace-only gap is preserved and separator added", input: "

A

\n

B

", - expected: "

A

\n

B

", + expected: "

A

\n


B

", }, { - name: "existing bare br separator is left untouched (idempotent)", + name: "caller-supplied bare br separator is left untouched", input: "

A


B

", expected: "

A


B

", }, { - name: "empty separator paragraph is left untouched (Lexxy canonical)", + name: "separator paragraph is left untouched (editor canonical)", input: "

A


B

", expected: "

A


B

", }, + { + // A
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: "

A


B

", + expected: "

A


B

", + }, + { + // Contiguous paragraphs nested in a blockquote get the same separator + // deliberately: lexxy quotes hold

children, and an empty paragraph + // is exactly what the editor itself authors for a blank line inside a + // quote, so it round-trips verbatim (verified against lexical 0.44 + // import/export). A bare
at that position also survives but is a + // shape the editor never authors, and inside

  • it balloons to + //


    on first edit while the empty paragraph normalizes away. + name: "contiguous paragraphs inside a blockquote get a separator", + input: "

    A

    B

    ", + expected: "

    A


    B

    ", + }, { name: "empty paragraph separator is left untouched", input: "

    A

    B

    ", @@ -2383,7 +2403,7 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) { { name: "paragraph with inline br is still non-empty and separated", input: "

    A
    C

    B

    ", - expected: "

    A
    C


    B

    ", + expected: "

    A
    C


    B

    ", }, { name: "leading empty paragraph is left alone", @@ -2393,7 +2413,7 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) { { name: "mix of separated and contiguous only fills the gap that lacks a separator", input: "

    A

    B


    C

    ", - expected: "

    A


    B


    C

    ", + expected: "

    A


    B


    C

    ", }, } @@ -2415,6 +2435,7 @@ func TestMarkdownToHTMLParagraphSeparatorsIdempotent(t *testing.T) { `

    A

    B

    `, "

    A

    \n

    B

    ", "

    A


    B

    ", + "

    A


    B

    ", "

    A

    H

    B

    ", } @@ -2435,11 +2456,28 @@ func TestMarkdownToHTMLParagraphSeparatorsMatchMarkdownPath(t *testing.T) { fromMarkdown := MarkdownToHTML("Line 1\n\nLine 2") fromHTML := MarkdownToHTML("

    Line 1

    Line 2

    ") - if !strings.Contains(fromMarkdown, "
    ") { - t.Fatalf("markdown path unexpectedly produced no
    : %q", fromMarkdown) + if fromMarkdown != "

    Line 1

    \n


    \n

    Line 2

    " { + t.Errorf("markdown path = %q", fromMarkdown) + } + if fromHTML != "

    Line 1


    Line 2

    " { + t.Errorf("HTML path = %q, want %q", fromHTML, "

    Line 1


    Line 2

    ") + } +} + +// Basecamp's editor discards a bare top-level
    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{"

    ", "

    ", "