From 3e8e97f02c9e39714adc195299d8499a5b5f5169 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Tue, 2 Jun 2026 01:31:15 +0300 Subject: [PATCH] fix: round-trip fidelity for inline TOC macros, nbsp emphasis, and strikethrough Surfaced by `verify` on outside-doc-core-v2.xml (real Confluence XML). - A TOC macro inside a heading round-trips as `# `, where goldmark parses the comment as inline RawHTML. renderRawHTML now calls convertComment (like the block path) so the macro is reconstructed instead of leaking back out as a literal comment. - Confluence stores structured-macro attributes in no fixed order (TOC is macro-id,name,schema-version on one page, name,schema-version,macro-id on another) and the verify formatter preserves order, so no reconstruction order matches everything; sortMacroAttrs canonicalizes them on both sides. - Bold applied to a lone nbsp (` `) emitted `** **`: the writeEmphasis trim cutset now includes U+00A0, and normalizeForVerify drops whitespace-only emphasis. - `` and `` (both strikethrough) are canonicalized to ``. Adds regression tests for each and documents the gotchas in CLAUDE.md. --- CLAUDE.md | 4 +++- cmd/mdcx/verify.go | 50 ++++++++++++++++++++++++++++++++++++++++ cmd/mdcx/verify_test.go | 19 +++++++++++++++ confluence/renderer.go | 38 +++++++++++++++++++----------- converter/md2xml_test.go | 28 ++++++++++++++++++++++ converter/xml2md.go | 8 +++++-- 6 files changed, 130 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 152d179ea8bce407b27668e2e3cfd1d8e414f367..55bb44c088f8c3827fff9eac14ff13fd6d145f51 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,11 +65,13 @@ These are subtle traps the round-trip has been bitten by; touching the related c - **Nested-list indent depends on the *parent* list type** — CommonMark places content of `1. ` at column 4 and `- ` at column 3, so a nested item must be indented by ≥ the parent marker's width. `xmlConverter.listIndents` is a stack of per-level widths (3 for `
    `, 2 for `
      ` / task-list); `itemIndent()` sums all *ancestor* contributions. Using a fixed `2*(depth-1)` collapses nested ordered lists into siblings. *Tests:* `TestRoundTrip_NestedOrderedList`, `TestRoundTrip_NestedMixedList`, `TestRoundTrip_DoubleNestedUnorderedList`. - **Tight lists are one stray `\n` away from loose** — a blank line anywhere in a list turns it loose, which makes goldmark wrap every `
    • ` in `

      `. `ensureSingleNewline()` writes `\n` only when the buffer doesn't already end in one, preventing the double newline that appeared when a `

    • `'s body ended with a nested list (which itself terminated in `\n`). Always use it instead of `c.buf.WriteString("\n")` after walking list-item content. - **Literal angle-bracket text turns into phantom elements** — text stored as `<table>` (a function signature like `box.backup.start()`) decodes to `
      ` in the text node; emitted raw into Markdown, goldmark parses it as an HTML table and the phantom element swallows the rest of the document. `xml2md.escapeMarkdownAngle` backslash-escapes a *tag-like* `<` (followed by a letter, `/`, `!`, `?`) in **both** text paths — the `walk` TextNode branch *and* `walkChildrenInline` (used for `
    • ` bodies, a separate branch that's easy to miss). The matching unescape is in `confluence.Renderer.renderText`: it strips only `\<` (goldmark keeps a backslash-escaped run as one Text node with the backslash intact, and this renderer emits the raw segment) — **never** a blanket `util.UnescapePunctuations`, which would also eat the deliberate `\|` table-pipe escaping and re-break `base.xml`. A non-tag `<` (`a < b`, `<=`) is left alone. *Test:* `TestRoundTrip_LiteralAngleBracketText`. -- **Emphasis can't be flanked by whitespace** — ` x ` (space just inside the tag) renders as literal `** x **` because CommonMark requires the run to be flanked by non-whitespace. `xml2md.writeEmphasis` (used for strong/em/del) captures the children into a scratch buffer and moves leading/trailing whitespace *outside* the markers (` **x** `). The stored and round-tripped forms then differ only in space placement, which `normalizeForVerify` reconciles (`reEmphLeadWS`/`reEmphTrailWS`). *Test:* `TestRoundTrip_EmphasisInteriorSpaces`. +- **Emphasis can't be flanked by whitespace** — ` x ` (space just inside the tag) renders as literal `** x **` because CommonMark requires the run to be flanked by non-whitespace. `xml2md.writeEmphasis` (used for strong/em/del) captures the children into a scratch buffer and moves leading/trailing whitespace *outside* the markers (` **x** `). Its trim cutset **must include U+00A0** — Confluence stores stray bold applied to a lone nbsp (` `), and an nbsp left as content emits `** **`, which round-trips as literal `** **`; `normalizeForVerify` drops the empty emphasis (`reEmptyStrong`/`reEmptyEm`/`reEmptyDel`) and reconciles the surviving space placement (`reEmphLeadWS`/`reEmphTrailWS`). *Tests:* `TestRoundTrip_EmphasisInteriorSpaces`, `TestRoundTrip_EmphasisNbspOnly`. +- **A macro marker comment can be inline, not just block** — a TOC macro that lived inside a heading (`

      `) round-trips as `# `, where goldmark parses the comment as *inline* RawHTML. `confluence.Renderer.renderRawHTML` must therefore call `convertComment` (the same reconstruction the block path `renderHTMLBlock` uses) before falling back to `convertRawSpan`, or the macro leaks back out as a literal comment. *Test:* `TestRoundTrip_TocMacroInHeading`. These live in the formatter / verify layer rather than the converter, but bite the same way: - **Some XML carries values Markdown has no slot for** — `` (Confluence renumbers them every save), `
        ` bullet styling, and a stray trailing nbsp before `` cannot survive a Markdown round-trip and aren't meaningful content. `normalizeForVerify` canonicalizes them on **both** sides (`reTaskID` zeroes the id, `reListStyleAttr` drops `
          /
            ` attributes, `reTrailWSTaskBody` strips the trailing space — note the whitespace class must include `\x{00a0}`, the actual byte stored). *Tests:* `TestNormalizeForVerify_TaskID`, `TestNormalizeForVerify_ListStyleAttr`, `TestNormalizeForVerify_TaskBodyTrailingWS`, `TestNormalizeForVerify_EmphasisWhitespace`. +- **Structured-macro attribute order isn't canonical** — Confluence emits a TOC macro's attributes as `macro-id,name,schema-version` on one page and `name,schema-version,macro-id` on another, and the verify formatter preserves order, so no fixed reconstruction order matches everything. `normalizeForVerify.sortMacroAttrs` sorts the attributes of every `` opener on both sides. Likewise `` ↔ `` (both strikethrough — xml2md emits `~~…~~`, md2xml renders ``) is canonicalized via `reStrikeOpen`/`reStrikeClose`. *Tests:* `TestNormalizeForVerify_MacroAttrOrder`, `TestNormalizeForVerify_StrikeTag`, `TestNormalizeForVerify_EmptyEmphasis`. - **A "visible char" class in regexes must exclude `<`, not just whitespace** — `verify.normalizeForVerify`'s `reBareTextAfterHeading` wraps bare text between a heading and a following block in `

            …

            `. Its "≥1 visible char" token must be `[^\s<]`, never `\S`: `\S` matches `<`, so the capture swallows the next tag's opening bracket (e.g. ``) and the inserted `

            ` lands between a code macro's open tag and its body — destroying round-trip for a no-language fenced code block after a heading. *Tests:* `TestNormalizeForVerify_BareTextAfterHeading`, `TestVerifyRoundTrip_CodeBlockAfterHeading`. - **`fmt` line-wrapping must keep `…` whole** — breaking a line inside an inline code span injects the continuation indent into the span's text, which `xml2md` collapses into a spurious space *inside* the span (`` ` app.manifest.lock` ``). `format.splitAtoms` emits an entire `…` as one unbreakable atom. Significant whitespace between adjacent inline elements (` `) is carried on each atom's `spaceBefore` flag — never via `strings.Fields`, which drops trailing space. *Tests:* `TestPrettyXML_WrapKeepsCodeSpanWhole`, `TestPrettyXML_WrapPreservesSpaceBeforeCodeAfterWord`, `TestPrettyXML_SpaceBetweenInlineElements`. diff --git a/cmd/mdcx/verify.go b/cmd/mdcx/verify.go index 8c8f025f13084acf52341854ed643350fc85d08e..4fb3b14a38b4e21ff5f674c459703bde163051ee 100644 --- a/cmd/mdcx/verify.go +++ b/cmd/mdcx/verify.go @@ -5,6 +5,7 @@ import ( "io" "os" "regexp" + "sort" "strings" "github.com/spf13/cobra" @@ -149,8 +150,48 @@ var ( // Markdown emphasis cannot be flanked by whitespace) to one shape. reEmphLeadWS = regexp.MustCompile(`<(strong|em|del)>([ \t]+)`) reEmphTrailWS = regexp.MustCompile(`([ \t]+)`) + // reStrikeTag canonicalizes ``/`` to ``/``. Both are + // strikethrough; xml2md emits `~~…~~`, which md2xml renders as , so the + // stored `` and the round-tripped `` must compare equal. + reStrikeOpen = regexp.MustCompile(``) + reStrikeClose = regexp.MustCompile(``) + // reEmptyEmphasis drops an emphasis tag whose entire body is whitespace + // (including nbsp) — Confluence stores stray bold/italic applied to a lone + // space, which carries no content and round-trips as bare whitespace. + reEmptyStrong = regexp.MustCompile(`([ \t\x{00a0}]*)`) + reEmptyEm = regexp.MustCompile(`([ \t\x{00a0}]*)`) + reEmptyDel = regexp.MustCompile(`([ \t\x{00a0}]*)`) + // reStructMacroOpen matches a `` opening tag (self- + // closing or not). Confluence does not store its attributes in a fixed order + // — different pages emit a TOC macro as `macro-id,name,schema-version` or + // `name,schema-version,macro-id` — so the round-trip can't reproduce the + // original order. sortMacroAttrs canonicalizes it on both sides instead. + reStructMacroOpen = regexp.MustCompile(`]*?\s*/?>`) + reTagAttr = regexp.MustCompile(`[\w:-]+="[^"]*"`) ) +// sortMacroAttrs rewrites every opener with its +// attributes sorted, so attribute order never affects the verify comparison. +func sortMacroAttrs(xml string) string { + return reStructMacroOpen.ReplaceAllStringFunc(xml, func(tag string) string { + selfClose := strings.HasSuffix(tag, "/>") + attrs := reTagAttr.FindAllString(tag, -1) + sort.Strings(attrs) + var b strings.Builder + b.WriteString("") + } else { + b.WriteString(">") + } + return b.String() + }) +} + // normalizeForVerify strips XML patterns that cannot survive a round-trip // through Markdown, so verify compares only what the converter can preserve. // This is applied to BOTH the original input and the round-trip output so that @@ -190,6 +231,15 @@ func normalizeForVerify(xml string) string { xml = reListStyleAttr.ReplaceAllString(xml, "<$1>") // Strip trailing whitespace before a task-body close. xml = reTrailWSTaskBody.ReplaceAllString(xml, "$1") + // Canonicalize attribute order. + xml = sortMacroAttrs(xml) + // Canonicalize strikethrough to . + xml = reStrikeOpen.ReplaceAllString(xml, "") + xml = reStrikeClose.ReplaceAllString(xml, "") + // Drop emphasis tags whose body is only whitespace. + xml = reEmptyStrong.ReplaceAllString(xml, "$1") + xml = reEmptyEm.ReplaceAllString(xml, "$1") + xml = reEmptyDel.ReplaceAllString(xml, "$1") // Move whitespace from just inside an emphasis tag to just outside it. xml = reEmphLeadWS.ReplaceAllString(xml, "$2<$1>") xml = reEmphTrailWS.ReplaceAllString(xml, "$1") diff --git a/cmd/mdcx/verify_test.go b/cmd/mdcx/verify_test.go index 35fd5d983ae0ace268cd3d37f407f6b72fbe6c45..1f84974cefbe42f0bb29b698671b1c2fff203123 100644 --- a/cmd/mdcx/verify_test.go +++ b/cmd/mdcx/verify_test.go @@ -208,6 +208,25 @@ func TestNormalizeForVerify_EmphasisWhitespace(t *testing.T) { assert.Equal(t, `a x b`, normalizeForVerify(stored)) } +func TestNormalizeForVerify_MacroAttrOrder(t *testing.T) { + // Confluence stores TOC-macro attributes in different orders on different + // pages; sorting them on both sides makes order irrelevant. + a := `` + b := `` + assert.Equal(t, normalizeForVerify(a), normalizeForVerify(b)) +} + +func TestNormalizeForVerify_StrikeTag(t *testing.T) { + // and are both strikethrough; canonicalize to . + assert.Equal(t, `x`, normalizeForVerify(`x`)) +} + +func TestNormalizeForVerify_EmptyEmphasis(t *testing.T) { + // Whitespace-only emphasis (incl. nbsp) is dropped to its bare content. + assert.Equal(t, "a b", normalizeForVerify("a b")) + assert.Equal(t, "a\u00a0b", normalizeForVerify("a\u00a0b")) +} + // --- computeDiffOps --- func TestComputeDiffOps_IdenticalInputs(t *testing.T) { diff --git a/confluence/renderer.go b/confluence/renderer.go index 8d76646c87960b9b8b05f088f30c5fd36a7d9718..50c22755b1c0fa4ec97aed329a60e371d8306d6a 100644 --- a/confluence/renderer.go +++ b/confluence/renderer.go @@ -300,6 +300,19 @@ func (r *Renderer) renderHTMLBlock(w util.BufWriter, source []byte, node ast.Nod return ast.WalkSkipChildren, nil } +// tocMacro builds a TOC structured-macro, emitting attributes in Confluence's +// stored order (macro-id, name, schema-version) so a round-tripped macro matches +// the original byte-for-byte under the verify formatter, which preserves order. +func tocMacro(macroID string) string { + var b strings.Builder + b.WriteString("`) + return b.String() +} + // convertComment converts preserved HTML comments back to Confluence XML. func (r *Renderer) convertComment(raw string) (string, bool) { trimmed := strings.TrimSpace(raw) @@ -325,23 +338,11 @@ func (r *Renderer) convertComment(raw string) (string, bool) { // TOC macro (in-p variant: wrap in

            for original

            shapes) case strings.HasPrefix(trimmed, "`, where goldmark parses the comment as inline + // RawHTML. Reconstruct the macro here too; otherwise it leaks back out as + // a literal comment instead of ``. + if converted, ok := r.convertComment(raw); ok { + w.WriteString(converted) + continue + } w.WriteString(r.convertRawSpan(raw)) } return ast.WalkContinue, nil diff --git a/converter/md2xml_test.go b/converter/md2xml_test.go index 19bcd73e604edb5b84dcbf1daca02734ffcfa00f..40452076d5bbec411999a463bf3ba10b2cd5de78 100644 --- a/converter/md2xml_test.go +++ b/converter/md2xml_test.go @@ -690,6 +690,34 @@ func TestRoundTrip_EmphasisInteriorSpaces(t *testing.T) { assert.NotContains(t, xmlOut, "**", "asterisks must not leak as literal text") } +// TestRoundTrip_TocMacroInHeading guards against the TOC macro that lives inside +// a heading round-tripping lossily. xml2md emits it as `# `; +// goldmark parses that comment as *inline* RawHTML, so md2xml must reconstruct +// the structured-macro from the inline path (renderRawHTML), not only the block +// path — otherwise it leaks back out as a literal comment instead of a macro. +func TestRoundTrip_TocMacroInHeading(t *testing.T) { + xmlIn := `

            ` + + md, err := ConfluenceToMarkdown(xmlIn) + require.NoError(t, err) + + xmlOut, err := MarkdownToConfluence([]byte(md)) + require.NoError(t, err) + assert.Contains(t, xmlOut, `ac:name="toc"`, "TOC macro must be reconstructed, not left as a comment") + assert.Contains(t, xmlOut, `ac:macro-id="f9579fa5-2198-4556-92de-04db447accec"`) + assert.NotContains(t, xmlOut, "