From 2a33b765c4d5a5252be7359988c56a94cd8e5d1c Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:17:22 -0400 Subject: [PATCH 1/9] docs(plans): 056 escape text that read writes --- _plans/056_escape-text-on-read.md | 286 ++++++++++++++++++++++++++++++ 1 file changed, 286 insertions(+) create mode 100644 _plans/056_escape-text-on-read.md diff --git a/_plans/056_escape-text-on-read.md b/_plans/056_escape-text-on-read.md new file mode 100644 index 0000000..730900d --- /dev/null +++ b/_plans/056_escape-text-on-read.md @@ -0,0 +1,286 @@ +# 056: escape text that `read` writes + +Answers #203, widened to cover inline syntax as well as block markers. + +`read` and `export` write a text node's characters straight into the +Markdown. Wherever those characters spell Markdown syntax, the next publish +turns them into that syntax, and the page's content changes with nothing +reported. #203 filed the block half (`

1. not a list

` publishes as a +list). Surveying it showed the same gap inline: literal `*not emphasis*` +publishes as ``, and literal `x` as real markup. Both halves come +from one missing mechanism, so they are fixed together. Fixing only the block +half would leave the same silent content change in place. + +Markdown is not at fault. CommonMark lets any ASCII punctuation character be +escaped with a backslash, and a Markdown writer is expected to escape text +that would otherwise read as syntax. `read` writes no escapes at all. + +Principles: **L5** (`roundtrip-from-confluence`), which this violates today. +**L6**'s Accepts covers the result, since an escape is a difference in +spelling: `\*` in an exported file publishes the same `*` the author typed. + +## What was checked + +Each row publishes a Markdown string through `MdToConfluence` (goldmark with +`extension.GFM`, the forward converter as shipped). All rows are Verified on +2026-09-28. + +**Text that becomes syntax today:** + +| text, as `read` writes it | publishes as | +|---|---| +| `1. x`, `1) x`, `10. x` | an ordered list (`start="10"` for the last) | +| `- x`, `+ x`, `* x` | a bulleted list | +| `# x` | `

` | +| `>x`, `> x` | a blockquote (no space needed) | +| `***`, `- - -` | `
` | +| `a` + hard break + `---` | `

a

` (setext) | +| `a` + hard break + `===` | `

a

` (setext) | +| `a \| b` + hard break + `--- \| ---` | a table | +| `a` + hard break + `# b` | `

a

b

`: a heading can interrupt a paragraph | +| `~~~` at a line start | an empty code macro | +| `2*3*4`, `_x_`, `a _b_ c` | `` | +| `a ~b~ c`, `a ~~b~~ c` | ``: goldmark accepts a single tilde | +| `` `x` `` | `` | +| `x`, `` | raw HTML (a comment is then stripped by Confluence) | +| ``, `` | a link | +| `©`, `©` | `©` | +| `[x]: y` at a line start | nothing: a link reference definition, consumed | +| `[x]` with a matching definition elsewhere | a link | +| `- [ ] x` | a task-list checkbox | +| `https://x.com`, `www.x.com`, `a@b.com` | a link (GFM autolinking) | +| `## Item #` | `

Item

`: a trailing ` #` is a closing sequence | + +**Text that is already safe**, which the escaping must leave alone so that +exported Markdown stays readable: + +| text | publishes as | +|---|---| +| `~5 min to ~10 min` | text: neither tilde can close | +| `a * b * c` | text: a `*` with space on both sides cannot open or close | +| `snake_case_word`, `foo_bar_` | text: an intraword `_` cannot open or close | +| `a < b`, `a`, `\-`, `\+`, `\*`, `\_`, `\~`, `` \` ``, `\<`, +`\©`, `\[`, `\]` (inside link text), `\\`, `\=`, `\|`, `## Item \#`, +`https\://`, `www\.`, `a\@b.com`. `\` publishes as the text +`` too, so the shield (which renames raw `ac:`/`ri:` tags before +goldmark sees them) does not interfere with an escaped one. + +**One place an escape does not work:** image alt text. `![a\]b](…)` publishes +`ac:alt="a\]b"`, backslash included, because the forward converter takes the +alt from the source bytes rather than from goldmark's unescaped text. See +Not in scope. + +## Decisions + +**D1. Two layers: inline escaping on each text node, and a line-start pass +over each paragraph.** Inline syntax can be judged from a text node and its +neighbours within the node, so it is escaped where the text enters the +Markdown: `renderInline`'s text case, and the loose text `blockStrings` +handles. Block syntax depends on where a line starts, which only the assembled +paragraph knows: a line starts at the paragraph's start and after every hard +break, and text nodes do not know either. So the block half is a pass over a +paragraph's rendered text, split at hard breaks. The pass is safe on rendered +Markdown because nothing markfluence emits inline starts a line with a block +marker (D4 has the argument). Both live in a new `internal/convert/escape.go`. + +**D2. Inline escaping depends on the characters around it, and a node's edge +counts as "could be anything".** Escaping every punctuation character would be +correct and unreadable (`snake\_case`, `a \* b`, `\~5 min`). Each rule below +escapes only where the character could take effect. A text node's first and +last characters sit next to whatever the neighbouring node renders, which the +node cannot see, so at an edge the rule assumes the worst and escapes. +Escaping too much is safe, since every escape round-trips as the literal +character. Escaping too little is this bug. So when a rule is unsure, it +escapes. + +| character | escaped when | why | +|---|---|---| +| `\` | followed by ASCII punctuation, or last in the node | only then is it an escape; at the edge the next node's first character is unknown | +| `` ` `` | always | any backtick can open a code span | +| `*` | unless whitespace on both sides | a `*` between spaces is neither left- nor right-flanking | +| `_` | unless letters or digits on both sides, or whitespace on both | intraword `_` cannot open or close; between spaces it is not flanking | +| `~` | preceded by non-whitespace (or the node's start), or next to another `~` | every strikethrough needs a closer, and a closer is preceded by non-whitespace; a run of two could pair with an emitted `~~`. This keeps `about ~5 min` readable | +| `[` | always | `[x]: y` is a definition, `[ ] x` in a list item a checkbox, and `[x]` a link wherever a definition exists | +| `]` | inside link text only (D5) | elsewhere a lone `]` is inert, and brackets are common in prose | +| `<` | followed by a letter, `/`, `!` or `?` | the starts of a tag, a comment, a declaration and an autolink; `a < b` and `<3` stay | +| `&` | followed by an entity (`&name;`, `&#nnn;`, `&#xhh;`) | `AT&T` stays | + +**D3. The line-start pass.** For each line of a paragraph (its first line, and +each line after a hard break), after skipping leading spaces, the first +character is escaped when the line: + +- starts with 1–6 `#` followed by a space or the end of the line (`\#`); +- starts with `>` (`\>`); +- starts with `-` or `+` followed by a space or the end of the line (`\-`, + `\+`; `*` is already covered by D2's rule); +- starts with 1–9 digits then `.` or `)` followed by a space or the end of the + line: the delimiter is escaped, as in `1\.` and `1\)`; +- is a thematic break (three or more `-`, `*` or `_`, alone or separated by + spaces), a setext underline (only `=` or only `-`), or a table delimiter row + (only `|`, `:`, `-` and spaces, holding at least one `|` and one `-`); +- starts with `~~~` or ```` ``` ```` (after D2 a text-origin fence is already + escaped; the pass covers any other way to reach it). + +A heading's text is not a paragraph, since ATX heading content cannot start a +block, so the pass does not apply to it. D6 handles headings. + +**D4. Why the pass cannot damage markup markfluence emits.** An inline +construct markfluence writes starts with `*`, `**`, `~~`, `` ` ``, `[`, `![`, +`<` (raw storage, or a heading's `
`) or `\` (an escape from D2). None of +them matches D3: +- a mark is never followed by a space, because `renderMark` moves edge + whitespace outside its delimiters (#204); +- a whitespace-only mark renders as its whitespace, so no `***` or `****` is + emitted; +- nothing markfluence emits inline starts with a digit, `#`, `>`, `-`, `+`, + `=`, `|` or `:`. + +The pass splits only at hard breaks (`" \n"`), never at every newline. So a +raw serialized inline element whose CDATA holds a newline is never split, and +its content never gains a backslash. + +**D5. Link text escapes `]` too, and `escapeLinkText` becomes the full +escaper.** Today `escapeLinkText` escapes `\`, `[` and `]` and nothing else. +Its comment records that a title like `*Foo*` renders as emphasis and was +knowingly left alone. With escaping at the text node, link text needs only one +addition: `]`, which ends a link's text early. The renderer tracks whether it +is inside link text (a depth counter on `mdRenderer`, set by +`inlineTextForLink`). `inlineTextForLink`'s `onlyText` split goes away: it +existed so that already-rendered markup was not escaped, and text is now +escaped at its source, before any markup is added around it. The raw sources, +which are strings rather than text nodes, go through the same escaper with the +link rule on: a page title, a space key, an anchor, a CDATA link body, a +fallback, and a mention's display name. + +`MentionMarkdown` therefore changes for a display name holding `*`, `_`, `` ` +``, `<`, `&` or `~`. `user-find` prints the same function's output, so the +guarantee that the line an author pastes is byte-identical to what `read` +writes holds by construction. The line itself changes for those names. + +**D6. A heading's closing sequence.** `## Item #` publishes as "Item". When a +heading's rendered text ends in whitespace followed by one or more `#`, the +first `#` of that run is escaped: `## Item \#`. `C#` has no space before the +`#`, so it is not a closing sequence and stays as it is. + +**D7. Bare URLs and email addresses are escaped out of autolinking.** GFM +autolinks `http://`, `https://`, `ftp://`, `www.` and email addresses, so +plain text holding a URL publishes as a link. An escape suppresses it +(Verified): `https\://`, `www\.`, `a\@b.com`. This is the one rule whose cost +is visible, since `https\://example.com` reads badly in an exported file. It +is included anyway: +- storage holds a URL as plain text only when someone chose that; the editor + turns a typed URL into a link; +- turning it into a link is a content change, of the kind this plan exists to + stop; +- without it, a page holding one is not a fixed point after one read. + +It escapes the `:` of `scheme://` for the three schemes goldmark links, the +`.` of a `www.` that starts a word, and the `@` of anything goldmark's email +pattern would match. + +**D8. What is never escaped:** +- **code spans and code blocks:** their text is literal already; +- **link destinations:** they are URLs, encoded by `encodeDestination`; +- **raw storage:** `serialize` and `renderRawBlock`'s tags go through + `xmlTextEscape` or pass through verbatim; +- **the TOC token, frontmatter and image alt text:** frontmatter is YAML with + its own writer; alt text is covered in Not in scope. + +Text inside a raw block's content container, such as a raw table cell or a +layout cell, is Markdown and is escaped like any other: those bodies go +through `blockStrings`. + +**D9. Pipe-table cells get the inline layer and not the line-start pass.** A +GFM table cell is inline, so `1. x` in a cell is text. `renderCellLines` goes +through `renderInlineChildren`, which picks up D2 without any change. +`escapeCellPipe` still runs afterwards, and D2 escapes no `|`, so the two do +not compound. + +## Implementation + +1. `escape.go`: `escapeText(s string, inLink bool) string` (D2, D5, D7) and + `escapeLineStarts(s string) string` (D3). Both are pure functions over + strings, tested on their own. +2. `renderInline`'s text case and `blockStrings`' loose text call + `escapeText`, passing `r.linkDepth > 0` as `inLink`. +3. A single helper, called wherever a paragraph's rendered text becomes a + Markdown block, applies `escapeLineStarts`. The call sites: + - `renderBlock`'s `p` case and its inline-element-at-block-level case; + - `renderListItem`'s text segments; + - `blockStrings`' loose text; + - `rawCellBlocks`' paragraphs and loose runs. + Callouts and layout cells reach it through `blockStrings`, and each call + site gets a test. +4. Headings apply D6. +5. `inlineTextForLink` increments `linkDepth` and drops the `onlyText` split. + `escapeLinkText` becomes `escapeText(s, true)`, and its comment is + rewritten. +6. The table property generator drops its #203 restriction and gains words + holding the characters above. `markText` stays as #204 left it. + +## Tests + +- **Unit tests** for `escapeText` and `escapeLineStarts`, one row for each + rule and each "stays readable" row in What was checked. Each row checks the + escaped form *and* that publishing it gives back the original text, so no + rule can emit an escape that does not round-trip. +- **A `storage2md/escaping` case** with one paragraph per row of the first + table, plus list items, a raw table cell, a layout cell, a callout, a link + text, a heading ending in ` #`, and a mention whose name holds `*`. Being a + storage2md case it also runs through `TestRoundTripMarkdownIsAFixedPoint` + and the output-parses-as-Markdown check. +- **Existing goldens** that hold these characters in text change. Each change + is reviewed and listed in the PR: every one should be a new backslash and + nothing else. +- **The property test** with the restriction lifted. It must fail against + `main`'s converter and pass with the fix, the way #204's did (checked by + swapping in `main`'s `storage_to_md.go`, not by stashing). +- `user-find`'s test that its line matches `read`'s still passes unchanged. + +## Documentation + +- `docs/markdown-file.md`: a short section saying that exported Markdown + escapes text that would otherwise read as Markdown, with the D7 cost stated, + so an author who sees `\*` or `https\://` in an exported file knows why. +- `CLAUDE.md`: the converter bullet gains `escape.go` and the two layers. The + regression-suite paragraph drops #203 from the gaps the generator avoids. +- `escapeLinkText`'s comment, which records the `*Foo*` gap as known, is + rewritten. + +## Not in scope + +- **Image alt text.** The forward converter reads alt from the source bytes, + so an escape there publishes the backslash. A `]` in alt text already breaks + the image today. Fixing it means unescaping on the forward side first, a + change to what `update` publishes for existing files, and it deserves its + own issue. +- **A code span holding a backtick.** `renderInline` writes `` `x` `` with a + single backtick whatever the content, so code holding a backtick breaks. It + is a separate, older gap with a known fix (a longer fence), not escaping. +- **The mention marker.** A link to an Atlassian Home profile whose text + starts with `@` publishes as a mention, by design (#91). Escaping that `@` + would break the convention for real mentions. +- **#213** (two hard breaks in a row) and **#214** (an empty paragraph before a + list in a cell) are left as they are, although both are in the same + functions. + +## Commits + +Each commit passes `make check` on its own. So `escape.go` arrives with its +first caller, because golangci-lint's `unused` check fails a function nothing +calls. + +1. `docs(plans): 056 escape text that read writes` +2. `fix(convert): escape inline syntax in text on read` (`escape.go`'s + `escapeText`, D2, D5, D9, unit tests, goldens) +3. `fix(convert): escape block markers at a line start on read` (D3, D6) +4. `fix(convert): keep bare URLs as text on read` (D7) +5. `test(convert): let the table property test write block markers` +6. `docs: what read escapes` From 1c9d15168b2359f077370e4547861fb60c9613ac Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:22:59 -0400 Subject: [PATCH 2/9] fix(convert): escape inline syntax in text on read read wrote a text node's characters straight into the Markdown, so text that spells inline syntax became that syntax on the next publish: literal "*not emphasis*" published as , "x" as real markup, "©" as the character, "[x]: y" vanished as a link definition. escapeText (escape.go) escapes each text node as it is rendered, and only where a character could take effect, so snake_case, "a * b", "about ~5 min", AT&T and "a < b" stay readable. A node's edges border whatever the next node renders, so every rule escapes there. Each rule and each escape was measured against goldmark (_plans/056, D2). Link text escapes a "]" too, tracked by a depth counter on the renderer. escapeLinkText becomes escapeText with that rule on, so a page title, an anchor or a mention's display name holding "*Foo*" no longer renders as emphasis. inlineTextForLink's plain-text-only split is gone: text is escaped at its source, before markup surrounds it. TestStorageToMarkdownOutputParsesAsMarkdown's bracket checks now remove escapes first, and allow a literal "]" outside link text. Part of #203. --- internal/convert/aclink.go | 78 ++++-------- internal/convert/aclink_test.go | 16 ++- internal/convert/escape.go | 116 ++++++++++++++++++ internal/convert/escape_test.go | 85 +++++++++++++ internal/convert/export_test.go | 8 +- internal/convert/output_parses_test.go | 23 +++- internal/convert/storage_to_md.go | 10 +- .../storage2md/escaping/input.storage | 15 +++ .../testdata/storage2md/escaping/output.md | 46 +++++++ 9 files changed, 328 insertions(+), 69 deletions(-) create mode 100644 internal/convert/escape.go create mode 100644 internal/convert/escape_test.go create mode 100644 internal/convert/testdata/storage2md/escaping/input.storage create mode 100644 internal/convert/testdata/storage2md/escaping/output.md diff --git a/internal/convert/aclink.go b/internal/convert/aclink.go index 4b501a8..6e82b2d 100644 --- a/internal/convert/aclink.go +++ b/internal/convert/aclink.go @@ -376,14 +376,10 @@ func (r *mdRenderer) renderAnchorLink(n *snode, anchor string) string { // is that name. // // A body comes in two spellings -- ac:link-body holds rich text, and -// ac:plain-text-link-body holds CDATA -- and both occur on real pages. -// -// Only the *raw* sources are escaped, and which is which is the whole point of -// the split below. An ac:link-body has already been rendered to Markdown by -// renderInlineChildren, so escaping it would turn a bold link body into a -// literal "\*\*bold\*\*". The CDATA body and the fallback are plain text -// straight off the server -- a page title, a space key, an anchor -- and a "]" -// in any of them ends the link early. +// ac:plain-text-link-body holds CDATA -- and both occur on real pages. The rich +// body's text nodes are escaped as they render; the CDATA body and the fallback +// are plain text straight off the server -- a page title, a space key, an +// anchor -- and are escaped whole. func (r *mdRenderer) acLinkText(n *snode, fallback string) string { if b := findChild(n, "ac:link-body"); b != nil { if s := r.inlineTextForLink(b); s != "" { @@ -398,71 +394,47 @@ func (r *mdRenderer) acLinkText(n *snode, fallback string) string { return escapeLinkText(fallback) } -// inlineTextForLink renders a node's children as a Markdown link's text, -// escaping the result when it is nothing but plain text. -// -// The distinction matters both ways, and an earlier version got it wrong in one -// direction. Escaping a *rendered* body turns "bold" into a -// literal "\*\*bold\*\*", which is why the escaping was first applied only to -// the raw sources. But the common case for a link body is plain text, and -// leaving it unescaped loses the link outright: a page titled "Q1 Draft]" -// rendered as "[Q1 Draft] notes](url)", which CommonMark reads as literal text, -// so the next update publishes no link at all. Found in review. +// inlineTextForLink renders a node's children as a Markdown link's text. // -// "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. +// Its text nodes are escaped as they render, with a "]" escaped too while +// linkDepth is raised: unescaped, a page titled "Q1 Draft]" rendered as +// "[Q1 Draft] notes](url)", which CommonMark reads as literal text, so the next +// update publishes no link at all. Escaping at the text node rather than over +// the rendered result is what lets a body holding markup be escaped at all: +// escaping "bold" after rendering turns it into a literal +// "\*\*bold\*\*", which is why this used to escape plain-text bodies only. // // 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 { + r.linkDepth++ + defer func() { r.linkDepth-- }() rendered := r.renderInlineRun(n) if strings.TrimSpace(rendered) == "" { return "" } - if !onlyText(n) { - return rendered - } - return escapeLinkText(rendered) + return rendered } -// onlyText reports whether every descendant of n is a text node, so rendering -// it produced no Markdown syntax of its own. -func onlyText(n *snode) bool { - for _, k := range n.kids { - if k.name != "" || !onlyText(k) { - return false - } - } - return true -} - -// escapeLinkText makes plain text safe to use as a Markdown link's text. -// -// The set is deliberately the one that *breaks* a link rather than everything -// Markdown reads specially: an unescaped "]" ends the text early and leaves the -// rest of the line as literal junk, and a backslash has to go first or it would -// escape the escapes. A title like "*Foo*" is a different problem -- it renders -// as emphasis instead of as asterisks, losing fidelity without breaking the -// link -- and is knowingly not handled here, since escaping every Markdown -// indicator in every recovered title is a larger change with its own round-trip -// consequences. +// escapeLinkText makes plain text -- a string rather than a rendered node -- +// safe to use as a Markdown link's text: escapeText with a "]" escaped too. // -// Before this, mdLink was a bare Sprintf: any page title holding a bracket -// exported as a broken link, which mentions turned from theoretical into likely -// because display names carry them. +// It escapes everything Markdown would read, not only what breaks the link. +// It began as "\", "[" and "]", the set that breaks a link: before that, +// mdLink was a bare Sprintf and any page title holding a bracket exported as a +// broken link, which mentions turned from theoretical into likely because +// display names carry them. A title like "*Foo*" then still rendered as +// emphasis, and #203 closed that the way it closed it for all text. func escapeLinkText(s string) string { - s = strings.ReplaceAll(s, `\`, `\\`) - s = strings.ReplaceAll(s, "[", `\[`) - return strings.ReplaceAll(s, "]", `\]`) + return escapeText(s, true) } // mdLink renders an inline Markdown link, falling back to showing the // destination when there is no text for it. func mdLink(text, dest string) string { if text == "" { - text = dest + text = escapeLinkText(dest) } return fmt.Sprintf("[%s](%s)", text, dest) } diff --git a/internal/convert/aclink_test.go b/internal/convert/aclink_test.go index d608e55..b848a5b 100644 --- a/internal/convert/aclink_test.go +++ b/internal/convert/aclink_test.go @@ -267,11 +267,17 @@ func TestLinkTextDoesNotEscapeARenderedBody(t *testing.T) { } } -// TestLinkTextEscapesABackslashFirst: the backslash pass has to run before the -// bracket passes, or it would escape the escapes they add. -func TestLinkTextEscapesABackslashFirst(t *testing.T) { - if got := convert.EscapeLinkTextForTest(`a\b]c`); got != `a\\b\]c` { - t.Errorf("EscapeLinkText = %q, want %q", got, `a\\b\]c`) +// TestLinkTextEscapesABackslashBeforeABracket: a backslash before the "]" it +// escapes must itself be escaped, or it would escape the escape. One before a +// letter is literal in CommonMark and is left alone. +func TestLinkTextEscapesABackslashBeforeABracket(t *testing.T) { + for in, want := range map[string]string{ + `a\]c`: `a\\\]c`, + `a\b]c`: `a\b\]c`, + } { + if got := convert.EscapeLinkTextForTest(in); got != want { + t.Errorf("EscapeLinkText(%q) = %q, want %q", in, got, want) + } } } diff --git a/internal/convert/escape.go b/internal/convert/escape.go new file mode 100644 index 0000000..309400d --- /dev/null +++ b/internal/convert/escape.go @@ -0,0 +1,116 @@ +package convert + +import ( + "regexp" + "strings" + "unicode" +) + +// Text read from storage is written into Markdown, and wherever its characters +// spell Markdown syntax the next publish turns them into that syntax: literal +// "*not emphasis*" publishes as , "1. not a list" as a list (#203). A +// backslash before any ASCII punctuation character makes it literal in +// CommonMark, so the fix is the one every Markdown writer makes: escape text +// that would otherwise read as syntax. What goldmark treats as syntax, and that +// each escape here publishes back as the literal character, was measured +// (_plans/056). +// +// The escaping is minimal rather than total. Escaping every punctuation +// character would be correct and unreadable -- snake\_case, a \* b -- so each +// rule escapes only where the character could take effect. A text node's +// first and last characters sit beside whatever the neighbouring node renders, +// which the node cannot see, so at an edge every rule assumes the worst: an +// extra escape round-trips as the literal character, and a missing one is the +// bug. + +// escapeText escapes the inline syntax in one text node's (collapsed) text. +// inLink is set inside a link's text, where a "]" would end the text early and +// must be escaped too; elsewhere a lone "]" is inert and common in prose. +func escapeText(s string, inLink bool) string { + if !strings.ContainsAny(s, "\\`*_~[]<&") { + return s + } + rs := []rune(s) + var b strings.Builder + for i, c := range rs { + if needsEscape(rs, i, inLink) { + b.WriteByte('\\') + } + b.WriteRune(c) + } + return b.String() +} + +// needsEscape reports whether rs[i] must be escaped. +func needsEscape(rs []rune, i int, inLink bool) bool { + // prev and next are -1 at the node's edge: unknown. + prev, next := rune(-1), rune(-1) + if i > 0 { + prev = rs[i-1] + } + if i < len(rs)-1 { + next = rs[i+1] + } + switch rs[i] { + case '\\': + // Only a backslash before punctuation is an escape; before anything + // else it is literal (C:\path). At the edge the next character is + // whatever the next node renders, which is usually punctuation. + return next == -1 || isASCIIPunct(next) + case '`': + // Any backtick can open a code span. + return true + case '*': + // A "*" with whitespace on both sides is neither left- nor + // right-flanking, so it can neither open nor close emphasis. + return !isSpace(prev) || !isSpace(next) + case '_': + // An intraword "_" cannot open or close emphasis (snake_case), and one + // with whitespace on both sides is not flanking at all. + intraword := isAlnum(prev) && isAlnum(next) + spaced := isSpace(prev) && isSpace(next) + return !intraword && !spaced + case '~': + // goldmark takes a single tilde as strikethrough, so every one could + // matter. But a strikethrough needs a closer, and a closer is preceded + // by something other than whitespace: escaping every possible closer + // leaves an opener with nothing to pair with. A run of two could pair + // with a "~~" markfluence emits, so a tilde beside another is escaped + // too. This keeps "about ~5 min" readable. + return !isSpace(prev) || prev == '~' || next == '~' + case '[': + // "[x]: y" at a line start is a link reference definition and + // vanishes, "[ ] x" in a list item is a checkbox, and "[x]" is a link + // wherever a definition exists. + return true + case ']': + return inLink + case '<': + // The starts of a tag, a comment, a declaration and an autolink. + // "a < b" and "<3" are text. + return next == -1 || unicode.IsLetter(next) || next == '/' || next == '!' || next == '?' + case '&': + // An entity reference publishes as its character; "AT&T" is text. + return next == -1 || entityRE.MatchString(string(rs[i:min(len(rs), i+maxEntity)])) + } + return false +} + +// entityRE matches a character reference at the start of a string: named, +// decimal or hexadecimal, as CommonMark recognises them. +var entityRE = regexp.MustCompile(`^&(?:#[0-9]{1,7}|#[xX][0-9a-fA-F]{1,6}|[A-Za-z][A-Za-z0-9]{1,31});`) + +// maxEntity bounds how far entityRE can match: "&", a 32-character name, ";". +const maxEntity = 34 + +// isSpace reports whether r is known whitespace; an unknown neighbour is not. +func isSpace(r rune) bool { return r != -1 && unicode.IsSpace(r) } + +// isAlnum reports whether r is a known letter or digit. +func isAlnum(r rune) bool { return r != -1 && (unicode.IsLetter(r) || unicode.IsDigit(r)) } + +// isASCIIPunct reports whether r is one of the characters CommonMark lets a +// backslash escape. +func isASCIIPunct(r rune) bool { + return r < 128 && strings.ContainsRune("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", r) +} diff --git a/internal/convert/escape_test.go b/internal/convert/escape_test.go new file mode 100644 index 0000000..1e618bc --- /dev/null +++ b/internal/convert/escape_test.go @@ -0,0 +1,85 @@ +package convert_test + +import ( + "html" + "strings" + "testing" + + "github.com/mozilla/markfluence/internal/convert" +) + +// escapeCase is one text, the escaped form escapeText writes for it, and why. +type escapeCase struct{ text, want, why string } + +// inlineEscapes are the inline rules, each row a whole paragraph's text. The +// rows that stay unchanged matter as much as the others: they are what keeps +// exported Markdown readable (_plans/056). +var inlineEscapes = []escapeCase{ + {`*not emphasis*`, `\*not emphasis\*`, "emphasis"}, + {`a * b * c`, `a * b * c`, "a * between spaces cannot open or close"}, + {`2*3*4`, `2\*3\*4`, "intraword * is emphasis"}, + {`snake_case_word`, `snake_case_word`, "intraword _ cannot open or close"}, + {`a _b_ c`, `a \_b\_ c`, "emphasis"}, + {`_x_`, `\_x\_`, "emphasis"}, + {`about ~5 min`, `about ~5 min`, "a tilde after a space cannot close"}, + {`~5 min to ~10 min`, `\~5 min to ~10 min`, "a tilde at the edge is unknown"}, + {`a ~b~ c`, `a ~b\~ c`, "goldmark takes a single tilde as strikethrough"}, + {`a ~~b~~ c`, `a \~\~b\~\~ c`, "strikethrough"}, + {"`x`", "\\`x\\`", "a code span"}, + {`x`, `\x\`, "raw HTML"}, + {``, `\`, "a comment Confluence would strip"}, + {`a < b`, `a < b`, "not a tag"}, + {`a") + if ok { + inner, ok = strings.CutSuffix(inner, "

") + } + if !ok || strings.Contains(inner, "<") || html.UnescapeString(inner) != text { + t.Errorf("%q publishes as %q, want a paragraph holding %q", md, got, text) + } +} + +// TestMentionMarkdownEscapesTheName: a display name is plain text off the +// server, so it is escaped like any other, and user-find prints this same line. +func TestMentionMarkdownEscapesTheName(t *testing.T) { + got := convert.MentionMarkdown("Ada *Lovelace* [she/her]", "abc") + want := `[@Ada \*Lovelace\* \[she/her\]](https://home.atlassian.com/people/abc)` + if got != want { + t.Errorf("MentionMarkdown = %q, want %q", got, want) + } +} diff --git a/internal/convert/export_test.go b/internal/convert/export_test.go index ce92474..75b3122 100644 --- a/internal/convert/export_test.go +++ b/internal/convert/export_test.go @@ -8,7 +8,11 @@ package convert // same way renderImage does rather than reimplementing the codec. func DecodeDestinationForTest(dest string) string { return decodeDestination(dest) } -// EscapeLinkTextForTest exposes escapeLinkText, so the ordering of its passes -// (backslash before brackets, or it escapes its own escapes) can be pinned +// EscapeLinkTextForTest exposes escapeLinkText, so that a backslash before a +// bracket is escaped (or it escapes the bracket's escape) can be pinned // directly rather than inferred from a rendered document. func EscapeLinkTextForTest(s string) string { return escapeLinkText(s) } + +// EscapeTextForTest exposes escapeText, so each of its rules can be pinned +// alongside a check that its output publishes back as the original text. +func EscapeTextForTest(s string, inLink bool) string { return escapeText(s, inLink) } diff --git a/internal/convert/output_parses_test.go b/internal/convert/output_parses_test.go index fa1d227..0ca75ba 100644 --- a/internal/convert/output_parses_test.go +++ b/internal/convert/output_parses_test.go @@ -51,6 +51,10 @@ var tableSeparatorRE = regexp.MustCompile(`(?m)^\s*\|?\s*:?-{3,}:?\s*(\|\s*:?-{3 // be excluded from checks that only make sense for prose. var fencedCodeRE = regexp.MustCompile("(?s)```.*?```") +// backslashEscapeRE matches one backslash escape, so that removing every match +// leaves only the Markdown syntax that is not escaped. +var backslashEscapeRE = regexp.MustCompile(`\\.`) + // TestStorageToMarkdownOutputParsesAsMarkdown converts every storage2md case's // input fresh -- the exact body read/export would write to disk -- through a // real GFM parser, and confirms the bracket/table syntax markfluence emitted @@ -148,8 +152,12 @@ func assertParsesCleanly(t *testing.T, source []byte) { // snippet documenting Markdown syntax) with no bearing on this guarantee, // and counting it would be this test's own false positive, not a bug. prose := fencedCodeRE.ReplaceAll(source, nil) + // Backslash escapes are removed before the syntax checks: read escapes + // literal text that would read as Markdown (#203), so an escaped "[x](y)" + // is prose, not a link the parser failed to find. + unescaped := backslashEscapeRE.ReplaceAll(prose, nil) - wantAtLeast := len(imageOrLinkRE.FindAllIndex(prose, -1)) + wantAtLeast := len(imageOrLinkRE.FindAllIndex(unescaped, -1)) if got := images + links; got < wantAtLeast { t.Errorf("parsed %d image/link node(s), want at least %d matching the source's bracket syntax:\n%s", got, wantAtLeast, source) @@ -163,10 +171,13 @@ func assertParsesCleanly(t *testing.T, source []byte) { // A structural sanity check independent of the floor comparison above, // which can't see a rendering bug that mangles bracket syntax badly enough // that the output no longer looks like link/image syntax at all -- nothing - // would be left for imageOrLinkRE to flag as missing. Every "[" markfluence - // emits outside a code fence closes, so an unequal count means something (a - // link, an alt-text bracket) was truncated or malformed outright. - if opens, closes := bytes.Count(prose, []byte("[")), bytes.Count(prose, []byte("]")); opens != closes { - t.Errorf("unbalanced brackets outside fenced code: %d '[' vs %d ']':\n%s", opens, closes, source) + // would be left for imageOrLinkRE to flag as missing. Every unescaped "[" + // markfluence emits outside a code fence opens a link, an image or an alert + // and closes, so more "[" than "]" means something (a link, an alt-text + // bracket) was truncated or malformed outright. Escapes are removed first, + // since read escapes every literal "[" (#203), and fewer "[" than "]" is + // fine: a literal "]" outside link text is inert and left unescaped. + if opens, closes := bytes.Count(unescaped, []byte("[")), bytes.Count(unescaped, []byte("]")); opens > closes { + t.Errorf("unclosed brackets outside fenced code: %d '[' vs %d ']':\n%s", opens, closes, source) } } diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 3183abe..2e776e5 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -116,6 +116,10 @@ type mdRenderer struct { // siteURL is the Confluence site base, for a space link. Never the gateway. siteURL string + // linkDepth is how many links' text is being rendered, so a text node knows + // to escape a "]", which would end the text early (escapeText). + linkDepth int + // userNames maps a mentioned account id -> that person's display name, // resolved by the caller. An id missing from it passes the mention through // as raw storage. See StorageOptions.UserNames. @@ -274,7 +278,7 @@ func (r *mdRenderer) blockStrings(kids []*snode, listIndent string) []string { for _, k := range kids { if k.name == "" { if s := strings.TrimSpace(collapse(k.text)); s != "" { - out = append(out, s) + out = append(out, escapeText(s, false)) } continue } @@ -875,7 +879,7 @@ func concatKids(a, b []*snode) []*snode { // renderInline renders one inline node. func (r *mdRenderer) renderInline(n *snode) string { if n.name == "" { - return collapse(n.text) + return escapeText(collapse(n.text), r.linkDepth > 0) } switch n.name { case "strong", "b": @@ -924,7 +928,7 @@ func (r *mdRenderer) renderLink(n *snode) string { href := n.attrs["href"] text := r.inlineTextForLink(n) if text == "" { - text = href + text = escapeLinkText(href) } if title := n.attrs["title"]; title != "" { return fmt.Sprintf("[%s](%s %s)", text, href, jstr(title)) diff --git a/internal/convert/testdata/storage2md/escaping/input.storage b/internal/convert/testdata/storage2md/escaping/input.storage new file mode 100644 index 0000000..f797ff0 --- /dev/null +++ b/internal/convert/testdata/storage2md/escaping/input.storage @@ -0,0 +1,15 @@ +

Emphasis: *not emphasis*, 2*3*4, a _b_ c, and a * b * c stays.

+

Strikethrough: a ~b~ c, a ~~b~~ c, and about ~5 min stays.

+

Code: `not code`.

+

HTML: <b>not bold</b>, <!-- not a comment -->, <ac:link>not storage</ac:link>, and a < b stays.

+

Entities: &copy; and &#169;, and AT&T stays.

+

Brackets: [x], [x][y], and x] stays.

+

Backslashes: a\*b, and C:\path stays.

+

A link: Q1 *Draft*], and one with markup: bold _text_.

+
    +
  • In a list item: *x* and [y]
    • +
+

In a callout: *x*

 +
Pipe cell
*x* and a|b
+

Raw cell: *x*

+

Layout cell: *x*

 diff --git a/internal/convert/testdata/storage2md/escaping/output.md b/internal/convert/testdata/storage2md/escaping/output.md new file mode 100644 index 0000000..680dad6 --- /dev/null +++ b/internal/convert/testdata/storage2md/escaping/output.md @@ -0,0 +1,46 @@ +Emphasis: \*not emphasis\*, 2\*3\*4, a \_b\_ c, and a * b * c stays. + +Strikethrough: a ~b\~ c, a \~\~b\~\~ c, and about ~5 min stays. + +Code: \`not code\`. + +HTML: \not bold\, \, \not storage\, and a < b stays. + +Entities: \© and \©, and AT&T stays. + +Brackets: \[x], \[x]\[y], and x] stays. + +Backslashes: a\\\*b, and C:\path stays. + +A link: [Q1 \*Draft\*\]](https://example.com), and one with markup: [**bold** \_text\_](https://example.com). + +- In a list item: \*x\* and \[y] + +> [!NOTE] +> In a callout: \*x\* + +| Pipe cell | +| --- | +| \*x\* and a\|b | + + + + + + + +
+ +Raw cell: \*x\* + +
+ + + + + +Layout cell: \*x\* + + + + From 99a83bac70e3a4da842aca54193c0440ed8b7af8 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:25:29 -0400 Subject: [PATCH 3/9] fix(convert): escape block markers at a line start on read Text that starts a line with a block marker opened that block on the next publish: "

1. not a list

" became a list, "# x" a heading, "> 90 days" a blockquote, and "---" after a hard break turned the line above into a setext heading. escapeLineStarts runs over each paragraph's rendered text, split at hard breaks, and escapes a marker at the start of each line: ordered and bulleted list markers, ATX headings, blockquotes, setext underlines, thematic breaks, fences and table delimiter rows. It runs on rendered Markdown because only the assembled paragraph knows where its lines start, which is safe because nothing markfluence emits inline begins with a block marker (_plans/056, D3/D4). It applies to paragraphs, list-item text, loose text, and raw cells' loose runs. A heading's trailing " #" was read as a closing sequence and dropped, so "## Item #" published as "Item"; its first "#" is now escaped. Fixes #203. --- internal/convert/escape.go | 63 +++++++++++++++++ internal/convert/escape_test.go | 68 +++++++++++++++++++ internal/convert/storage_to_md.go | 11 +-- internal/convert/storage_to_md_table.go | 2 +- .../storage2md/escaping/input.storage | 7 ++ .../testdata/storage2md/escaping/output.md | 39 +++++++++++ 6 files changed, 184 insertions(+), 6 deletions(-) diff --git a/internal/convert/escape.go b/internal/convert/escape.go index 309400d..11be38c 100644 --- a/internal/convert/escape.go +++ b/internal/convert/escape.go @@ -114,3 +114,66 @@ func isAlnum(r rune) bool { return r != -1 && (unicode.IsLetter(r) || unicode.Is func isASCIIPunct(r rune) bool { return r < 128 && strings.ContainsRune("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", r) } + +// escapeLineStarts escapes a block marker at the start of each line of a +// paragraph's rendered text, where a line starts at the paragraph's start and +// after every hard break. A marker there would open a block on publish: "1. x" +// a list, "# x" a heading, "---" under a line a setext heading (_plans/056, +// D3). It runs over rendered Markdown rather than text nodes, because only the +// assembled paragraph knows where its lines start; that is safe because +// nothing markfluence emits inline begins with a block marker (D4). It splits +// only at hard breaks, so raw storage holding a newline is never touched. +func escapeLineStarts(s string) string { + lines := strings.Split(s, hardBreak) + for i, line := range lines { + lines[i] = escapeLineStart(line) + } + return strings.Join(lines, hardBreak) +} + +// escapeLineStart escapes the block marker, if any, that line starts with. +func escapeLineStart(line string) string { + body := strings.TrimLeft(line, " ") + indent := line[:len(line)-len(body)] + if m := orderedMarkerRE.FindStringSubmatchIndex(body); m != nil { + // Escape the delimiter: "1\. x". + return indent + body[:m[2]] + `\` + body[m[2]:] + } + for _, re := range blockStartREs { + if re.MatchString(body) { + return indent + `\` + body + } + } + return line +} + +// orderedMarkerRE matches an ordered list marker; its group is the delimiter. +var orderedMarkerRE = regexp.MustCompile(`^[0-9]{1,9}([.)])(?: |$)`) + +// blockStartREs are the other lines that open a block, each escaped by a +// backslash before its first character. +var blockStartREs = []*regexp.Regexp{ + regexp.MustCompile(`^#{1,6}(?: |$)`), // an ATX heading + regexp.MustCompile(`^>`), // a blockquote, space or not + regexp.MustCompile(`^[-+*](?: |$)`), // a bulleted list item + regexp.MustCompile(`^(?:-+|=+) *$`), // a setext underline or thematic break + regexp.MustCompile(`^(?:[-*_] *){3,}$`), // a thematic break + regexp.MustCompile(`^(?:~~~|` + "```" + `)`), // a code fence + // A table delimiter row, under a line that then becomes the header. + regexp.MustCompile(`^[|:-][|: -]*$`), +} + +// escapeHeadingClose escapes a trailing run of "#" after whitespace in a +// heading's text, which ATX headings read as a closing sequence and drop: +// "## Item #" publishes as "Item" (_plans/056, D6). "C#" has no whitespace +// before its "#" and is not a closing sequence. +func escapeHeadingClose(s string) string { + if m := headingCloseRE.FindStringIndex(s); m != nil { + i := m[0] + len(strings.TrimRight(s[m[0]:], "#")) + return s[:i] + `\` + s[i:] + } + return s +} + +// headingCloseRE matches whitespace then a run of "#" ending the text. +var headingCloseRE = regexp.MustCompile(`\s#+$`) diff --git a/internal/convert/escape_test.go b/internal/convert/escape_test.go index 1e618bc..346ff2a 100644 --- a/internal/convert/escape_test.go +++ b/internal/convert/escape_test.go @@ -2,6 +2,7 @@ package convert_test import ( "html" + "regexp" "strings" "testing" @@ -83,3 +84,70 @@ func TestMentionMarkdownEscapesTheName(t *testing.T) { t.Errorf("MentionMarkdown = %q, want %q", got, want) } } + +// lineStartEscapes are storage paragraphs whose text would open a block at a +// line start, and the Markdown read must write for each. +var lineStartEscapes = []struct{ storage, want string }{ + {`

1. not a list

`, `1\. not a list`}, + {`

1) not a list

`, `1\) not a list`}, + {`

10. not a list

`, `10\. not a list`}, + {`

- not a list

`, `\- not a list`}, + {`

+ not a list

`, `\+ not a list`}, + {`

* not a list

`, `\* not a list`}, + {`

# not a heading

`, `\# not a heading`}, + {`

> 5 errors

`, `\> 5 errors`}, + {`

>90 days

`, `\>90 days`}, + {`

---

`, `\---`}, + {`

- - -

`, `\- - -`}, + {`

***

`, `\*\*\*`}, + {`

~~~

`, `\~\~\~`}, + {`

a
---

`, "a \n\\---"}, + {`

a
===

`, "a \n\\==="}, + {`

a | b
--- | ---

`, "a | b \n\\--- | ---"}, + {`

a
# b

`, "a \n\\# b"}, + {`

a
2. b

`, "a \n2\\. b"}, + {`

a
1. b

`, "**a \n1\\. b**"}, + {`

#hashtag and C#

`, `#hashtag and C#`}, + {`

-1 is negative

`, `-1 is negative`}, + {`
  • 1. not nested
`, `- 1\. not nested`}, + {`
  • > not a quote
`, `- \> not a quote`}, + {`
  • [ ] not a task
`, `- \[ ] not a task`}, + {`

Item #

`, `## Item \#`}, + {`

Item ##

`, `## Item \##`}, + {`

C#

`, `## C#`}, +} + +// blockTagRE matches the tags a paragraph's text must never publish as. +var blockTagRE = regexp.MustCompile(`<(ol|ul|h[1-6]|blockquote|hr|table|ac:structured-macro|input)\b`) + +func TestEscapeLineStarts(t *testing.T) { + for _, c := range lineStartEscapes { + t.Run(c.storage, func(t *testing.T) { + md, err := convert.StorageToMarkdown(c.storage, convert.StorageOptions{}) + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(md); got != c.want { + t.Errorf("read = %q, want %q", got, c.want) + } + published := publish(t, md) + if tag := blockTagRE.FindString(strings.ReplaceAll(published, "<"+firstTag(c.storage), "")); tag != "" { + t.Errorf("%q publishes a %s: %q", md, tag, published) + } + again, err := convert.StorageToMarkdown(published, convert.StorageOptions{}) + if err != nil { + t.Fatal(err) + } + if again != md { + t.Errorf("not a fixed point: %q, then %q", md, again) + } + }) + } +} + +// firstTag is the name of the storage's own outermost element, which the +// published form is allowed to contain. +func firstTag(storage string) string { + name, _, _ := strings.Cut(strings.TrimPrefix(storage, "<"), ">") + return name +} diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 2e776e5..4f68a45 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -278,7 +278,7 @@ func (r *mdRenderer) blockStrings(kids []*snode, listIndent string) []string { for _, k := range kids { if k.name == "" { if s := strings.TrimSpace(collapse(k.text)); s != "" { - out = append(out, escapeText(s, false)) + out = append(out, escapeLineStarts(escapeText(s, false))) } continue } @@ -296,10 +296,11 @@ func (r *mdRenderer) renderBlock(n *snode, listIndent string) string { level := int(n.name[1] - '0') // 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, "
") + text := escapeHeadingClose(r.renderInlineChildren(n)) + text = strings.ReplaceAll(text, hardBreak, "
") return strings.Repeat("#", level) + " " + text case "p": - return r.renderInlineChildren(n) + return escapeLineStarts(r.renderInlineChildren(n)) case "ul": return r.renderList(n, false, listIndent) case "ol": @@ -322,7 +323,7 @@ func (r *mdRenderer) renderBlock(n *snode, listIndent string) string { case "ac:image", "ac:link", "a", "strong", "b", "em", "i", "code", "del", "s", "strike", "br": // An inline element sitting at block level (Confluence often emits a bare // not wrapped in

) is rendered as its own paragraph. - return r.renderInline(n) + return escapeLineStarts(r.renderInline(n)) case "ac:layout", "ac:layout-section", "ac:layout-cell": return r.renderRawBlock(n) case "div": @@ -391,7 +392,7 @@ func (r *mdRenderer) renderListItem(li *snode, cont string) string { flushLine := func() { flushRun() if s := strings.TrimSpace(line.String()); s != "" { - segs = append(segs, itemSeg{text: s, kind: segText}) + segs = append(segs, itemSeg{text: escapeLineStarts(s), kind: segText}) } line.Reset() } diff --git a/internal/convert/storage_to_md_table.go b/internal/convert/storage_to_md_table.go index 0e7d98d..fc8db1b 100644 --- a/internal/convert/storage_to_md_table.go +++ b/internal/convert/storage_to_md_table.go @@ -539,7 +539,7 @@ func (r *mdRenderer) rawCellBlocks(c *snode) []string { var blocks []string flush := func() { if s := strings.TrimSpace(r.renderInlineChildren(&snode{kids: run})); s != "" { - blocks = append(blocks, s) + blocks = append(blocks, escapeLineStarts(s)) } run = nil } diff --git a/internal/convert/testdata/storage2md/escaping/input.storage b/internal/convert/testdata/storage2md/escaping/input.storage index f797ff0..958cb31 100644 --- a/internal/convert/testdata/storage2md/escaping/input.storage +++ b/internal/convert/testdata/storage2md/escaping/input.storage @@ -13,3 +13,10 @@
Pipe cell
*x* and a|b

Raw cell: *x*

Layout cell: *x*

+

1. not a list

+

# not a heading, and a line after a break:
# not a heading either

+

> 90 days

+

- not a list in a callout

 +

1. not a list in a raw cell

> loose text in a raw cell
+

+ not a list in a layout cell

 +

Item #

diff --git a/internal/convert/testdata/storage2md/escaping/output.md b/internal/convert/testdata/storage2md/escaping/output.md index 680dad6..59cfb9a 100644 --- a/internal/convert/testdata/storage2md/escaping/output.md +++ b/internal/convert/testdata/storage2md/escaping/output.md @@ -44,3 +44,42 @@ Layout cell: \*x\* + +1\. not a list + +\# not a heading, and a line after a break: +\# not a heading either + +\> 90 days + +> [!WARNING] +> \- not a list in a callout + + + + + + + + +
+ +1\. not a list in a raw cell + + + +\> loose text in a raw cell + +
+ + + + + +\+ not a list in a layout cell + + + + + +## Item \# From a9f2315b96452320d03dc13d3aab3f2b33f71eba Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:26:37 -0400 Subject: [PATCH 4/9] fix(convert): keep bare URLs as text on read GFM autolinks a bare URL, a www. address and an email address, so plain text holding one published as a link. Storage holds a URL as plain text only when someone chose that -- the editor turns a typed URL into a link -- so the escape keeps it text: "https\://", "www\.", "a\@b.com". It reads worse in an exported file, which docs will say (_plans/056, D7). An "@" at a text node's start is left alone, the one exception to escaping at an edge: a link whose text starts with "@" is how a mention is written (#91). --- internal/convert/escape.go | 36 ++++++++++++++++++- internal/convert/escape_test.go | 8 +++++ .../storage2md/escaping/input.storage | 1 + .../testdata/storage2md/escaping/output.md | 2 ++ 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/internal/convert/escape.go b/internal/convert/escape.go index 11be38c..eb6e3bc 100644 --- a/internal/convert/escape.go +++ b/internal/convert/escape.go @@ -27,7 +27,7 @@ import ( // inLink is set inside a link's text, where a "]" would end the text early and // must be escaped too; elsewhere a lone "]" is inert and common in prose. func escapeText(s string, inLink bool) string { - if !strings.ContainsAny(s, "\\`*_~[]<&") { + if !strings.ContainsAny(s, "\\`*_~[]<&:.@") { return s } rs := []rune(s) @@ -92,10 +92,44 @@ func needsEscape(rs []rune, i int, inLink bool) bool { case '&': // An entity reference publishes as its character; "AT&T" is text. return next == -1 || entityRE.MatchString(string(rs[i:min(len(rs), i+maxEntity)])) + case ':', '.', '@': + return autolinks(rs, i) } return false } +// autolinks reports whether rs[i] is the character that makes GFM autolink a +// bare URL or email address: the ":" of "https://", the "." of a "www." that +// starts a word, or the "@" of an address. Escaping it keeps plain text plain; +// a URL stored as text rather than as a link is someone's choice, and the +// editor turns a typed one into a link (_plans/056, D7). +// +// An "@" at the node's start is the one character left alone at an edge: a +// link whose text starts with "@" is how a mention is written (#91), and +// escaping it would turn every such link into a plain profile link. +func autolinks(rs []rune, i int) bool { + before := strings.ToLower(string(rs[max(0, i-5):i])) + switch rs[i] { + case ':': + after := string(rs[i+1 : min(len(rs), i+3)]) + return after == "//" && (strings.HasSuffix(before, "http") || + strings.HasSuffix(before, "https") || strings.HasSuffix(before, "ftp")) + case '.': + if !strings.HasSuffix(before, "www") { + return false + } + return i == 3 || !isAlnum(rs[i-4]) + case '@': + return i > 0 && i < len(rs)-1 && isEmailLocal(rs[i-1]) && isAlnum(rs[i+1]) + } + return false +} + +// isEmailLocal reports whether r may end the local part of an email address. +func isEmailLocal(r rune) bool { + return isAlnum(r) || strings.ContainsRune(".!#$%&'*+/=?^_`{|}~-", r) +} + // entityRE matches a character reference at the start of a string: named, // decimal or hexadecimal, as CommonMark recognises them. var entityRE = regexp.MustCompile(`^&(?:#[0-9]{1,7}|#[xX][0-9a-fA-F]{1,6}|[A-Za-z][A-Za-z0-9]{1,31});`) diff --git a/internal/convert/escape_test.go b/internal/convert/escape_test.go index 346ff2a..93cfaa8 100644 --- a/internal/convert/escape_test.go +++ b/internal/convert/escape_test.go @@ -41,6 +41,14 @@ var inlineEscapes = []escapeCase{ {`C:\path`, `C:\path`, "a backslash before a letter is literal"}, {`a\*b`, `a\\\*b`, "a backslash before punctuation is an escape"}, {`a\`, `a\\`, "a backslash at the edge could meet punctuation"}, + {`see https://x.com.`, `see https\://x.com.`, "a bare URL autolinks"}, + {`HTTP://X.COM and ftp://x`, `HTTP\://X.COM and ftp\://x`, "every scheme goldmark links"}, + {`www.x.com`, `www\.x.com`, "a www. at a word start autolinks"}, + {`awww.x`, `awww.x`, "not at a word start"}, + {`mail a@b.com`, `mail a\@b.com`, "an email address autolinks"}, + {`@alice`, `@alice`, "an @ at the edge is a mention marker"}, + {`x @ y`, `x @ y`, "not an address"}, + {`at 10:30, http: fine`, `at 10:30, http: fine`, "a colon not before //"}, } func TestEscapeText(t *testing.T) { diff --git a/internal/convert/testdata/storage2md/escaping/input.storage b/internal/convert/testdata/storage2md/escaping/input.storage index 958cb31..087a345 100644 --- a/internal/convert/testdata/storage2md/escaping/input.storage +++ b/internal/convert/testdata/storage2md/escaping/input.storage @@ -20,3 +20,4 @@

1. not a list in a raw cell

> loose text in a raw cell

+ not a list in a layout cell

Item #

+

Plain text, not links: https://example.com, www.example.com and ops@example.com.

diff --git a/internal/convert/testdata/storage2md/escaping/output.md b/internal/convert/testdata/storage2md/escaping/output.md index 59cfb9a..3fb9898 100644 --- a/internal/convert/testdata/storage2md/escaping/output.md +++ b/internal/convert/testdata/storage2md/escaping/output.md @@ -83,3 +83,5 @@ Layout cell: \*x\* ## Item \# + +Plain text, not links: https\://example.com, www\.example.com and ops\@example.com. From f88cfb6f89da1bc939c74e1b33611ff665178525 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:34:55 -0400 Subject: [PATCH 5/9] fix(convert): escape consistently across a read and the next Letting the table property test write Markdown-significant text found five ways the escaping so far could change from one read to the next, or miss text entirely: - Whitespace at a text node's edge now counts as unknown, like the edge itself. renderMark moves a mark's edge whitespace outside it and publishing stores it in the next node, so trusting it escaped "\" in one read and not the next -- and a backslash before a space the mark gave away went on to escape the closing delimiter. - Adjacent text nodes are merged before rendering. coalesceSplitMarks leaves two when it merges two runs of one mark, and the split moved an edge into the middle of what publishing writes back as one node. - A thematic break must repeat one character, as CommonMark requires. The looser pattern matched "**---**", bold markup markfluence emits. - A heading that is nothing but "#" is an empty heading with a closing sequence, so its "#" is escaped too. - Raw storage written inline -- an inline macro, a list in a pipe-table cell, an passed through -- has its text parsed as Markdown between the tags, so "_x_" in a status macro's title published as . serializeInline escapes those text nodes; a raw block needs nothing, since an HTML block is not parsed. Part of #203. --- internal/convert/aclink.go | 14 ++-- internal/convert/escape.go | 64 +++++++++++++++---- internal/convert/escape_test.go | 2 + internal/convert/storage_to_md.go | 45 +++++++++++-- .../storage2md/escaping/input.storage | 5 ++ .../testdata/storage2md/escaping/output.md | 12 ++++ 6 files changed, 115 insertions(+), 27 deletions(-) diff --git a/internal/convert/aclink.go b/internal/convert/aclink.go index 6e82b2d..4e3ef6f 100644 --- a/internal/convert/aclink.go +++ b/internal/convert/aclink.go @@ -218,7 +218,7 @@ func (r *mdRenderer) renderACLink(n *snode) string { return r.renderAnchorLink(n, anchor) } // No target and no anchor: there is nothing to point at. - return serialize(n) + return serializeInline(n) case target.name == "ri:page": return r.renderPageLink(n, target, anchor) case target.name == "ri:space": @@ -230,7 +230,7 @@ func (r *mdRenderer) renderACLink(n *snode) string { // images are uploaded (images.go); ri:blog-post cannot be resolved to // an id, because SearchPagesByTitle does not see blog posts. Both // round-trip exactly as storage. - return serialize(n) + return serializeInline(n) } } @@ -249,7 +249,7 @@ func acLinkTarget(n *snode) *snode { func (r *mdRenderer) renderPageLink(n, target *snode, anchor string) string { href, ok := r.pageLinks[pageTarget(target)] if !ok { - return serialize(n) + return serializeInline(n) } if anchor != "" { // Appended verbatim, still percent-encoded: it is a URL fragment, and @@ -264,7 +264,7 @@ func (r *mdRenderer) renderPageLink(n, target *snode, anchor string) string { func (r *mdRenderer) renderSpaceLink(n, target *snode) string { key := target.attrs["ri:space-key"] if key == "" || r.siteURL == "" { - return serialize(n) + return serializeInline(n) } return mdLink(r.acLinkText(n, key), r.siteURL+"/wiki/spaces/"+key) } @@ -348,11 +348,11 @@ const unknownUserName = "Unlicensed user" func (r *mdRenderer) renderUserMention(n, target *snode) string { id := target.attrs["ri:account-id"] if id == "" { - return serialize(n) + return serializeInline(n) } name, known := r.userNames[id] if !known { - return serialize(n) + return serializeInline(n) } if name == "" { name = unknownUserName @@ -366,7 +366,7 @@ func (r *mdRenderer) renderAnchorLink(n *snode, anchor string) string { if !ok { // A "#slug" matching no heading publishes as a dead relative href, and // the forward path says nothing about it. Keep the storage, which works. - return serialize(n) + return serializeInline(n) } return mdLink(r.acLinkText(n, anchor), "#"+slug) } diff --git a/internal/convert/escape.go b/internal/convert/escape.go index eb6e3bc..8bb02f4 100644 --- a/internal/convert/escape.go +++ b/internal/convert/escape.go @@ -27,12 +27,30 @@ import ( // inLink is set inside a link's text, where a "]" would end the text early and // must be escaped too; elsewhere a lone "]" is inert and common in prose. func escapeText(s string, inLink bool) string { + return escapeFor(s, inLink, false) +} + +// escapeRawText is escapeText for a text node inside raw storage written +// inline -- an inline macro, a list in a pipe-table cell, an passed +// through. The text between inline HTML tags is still Markdown, so "_x_" in a +// status macro's title published as (_plans/056, amended). "<" and "&" +// are left to xmlTextEscape, which writes both as entities. +func escapeRawText(s string) string { + return escapeFor(s, false, true) +} + +// escapeFor is escapeText, and escapeRawText when inRaw is set. +func escapeFor(s string, inLink, inRaw bool) string { if !strings.ContainsAny(s, "\\`*_~[]<&:.@") { return s } rs := []rune(s) var b strings.Builder for i, c := range rs { + if inRaw && (c == '<' || c == '&') { + b.WriteRune(c) + continue + } if needsEscape(rs, i, inLink) { b.WriteByte('\\') } @@ -42,13 +60,19 @@ func escapeText(s string, inLink bool) string { } // needsEscape reports whether rs[i] must be escaped. +// +// A neighbour is unknown (-1) past the node's edge, and also when it is +// whitespace at the node's edge: that whitespace is not reliably this node's. +// renderMark moves a mark's edge whitespace outside its delimiters and +// publishing then stores it in the neighbouring node, so a decision that +// trusted it would differ between a read and the read after it -- "\" before +// a space the mark gives away would go unescaped and escape the delimiter. func needsEscape(rs []rune, i int, inLink bool) bool { - // prev and next are -1 at the node's edge: unknown. prev, next := rune(-1), rune(-1) - if i > 0 { + if i > 0 && !edgeSpace(rs[:i]) { prev = rs[i-1] } - if i < len(rs)-1 { + if i < len(rs)-1 && !edgeSpace(rs[i+1:]) { next = rs[i+1] } switch rs[i] { @@ -134,6 +158,17 @@ func isEmailLocal(r rune) bool { // decimal or hexadecimal, as CommonMark recognises them. var entityRE = regexp.MustCompile(`^&(?:#[0-9]{1,7}|#[xX][0-9a-fA-F]{1,6}|[A-Za-z][A-Za-z0-9]{1,31});`) +// edgeSpace reports whether rs is nothing but whitespace, so that whitespace +// beside a character reaches all the way to the node's edge. +func edgeSpace(rs []rune) bool { + for _, r := range rs { + if !unicode.IsSpace(r) { + return false + } + } + return true +} + // maxEntity bounds how far entityRE can match: "&", a 32-character name, ";". const maxEntity = 34 @@ -187,20 +222,21 @@ var orderedMarkerRE = regexp.MustCompile(`^[0-9]{1,9}([.)])(?: |$)`) // blockStartREs are the other lines that open a block, each escaped by a // backslash before its first character. var blockStartREs = []*regexp.Regexp{ - regexp.MustCompile(`^#{1,6}(?: |$)`), // an ATX heading - regexp.MustCompile(`^>`), // a blockquote, space or not - regexp.MustCompile(`^[-+*](?: |$)`), // a bulleted list item - regexp.MustCompile(`^(?:-+|=+) *$`), // a setext underline or thematic break - regexp.MustCompile(`^(?:[-*_] *){3,}$`), // a thematic break - regexp.MustCompile(`^(?:~~~|` + "```" + `)`), // a code fence + regexp.MustCompile(`^#{1,6}(?: |$)`), // an ATX heading + regexp.MustCompile(`^>`), // a blockquote, space or not + regexp.MustCompile(`^[-+*](?: |$)`), // a bulleted list item + regexp.MustCompile(`^(?:-+|=+) *$`), // a setext underline or thematic break + regexp.MustCompile(`^(?:(?:\* *){3,}|(?:- *){3,}|(?:_ *){3,})$`), // a thematic break: one character, repeated + regexp.MustCompile(`^(?:~~~|` + "```" + `)`), // a code fence // A table delimiter row, under a line that then becomes the header. regexp.MustCompile(`^[|:-][|: -]*$`), } // escapeHeadingClose escapes a trailing run of "#" after whitespace in a // heading's text, which ATX headings read as a closing sequence and drop: -// "## Item #" publishes as "Item" (_plans/056, D6). "C#" has no whitespace -// before its "#" and is not a closing sequence. +// "## Item #" publishes as "Item" (_plans/056, D6), and "### #" as an empty +// heading. "C#" has no whitespace before its "#" and is not a closing +// sequence. func escapeHeadingClose(s string) string { if m := headingCloseRE.FindStringIndex(s); m != nil { i := m[0] + len(strings.TrimRight(s[m[0]:], "#")) @@ -209,5 +245,7 @@ func escapeHeadingClose(s string) string { return s } -// headingCloseRE matches whitespace then a run of "#" ending the text. -var headingCloseRE = regexp.MustCompile(`\s#+$`) +// headingCloseRE matches a run of "#" ending the text after whitespace, or +// making up the whole text: "### #" is an empty heading with a closing +// sequence. +var headingCloseRE = regexp.MustCompile(`(?:^|\s)#+$`) diff --git a/internal/convert/escape_test.go b/internal/convert/escape_test.go index 93cfaa8..cbfd1dd 100644 --- a/internal/convert/escape_test.go +++ b/internal/convert/escape_test.go @@ -116,6 +116,7 @@ var lineStartEscapes = []struct{ storage, want string }{ {`

a
2. b

`, "a \n2\\. b"}, {`

a
1. b

`, "**a \n1\\. b**"}, {`

#hashtag and C#

`, `#hashtag and C#`}, + {`

---

`, `**---**`}, {`

-1 is negative

`, `-1 is negative`}, {`
  • 1. not nested
`, `- 1\. not nested`}, {`
  • > not a quote
`, `- \> not a quote`}, @@ -123,6 +124,7 @@ var lineStartEscapes = []struct{ storage, want string }{ {`

Item #

`, `## Item \#`}, {`

Item ##

`, `## Item \##`}, {`

C#

`, `## C#`}, + {`

#

`, `### \#`}, } // blockTagRE matches the tags a paragraph's text must never publish as. diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 4f68a45..d6d1485 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -553,10 +553,11 @@ func (r *mdRenderer) renderCellLines(c *snode) string { // expressed as Markdown list syntax inside a cell at all. // Passthrough as the raw tags, which markfluence's own write side // already accepts typed directly into a cell (goldmark's raw HTML - // passes through unchanged), keeps it exact instead of running + // passes through, though the text between the tags is still + // Markdown and is escaped), keeps it exact instead of running // every item together the way rendering it as inline content would. flush() - lines = append(lines, serialize(k)) + lines = append(lines, serializeInline(k)) isList[len(lines)-1] = true default: run = append(run, k) @@ -608,7 +609,7 @@ func (r *mdRenderer) renderMacro(n *snode, block bool) string { case block: return r.renderRawBlock(n) default: - return serialize(n) + return serializeInline(n) } } @@ -655,7 +656,7 @@ func (r *mdRenderer) renderInlineChildren(n *snode) string { // must see the whitespace at its own edges to move it outside its delimiters. func (r *mdRenderer) renderInlineRun(n *snode) string { var buf []byte - for _, k := range coalesceSplitMarks(n.kids) { + for _, k := range mergeText(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 @@ -691,6 +692,23 @@ func (r *mdRenderer) renderInlineRun(n *snode) string { return string(buf) } +// mergeText joins adjacent text nodes into one. escapeText judges a character +// by its neighbours within its node, so the same text split in two -- which +// coalesceSplitMarks produces when it merges two runs of one mark -- could be +// escaped differently from the single node publishing writes back, and the +// Markdown would not be a fixed point. +func mergeText(kids []*snode) []*snode { + out := make([]*snode, 0, len(kids)) + for _, k := range kids { + if n := len(out); n > 0 && k.name == "" && out[n-1].name == "" { + out[n-1] = &snode{text: out[n-1].text + k.text} + continue + } + out = append(out, k) + } + return out +} + // 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. @@ -906,7 +924,7 @@ func (r *mdRenderer) renderInline(n *snode) string { case "ac:adf-extension": // Likewise for an inline ADF extension, whose transparent-wrapper default // would otherwise render the node and the fallback one after the other. - return serialize(adfPassthrough(n)) + return serializeInline(adfPassthrough(n)) default: // A wrapper Markdown has no syntax for (a coloured , , ) // keeps its edge whitespace for the run around it to settle: trimming it @@ -1053,8 +1071,21 @@ func jstr(s string) string { // serialize re-emits a node as storage XML, for passing an unknown macro through // unchanged (MdToConfluence's shield re-publishes it verbatim). func serialize(n *snode) string { + return serializeWith(n, xmlTextEscape) +} + +// serializeInline is serialize for raw storage written inline, inside a +// paragraph or a pipe-table cell. goldmark parses the text between inline HTML +// tags as Markdown, so each text node is escaped as well (#203). A raw block +// needs none of this: an HTML block's content is not parsed. +func serializeInline(n *snode) string { + return serializeWith(n, func(s string) string { return xmlTextEscape(escapeRawText(s)) }) +} + +// serializeWith is serialize with text nodes written by text. +func serializeWith(n *snode, text func(string) string) string { if n.name == "" { - return xmlTextEscape(n.text) + return text(n.text) } var b strings.Builder b.WriteString("<" + n.name + attrString(n.attrs)) @@ -1064,7 +1095,7 @@ func serialize(n *snode) string { } b.WriteString(">") for _, k := range n.kids { - b.WriteString(serialize(k)) + b.WriteString(serializeWith(k, text)) } b.WriteString("") return b.String() diff --git a/internal/convert/testdata/storage2md/escaping/input.storage b/internal/convert/testdata/storage2md/escaping/input.storage index 087a345..c32dc83 100644 --- a/internal/convert/testdata/storage2md/escaping/input.storage +++ b/internal/convert/testdata/storage2md/escaping/input.storage @@ -21,3 +21,8 @@

+ not a list in a layout cell

Item #

Plain text, not links: https://example.com, www.example.com and ops@example.com.

+

A status macro: _x_ *y* stays raw.

+
A list in a pipe cell
  • _x_ and [y]
+

A backslash before a mark's moved space: a \ x, and two runs of one mark: \ down.

+

---

+

#

diff --git a/internal/convert/testdata/storage2md/escaping/output.md b/internal/convert/testdata/storage2md/escaping/output.md index 3fb9898..d26ab49 100644 --- a/internal/convert/testdata/storage2md/escaping/output.md +++ b/internal/convert/testdata/storage2md/escaping/output.md @@ -85,3 +85,15 @@ Layout cell: \*x\* ## Item \# Plain text, not links: https\://example.com, www\.example.com and ops\@example.com. + +A status macro: \_x\_ \*y\* stays raw. + +| A list in a pipe cell | +| --- | +|
  • \_x\_ and \[y]
| + +A backslash before a mark's moved space: a \\ *x*, and two runs of one mark: *\ down*. + +**---** + +### \# From f8cc1708f16b689d9d94266d0922a69266769698 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Mon, 28 Sep 2026 15:34:55 -0400 Subject: [PATCH 6/9] test(convert): let the table property test write Markdown syntax The generator's words now include text that spells Markdown -- block markers, emphasis, tildes, backticks, brackets, a tag, an entity, a backslash, a URL and an address -- which it avoided until #203. Against the converter from before #203 it fails at seed 0. Two older gaps stay out of its way: a code span never holds a backtick, since read writes one with a single backtick whatever it holds, and a mark's text never starts or ends with punctuation (#216). --- internal/convert/table_property_test.go | 41 +++++++++++++++++++------ 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/internal/convert/table_property_test.go b/internal/convert/table_property_test.go index 50dbed8..e11ef4a 100644 --- a/internal/convert/table_property_test.go +++ b/internal/convert/table_property_test.go @@ -554,13 +554,12 @@ func modelHasElement(n *snode) bool { // tableGen generates table storage from a seed. The attribute vocabulary is // what the live survey found (layouts, colgroups, colours, alignments, ids, // valign, class), weighted towards tables a pipe table can hold, so both paths -// get exercised. Text avoids a leading Markdown block marker ("1. ", "# "), -// which #203 covers, and inline tags read has no Markdown for (