From 11129633771569d1916df603b426a5fdd0521d8e Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 10:52:16 -0400 Subject: [PATCH 1/2] fix(convert): move a mark's edge whitespace outside its delimiters read trimmed the whitespace at the edge of a bold, italic or strikethrough span and did not put it back, so "bold next" read as "**bold**next" and published as one word. The editor leaves a space typed at the end of a bold run inside the run, so this shape is common. The whitespace now moves outside the delimiters: "**bold** next". It cannot stay inside, since CommonMark refuses "**bold **" as emphasis. A mark holding only whitespace rendered as "****" and now renders as the whitespace. A
at a mark's edge was dropped and now moves out with the rest. Where moved whitespace meets whitespace beside the mark, one space is kept, and whitespace before a hard break is trimmed, since storage loses both on the next publish and the Markdown would not be a fixed point. The last also fixes "a
" reading as three trailing spaces rather than two. The split-marks golden recorded the bug: its partly-bold link case lost the space after "a". Fixes #204. --- internal/convert/storage_to_md.go | 64 +++++++++++++++++-- internal/convert/storage_to_md_test.go | 2 +- .../mark-edge-whitespace/input.storage | 11 ++++ .../storage2md/mark-edge-whitespace/output.md | 26 ++++++++ .../testdata/storage2md/split-marks/output.md | 2 +- 5 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage create mode 100644 internal/convert/testdata/storage2md/mark-edge-whitespace/output.md diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 10403f4..697d51d 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -24,6 +24,7 @@ import ( "regexp" "sort" "strings" + "unicode" ) // calloutMacroInverse maps a Confluence callout macro back to a GitHub alert @@ -638,9 +639,31 @@ func (r *mdRenderer) renderCallout(n *snode, alert string) string { // renderInlineChildren renders a node's children as a single inline string. func (r *mdRenderer) renderInlineChildren(n *snode) string { - var b strings.Builder + return strings.TrimSpace(r.renderInlineRun(n)) +} + +// renderInlineRun is renderInlineChildren without the trim, for a mark, which +// must see the whitespace at its own edges to move it outside its delimiters. +func (r *mdRenderer) renderInlineRun(n *snode) string { + var s string for _, k := range coalesceSplitMarks(n.kids) { part := r.renderInline(k) + // A mark moves its edge whitespace outside its delimiters, so a space + // there can meet a space in the text beside it. One is kept: storage + // collapses a run of spaces anyway, so the second would only reach the + // Markdown on this read and be gone on the next, and the Markdown would + // not be a fixed point. + if strings.HasSuffix(s, " ") && strings.HasPrefix(part, " ") && !opensWithBreak(part) { + part = strings.TrimLeft(part, " ") + } + // Likewise, whitespace before a hard break is gone on the next read, + // since publishing ends the line at the break: "a
" read as + // "a " (three spaces) and read back as two. The break is written as + // exactly the two spaces that make it one. + if opensWithBreak(part) { + s = strings.TrimRight(s, " \t") + part = hardBreak + strings.TrimLeft(part, " \t")[1:] + } // Storage is XHTML and its newlines are insignificant, so // "
\nSecond" is the ordinary spelling -- and that newline // normalizes to a space, landing immediately after the two-space hard @@ -651,12 +674,39 @@ func (r *mdRenderer) renderInlineChildren(n *snode) string { // *are* the next hard break. Found by the round-trip property test. // Never trim a hard break itself: two of them in a row are two blank // line-endings, and its own leading spaces are what make it one. - if k.name != "br" && strings.HasSuffix(b.String(), hardBreak) { + if !opensWithBreak(part) && strings.HasSuffix(s, hardBreak) { part = strings.TrimLeft(part, " \t") } - b.WriteString(part) + s += part } - return strings.TrimSpace(b.String()) + return s +} + +// opensWithBreak reports whether an inline part begins with a hard break -- +// a
itself, or a mark that moved one out from its leading edge -- whose +// leading spaces are what make it a break and must never be trimmed. +func opensWithBreak(part string) bool { + return strings.HasPrefix(strings.TrimLeft(part, " \t"), "\n") +} + +// renderMark renders a bold, italic or strikethrough span, moving whitespace +// at its edges outside the delimiters (#204). It cannot stay inside, since +// CommonMark refuses "**bold **" as emphasis -- a closing delimiter preceded by +// whitespace does not close -- and it cannot be dropped, since the editor +// leaves a space typed at the end of a bold run inside the run, and dropping +// it published "**bold**next" as one word. Unicode whitespace counts, because +// CommonMark's flanking rule counts it: a trailing no-break space refuses the +// delimiter as surely as a space does. A mark holding only whitespace has +// nothing Markdown can mark, so it renders as that whitespace. +func (r *mdRenderer) renderMark(n *snode, delim string) string { + s := r.renderInlineRun(n) + inner := strings.TrimFunc(s, unicode.IsSpace) + if inner == "" { + return s + } + lead := s[:len(s)-len(strings.TrimLeftFunc(s, unicode.IsSpace))] + trail := s[len(strings.TrimRightFunc(s, unicode.IsSpace)):] + return lead + delim + inner + delim + trail } // hardBreak is Markdown's two-space line break, as renderInline emits it for a @@ -821,13 +871,13 @@ func (r *mdRenderer) renderInline(n *snode) string { } switch n.name { case "strong", "b": - return "**" + r.renderInlineChildren(n) + "**" + return r.renderMark(n, "**") case "em", "i": - return "*" + r.renderInlineChildren(n) + "*" + return r.renderMark(n, "*") case "code": return "`" + textContent(n) + "`" case "del", "s", "strike": - return "~~" + r.renderInlineChildren(n) + "~~" + return r.renderMark(n, "~~") case "br": return hardBreak case "a": diff --git a/internal/convert/storage_to_md_test.go b/internal/convert/storage_to_md_test.go index 8b80927..ba0f83e 100644 --- a/internal/convert/storage_to_md_test.go +++ b/internal/convert/storage_to_md_test.go @@ -350,7 +350,7 @@ func TestStorageToMarkdownCoalescesSplitMarks(t *testing.T) { }, "link only partly bold does not merge": { in: `

a bc

`, - want: "**a**[b**c**](https://example.com)\n", + want: "**a** [b**c**](https://example.com)\n", }, } for name, tc := range tests { diff --git a/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage b/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage new file mode 100644 index 0000000..03e52a1 --- /dev/null +++ b/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage @@ -0,0 +1,11 @@ +

A trailing space inside bold: bold next.

+

Inside italic: x y, and strikethrough: lost for 13.5h.

+

A leading space: a bold word.

+

A space on both sides of the edge: bold next.

+

A no-break space: a b.

+

Nested marks: n x.

+

A mark holding only a space: x y.

+

A hard break at the end of a mark: a
b.

+

A hard break at the start of a mark, after a space: a
b.

+

A space before a hard break: a
b.

+
ItemTime
lost for 13.5h1
diff --git a/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md b/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md new file mode 100644 index 0000000..7ba7fe9 --- /dev/null +++ b/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md @@ -0,0 +1,26 @@ +A trailing space inside bold: **bold** next. + +Inside italic: *x* y, and strikethrough: ~~lost for~~ 13.5h. + +A leading space: a **bold** word. + +A space on both sides of the edge: **bold** next. + +A no-break space: **a** b. + +Nested marks: ***n*** x. + +A mark holding only a space: x y. + +A hard break at the end of a mark: **a** +b. + +A hard break at the start of a mark, after a space: a +**b**. + +A space before a hard break: a +b. + +| Item | Time | +| --- | --- | +| ~~lost for~~ 13.5h | 1 | diff --git a/internal/convert/testdata/storage2md/split-marks/output.md b/internal/convert/testdata/storage2md/split-marks/output.md index d60c96f..d3ff399 100644 --- a/internal/convert/testdata/storage2md/split-marks/output.md +++ b/internal/convert/testdata/storage2md/split-marks/output.md @@ -4,4 +4,4 @@ Editor-split italic link then text: *[x](https://example.com) more text*. Adjacent same-tag runs with no link nearby: **ab**. -A link only partly bold does not merge: **a**[b**c**](https://example.com). +A link only partly bold does not merge: **a** [b**c**](https://example.com). From 52e7b79e6990b23f0514e6bd4483e37dc612226d Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 11:03:59 -0400 Subject: [PATCH 2/2] fix(convert): address the mark-edge-whitespace review The whitespace renderMark moved outside a mark was trimmed again by any wrapper Markdown has no syntax for, so coloured bold text (a around a ) still read as "**bold**next", and / lost their edge spaces outright. Such a wrapper now returns its run untrimmed and leaves the whitespace to the run around it. Link text was trimmed the same way, so "see here" read as "[see](url)here". The whitespace now stays inside the brackets, where Markdown allows it and publishes it back. A heading is one line, so a hard break in one, now reachable from a break at a bold run's edge, ended it and published the rest as a paragraph. A break in a heading stays a literal
. renderInlineRun builds into a byte slice rather than concatenating strings, which was quadratic in a paragraph's inline children, and renderMark trims each edge once. The table property test now generates a space at a mark's edge, from a stream of its own so every existing seed's table is otherwise unchanged, and follows such a mark directly with the next piece of text. Its model treats a space at a mark's edge as the same inside or outside, and two runs of one mark separated only by a space as one run, since both look the same in Confluence. With the converter from before #204 it fails at seed 5. --- internal/convert/aclink.go | 9 +++- internal/convert/storage_to_md.go | 38 ++++++++----- internal/convert/table_property_test.go | 53 ++++++++++++++++--- .../mark-edge-whitespace/input.storage | 4 ++ .../storage2md/mark-edge-whitespace/output.md | 8 +++ 5 files changed, 92 insertions(+), 20 deletions(-) diff --git a/internal/convert/aclink.go b/internal/convert/aclink.go index 4229fde..4b501a8 100644 --- a/internal/convert/aclink.go +++ b/internal/convert/aclink.go @@ -412,8 +412,15 @@ func (r *mdRenderer) acLinkText(n *snode, fallback string) string { // "Nothing but plain text" is checkable rather than guessable: a text node is // an snode with an empty name, so a body whose every descendant is one carries // no markup for escaping to damage. +// +// Whitespace at the text's edges is kept inside the brackets, where Markdown +// allows it and publishes it back; trimming it joined "see here" into +// "[see](url)here" (#204). func (r *mdRenderer) inlineTextForLink(n *snode) string { - rendered := r.renderInlineChildren(n) + rendered := r.renderInlineRun(n) + if strings.TrimSpace(rendered) == "" { + return "" + } if !onlyText(n) { return rendered } diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 697d51d..3183abe 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -15,6 +15,7 @@ package convert // aclink.go. import ( + "bytes" "encoding/json" "encoding/xml" "fmt" @@ -289,7 +290,10 @@ func (r *mdRenderer) renderBlock(n *snode, listIndent string) string { switch n.name { case "h1", "h2", "h3", "h4", "h5", "h6": level := int(n.name[1] - '0') - return strings.Repeat("#", level) + " " + r.renderInlineChildren(n) + // A heading is one line, so a hard break would end it and publish the + // rest as a paragraph. The break stays as the
it was. + text := strings.ReplaceAll(r.renderInlineChildren(n), hardBreak, "
") + return strings.Repeat("#", level) + " " + text case "p": return r.renderInlineChildren(n) case "ul": @@ -645,7 +649,7 @@ func (r *mdRenderer) renderInlineChildren(n *snode) string { // renderInlineRun is renderInlineChildren without the trim, for a mark, which // must see the whitespace at its own edges to move it outside its delimiters. func (r *mdRenderer) renderInlineRun(n *snode) string { - var s string + var buf []byte for _, k := range coalesceSplitMarks(n.kids) { part := r.renderInline(k) // A mark moves its edge whitespace outside its delimiters, so a space @@ -653,7 +657,7 @@ func (r *mdRenderer) renderInlineRun(n *snode) string { // collapses a run of spaces anyway, so the second would only reach the // Markdown on this read and be gone on the next, and the Markdown would // not be a fixed point. - if strings.HasSuffix(s, " ") && strings.HasPrefix(part, " ") && !opensWithBreak(part) { + if bytes.HasSuffix(buf, []byte(" ")) && strings.HasPrefix(part, " ") && !opensWithBreak(part) { part = strings.TrimLeft(part, " ") } // Likewise, whitespace before a hard break is gone on the next read, @@ -661,7 +665,7 @@ func (r *mdRenderer) renderInlineRun(n *snode) string { // "a " (three spaces) and read back as two. The break is written as // exactly the two spaces that make it one. if opensWithBreak(part) { - s = strings.TrimRight(s, " \t") + buf = bytes.TrimRight(buf, " \t") part = hardBreak + strings.TrimLeft(part, " \t")[1:] } // Storage is XHTML and its newlines are insignificant, so @@ -674,12 +678,12 @@ func (r *mdRenderer) renderInlineRun(n *snode) string { // *are* the next hard break. Found by the round-trip property test. // Never trim a hard break itself: two of them in a row are two blank // line-endings, and its own leading spaces are what make it one. - if !opensWithBreak(part) && strings.HasSuffix(s, hardBreak) { + if !opensWithBreak(part) && bytes.HasSuffix(buf, []byte(hardBreak)) { part = strings.TrimLeft(part, " \t") } - s += part + buf = append(buf, part...) } - return s + return string(buf) } // opensWithBreak reports whether an inline part begins with a hard break -- @@ -698,15 +702,19 @@ func opensWithBreak(part string) bool { // CommonMark's flanking rule counts it: a trailing no-break space refuses the // delimiter as surely as a space does. A mark holding only whitespace has // nothing Markdown can mark, so it renders as that whitespace. +// +// This is the whitespace half of the flanking rule only. A mark whose text +// starts or ends with punctuation against a letter outside it ("a**(b)**") +// still fails the other half, and did before. func (r *mdRenderer) renderMark(n *snode, delim string) string { s := r.renderInlineRun(n) - inner := strings.TrimFunc(s, unicode.IsSpace) - if inner == "" { + body := strings.TrimLeftFunc(s, unicode.IsSpace) + if body == "" { return s } - lead := s[:len(s)-len(strings.TrimLeftFunc(s, unicode.IsSpace))] - trail := s[len(strings.TrimRightFunc(s, unicode.IsSpace)):] - return lead + delim + inner + delim + trail + lead := s[:len(s)-len(body)] + inner := strings.TrimRightFunc(body, unicode.IsSpace) + return lead + delim + inner + delim + body[len(inner):] } // hardBreak is Markdown's two-space line break, as renderInline emits it for a @@ -895,7 +903,11 @@ func (r *mdRenderer) renderInline(n *snode) string { // would otherwise render the node and the fallback one after the other. return serialize(adfPassthrough(n)) default: - return r.renderInlineChildren(n) + // A wrapper Markdown has no syntax for (a coloured , , ) + // keeps its edge whitespace for the run around it to settle: trimming it + // here joined "bold next" into one word + // after renderMark had moved the space out. + return r.renderInlineRun(n) } } diff --git a/internal/convert/table_property_test.go b/internal/convert/table_property_test.go index 7eb2454..50dbed8 100644 --- a/internal/convert/table_property_test.go +++ b/internal/convert/table_property_test.go @@ -106,7 +106,7 @@ func newPropertyEnv(t testing.TB) *propertyEnv { } func (e *propertyEnv) check(seed uint64) string { - g := &tableGen{r: rand.New(rand.NewPCG(seed, 0x55))} + g := &tableGen{r: rand.New(rand.NewPCG(seed, 0x55)), edge: rand.New(rand.NewPCG(seed, 0x204))} return e.checkStorage(fmt.Sprintf("seed %d", seed), g.table(0)) } @@ -416,10 +416,27 @@ func modelInline(kids []*snode) string { if a, ok := inlineAlias[name]; ok { name = a } - fmt.Fprintf(&b, "<%s>%s", name, modelInline(k.kids), name) + inner := modelInline(k.kids) + // A space at the edge of a mark separates the same words inside it + // as outside, and Markdown can only put it outside (#204). Only the + // marks: a span's spaces are its text. + var lead, trail string + if name == "strong" || name == "em" || name == "del" { + body := strings.TrimLeft(inner, " ") + lead = inner[:len(inner)-len(body)] + inner = strings.TrimRight(body, " ") + trail = body[len(inner):] + } + fmt.Fprintf(&b, "%s<%s>%s%s", lead, name, inner, name, trail) } } s := whitespaceRunRE.ReplaceAllString(b.String(), " ") + // Two runs of one mark with nothing but a space between them read as one + // run: "a b" and "a b" look the same. + for _, m := range []string{"strong", "em", "del"} { + s = strings.ReplaceAll(s, "<"+m+">", "") + s = strings.ReplaceAll(s, " <"+m+">", " ") + } // Whitespace beside a line break shows nothing, and publishing writes a // newline after every
. return strings.ReplaceAll(strings.ReplaceAll(s, " ⏎", "⏎"), "⏎ ", "⏎") @@ -541,7 +558,10 @@ func modelHasElement(n *snode) bool { // which #203 covers, and inline tags read has no Markdown for (") || strings.HasSuffix(body, " ") || + strings.HasPrefix(next, " ") || strings.HasPrefix(next, " ") { + sep = "" + } + body += sep + next } if g.chance(0.1) { body += "
" + g.text() diff --git a/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage b/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage index 03e52a1..92f9c92 100644 --- a/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage +++ b/internal/convert/testdata/storage2md/mark-edge-whitespace/input.storage @@ -8,4 +8,8 @@

A hard break at the end of a mark: a
b.

A hard break at the start of a mark, after a space: a
b.

A space before a hard break: a
b.

+

Coloured bold: bold next, and underline: under next.

+

Link text: see here, and bold next.

+

A heading with a break at a mark's edge
continued

+

A heading with a bare break
continued

ItemTime
lost for 13.5h1
diff --git a/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md b/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md index 7ba7fe9..64e8d08 100644 --- a/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md +++ b/internal/convert/testdata/storage2md/mark-edge-whitespace/output.md @@ -21,6 +21,14 @@ A hard break at the start of a mark, after a space: a A space before a hard break: a b. +Coloured bold: **bold** next, and underline: under next. + +Link text: [see ](https://example.com)here, and [**bold** ](https://example.com)next. + +## **A heading with a break at a mark's edge**
continued + +### A heading with a bare break
continued + | Item | Time | | --- | --- | | ~~lost for~~ 13.5h | 1 |