From cb57e6dca2f867670d347aab8ba20065bbad6ce6 Mon Sep 17 00:00:00 2001
From: Jorge Manrubia A B
-// blocks are separated; anything between them (whitespace excepted), such as a
-// heading, list, or attachment, already provides its own break and is left
-// alone.
+// A boundary already carrying an empty separator paragraph ( 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 {
@@ -416,9 +431,9 @@ func insertParagraphSeparators(s string) string {
nextStart := locs[i+1][0]
gap := s[end:nextStart]
- if !empty[i] && !empty[i+1] && strings.TrimSpace(gap) == "" {
- b.WriteString(gap)
- b.WriteString(" ... Following paragraph. Following paragraph. This is bold and italic and This is bold and italic and First paragraph Second paragraph First paragraph Second paragraph First Second First Second Intro Intro Intro Intro Intro Intro Intro Intro intro intro Line 1 Line 2 Line 1 Line 2 Line 1 Line 2 A B C A B C A B C A B A B A B A B A B A B A B A B A B A B A B A B A B A B A A B C A B C A B C A B A B A B A B A B Line 1 Line 2 Line 1 Line 2 Line 1 Line 2 Line 1 Line 2 ", " Line 1 Line 2 Line 1 Line 2 blocks are separated; anything else between them,
-// such as a heading, list, or attachment, already provides its own break and is
-// left alone.
+// The transform is byte-preserving apart from the inserted separators and is
+// idempotent: a boundary that already carries a separator — a bare 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 {
@@ -431,8 +433,8 @@ func insertParagraphSeparators(s string) string {
nextStart := locs[i+1][0]
gap := s[end:nextStart]
- if !empty[i] && !empty[i+1] && isSeparatorGap(gap) {
- b.WriteString(reBR.ReplaceAllString(gap, ""))
+ if !empty[i] && !empty[i+1] && strings.TrimSpace(gap) == "" {
+ b.WriteString(gap)
b.WriteString(paragraphSeparator)
cursor = nextStart
}
@@ -441,13 +443,6 @@ func insertParagraphSeparators(s string) string {
return b.String()
}
-// isSeparatorGap reports whether the markup between two paragraph blocks holds
-// nothing but whitespace and bare ... A B A B A B A B A B A B A B A B A B A A B C A B C A B C
with with single line
+// breaks as " + strings.Join(run, " hello world <strong>x</strong> & y line1 line1 line1 a b a b a c a b a a ping @Jane.Smith now <b>a</b>
, and
the raw-HTML passthrough inserted one between adjacent paragraphs. A bare
top-level
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
: there it is inline content, which the editor keeps. Bare
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.
---
internal/richtext/richtext.go | 62 +++++++++++++++++++----------
internal/richtext/richtext_test.go | 64 +++++++++++++++++++-----------
2 files changed, 83 insertions(+), 43 deletions(-)
diff --git a/internal/richtext/richtext.go b/internal/richtext/richtext.go
index b8a9054ba..aea01fedb 100644
--- a/internal/richtext/richtext.go
+++ b/internal/richtext/richtext.go
@@ -134,7 +134,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 = "
inside a block (see
+// renderTrixBreak).
type TrixBreak struct{ ast.BaseBlock }
// KindTrixBreak is the node kind for TrixBreak.
@@ -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
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
}
@@ -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
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 ""
@@ -370,9 +385,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
between
-// the paragraphs, or an empty separator paragraph (
tags is rewritten to the separator paragraph: the editor drops those on
+// the first edit, so leaving them would keep the spacing they express fragile.
+// Only directly adjacent
")
+ if !empty[i] && !empty[i+1] && isSeparatorGap(gap) {
+ b.WriteString(reBR.ReplaceAllString(gap, ""))
+ b.WriteString(paragraphSeparator)
cursor = nextStart
}
}
@@ -426,11 +441,18 @@ func insertParagraphSeparators(s string) string {
return b.String()
}
+// isSeparatorGap reports whether the markup between two paragraph blocks holds
+// nothing but whitespace and bare
tags — i.e. whatever spacing it expresses
+// can be replaced by a separator paragraph.
+func isSeparatorGap(gap string) bool {
+ return strings.TrimSpace(reBR.ReplaceAllString(gap, "")) == ""
+}
+
// isEmptyParagraph reports whether a
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 {
diff --git a/internal/richtext/richtext_test.go b/internal/richtext/richtext_test.go
index 8fb3246e3..15d1cfcb8 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
\n\n
\nTitle
\n
\ncode.Title
\ncode.
\n
\n
\n\n
",
+ expected: "\n
",
},
{
name: "blank line before code block",
input: "Intro\n\n```\ncode\n```",
- expected: "
\n
",
+ expected: "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: "code\n
\nA quote
",
+ expected: "A quote
",
},
{
name: "blank line before horizontal rule",
input: "Intro\n\n---",
- expected: "
\n
",
+ expected: "
",
},
{
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
\n
",
+ expected: "<div>hello</div>\n
",
},
}
@@ -2223,30 +2223,30 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) {
{
name: "two contiguous paragraphs get a separator",
input: "<div>hello</div>\n
C
C
CH
") {
- t.Fatalf("markdown path unexpectedly produced no
: %q", fromMarkdown)
+ if fromMarkdown != "
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{"", "
", "
", "
", "
"} {
+ if strings.Contains(html, "
\n"+block) {
+ t.Errorf("bare
separator before %s in %q", block, html)
+ }
}
- if fromHTML != "
boundary reached into blockquotes: the paragraph
matching is regex-based, not nesting-aware, and a
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.
---
internal/richtext/richtext.go | 29 ++++++++++++-----------------
internal/richtext/richtext_test.go | 16 ++++++++++++----
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/internal/richtext/richtext.go b/internal/richtext/richtext.go
index aea01fedb..691c6c271 100644
--- a/internal/richtext/richtext.go
+++ b/internal/richtext/richtext.go
@@ -398,14 +398,16 @@ func MarkdownToHTML(md string) string {
// distinction), so contiguous paragraphs from HTML input are treated as
// separate paragraphs.
//
-// A boundary already carrying an empty separator paragraph (
tags is rewritten to the separator paragraph: the editor drops those on
-// the first edit, so leaving them would keep the spacing they express fragile.
-// Only directly adjacent
between
+// the paragraphs, or an empty separator paragraph (
the
+// caller put between two paragraphs inside a blockquote is legal inline content
+// that survives editing. Only directly adjacent
tags — i.e. whatever spacing it expresses
-// can be replaced by a separator paragraph.
-func isSeparatorGap(gap string) bool {
- return strings.TrimSpace(reBR.ReplaceAllString(gap, "")) == ""
-}
-
// isEmptyParagraph reports whether a
tags and whitespace, including
// non-breaking-space entities ( , , ) that rich text editors
diff --git a/internal/richtext/richtext_test.go b/internal/richtext/richtext_test.go
index 15d1cfcb8..c213a5731 100644
--- a/internal/richtext/richtext_test.go
+++ b/internal/richtext/richtext_test.go
@@ -2241,15 +2241,23 @@ func TestMarkdownToHTMLInsertsParagraphSeparators(t *testing.T) {
expected: "
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: "
",
+ expected: "
",
+ },
{
name: "empty paragraph separator is left untouched",
input: "
s (line
line
line) for
`chat update --content-type text/plain`. Basecamp's editor drops every
root-level
on import and keeps a
only when it sits between two text
runs inside a block, so the first edit in Basecamp squashed single line breaks
into separate paragraphs and lost blank lines entirely — the same bug class as
the Markdown separator, from a different producer.
Emit the one shape the editor preserves: each run of non-blank lines is a
between its lines, and each interior blank line is its own empty
paragraph (the same paragraphSeparator the Markdown path uses). Leading and
trailing blank lines are dropped, matching the Markdown path; a
whitespace-only line counts as blank. The result also round-trips through
HTMLToMarkdown unchanged.
---
internal/commands/chat_test.go | 9 ++---
internal/richtext/richtext.go | 60 +++++++++++++++++++++++++-----
internal/richtext/richtext_test.go | 34 +++++++++++++----
3 files changed, 81 insertions(+), 22 deletions(-)
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
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
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("
") + "
", "line1\nline2", "line1
line2"},
- {"collapses CRLF to a single break", "line1\r\nline2", "line1
line2"},
- {"normalizes bare CR", "line1\rline2", "line1
line2"},
- {"leaves @mentions literal", "ping @Jane.Smith now", "ping @Jane.Smith now"},
- {"escapes then breaks multiline markup", "a\nb", "<b>a</b>
<i>b</i>"},
+ {"plain text", "hello world", "
line2
line2
line2
b
<i>b</i>
only between two text runs inside a block;
+ // a doubled
or one at the root is dropped on the first edit.
+ if strings.Contains(got, "
") || strings.Contains(got, "
pairs nested in a blockquote,
where renderTrixBreak's nested rule emits a bare
. That rule covers inline
breaks between text runs; between two
children of a quote the empty
paragraph is the shape the editor itself authors for a blank line, so it
round-trips verbatim (verified against lexical 0.44, lexxy 0.9.29's engine,
with lexxy's import filters). A bare
at that position survives too but is
never editor-authored, and inside
", expected: "A
B
", }, + { + // Contiguous paragraphs nested in a blockquote get the same separator + // deliberately: lexxy quotes holdA
B
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
", + expected: "A
B
", + }, { name: "empty paragraph separator is left untouched", input: "A
B
A
B
",