~bigbes/confluence-md-utilities

3c0272ac2232ee3a0edd0c259b71e9fd3384f211 — Eugene Blikh a month ago f6c846a
fix: round-trip fidelity for heading comments, literal tags, and emphasis

Surfaced by `verify` on two real documents that aren't in the justfile set.

- Heading-only HTML comment (a TOC macro marker, `<h1><!-- ac:toc ... --></h1>`)
  was dropped by xml2md — a CommentNode is neither text nor element, so it fell
  through walkChildren and vanished, collapsing `# <!-- ... -->` to `#`.
- Literal angle-bracket text stored as `&lt;table&gt;` (e.g. `box.backup.start(<table>)`)
  round-tripped into a phantom `<table>` element that swallowed the rest of the
  document: xml2md now backslash-escapes a tag-like `<` in both the TextNode and
  walkChildrenInline paths, and renderText unescapes only `\<` (never a blanket
  UnescapePunctuations, which would eat the deliberate `\|` table-pipe escaping).
- `<strong> x </strong>` (whitespace inside the markers) rendered as literal
  `** x **`; writeEmphasis now moves the whitespace outside the markers.
- normalizeForVerify canonicalizes values Markdown can't carry — task ids,
  <ul>/<ol> bullet styling, and a trailing nbsp before </ac:task-body>.

Adds regression tests for each and documents the new gotchas in CLAUDE.md.
M CLAUDE.md => CLAUDE.md +5 -1
@@ 64,8 64,12 @@ These are subtle traps the round-trip has been bitten by; touching the related c
- **Page links have no markdown form** — `<ac:link><ri:page ri:space-key="…" ri:content-title="…"/></ac:link>` is preserved as a `<span data-page-link="1" …>` placeholder. Goldmark cannot parse `<ac:link>` as raw inline HTML because of the `:` in the tag name, so a span wrapper is required. The reverse path lives in `confluence.Renderer.convertRawSpan`; closing `</span>` consults `pageLinkDepth` to emit `</ac:link>` at the right point. *Test:* `TestRoundTrip_PageLink`.
- **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 `<ol>`, 2 for `<ul>` / 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 `<li>` in `<p>`. `ensureSingleNewline()` writes `\n` only when the buffer doesn't already end in one, preventing the double newline that appeared when a `<li>`'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 `&lt;table&gt;` (a function signature like `box.backup.start(<table>)`) decodes to `<table>` 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 `<li>` 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** — `<strong> x </strong>` (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`.

These two live in the formatter / verify layer rather than the converter, but bite the same way:
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** — `<ac:task-id>` (Confluence renumbers them every save), `<ul style="list-style-type: square;">` bullet styling, and a stray trailing nbsp before `</ac:task-body>` cannot survive a Markdown round-trip and aren't meaningful content. `normalizeForVerify` canonicalizes them on **both** sides (`reTaskID` zeroes the id, `reListStyleAttr` drops `<ul>/<ol>` 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`.

- **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 `<p>…</p>`. Its "≥1 visible char" token must be `[^\s<]`, never `\S`: `\S` matches `<`, so the capture swallows the next tag's opening bracket (e.g. `<ac:structured-macro>`) and the inserted `</p>` 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 `<code>…</code>` 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 `<code>…</code>` as one unbreakable atom. Significant whitespace between adjacent inline elements (`</strong> <code>`) is carried on each atom's `spaceBefore` flag — never via `strings.Fields`, which drops trailing space. *Tests:* `TestPrettyXML_WrapKeepsCodeSpanWhole`, `TestPrettyXML_WrapPreservesSpaceBeforeCodeAfterWord`, `TestPrettyXML_SpaceBetweenInlineElements`.

M cmd/mdcx/verify.go => cmd/mdcx/verify.go +28 -0
@@ 130,6 130,25 @@ var (
	// (e.g. <ac:structured-macro>) and wrap a structural element in <p>...</p> —
	// which splits a code macro open from its body and destroys round-trip.
	reBareTextAfterHeading = regexp.MustCompile(`(?s)(</h[1-6]>)([^<]*?[^\s<][^<]*?)(\s*<(?:table|ul|ol|h[1-6]|ac:|p|hr))`)
	// reTaskID canonicalizes the numeric task id. Confluence assigns these ids
	// itself and renumbers them on every save; Markdown has no slot to carry the
	// original value, so the round-trip re-numbers from 1. The exact number is
	// not meaningful content — zero it on both sides so verify ignores it.
	reTaskID = regexp.MustCompile(`<ac:task-id>\d+</ac:task-id>`)
	// reListStyleAttr strips presentational attributes (e.g.
	// `style="list-style-type: square;"`) from <ul>/<ol> openers. Markdown has no
	// way to carry list bullet styling, so the round-trip emits a bare <ul>/<ol>.
	reListStyleAttr = regexp.MustCompile(`<(ul|ol)\s+[^>]*>`)
	// reTrailWSTaskBody strips trailing horizontal whitespace before a task-body
	// close — the original often ends a task body with a stray space after the
	// user link that no Markdown construct reproduces.
	reTrailWSTaskBody = regexp.MustCompile(`[ \t\x{00a0}]+(</ac:task-body>)`)
	// reEmphLeadWS / reEmphTrailWS move whitespace that sits just inside an
	// emphasis tag to just outside it, canonicalizing `<strong> x </strong>`
	// (the stored form) and ` <strong>x</strong> ` (the round-trip form, since
	// Markdown emphasis cannot be flanked by whitespace) to one shape.
	reEmphLeadWS  = regexp.MustCompile(`<(strong|em|del)>([ \t]+)`)
	reEmphTrailWS = regexp.MustCompile(`([ \t]+)</(strong|em|del)>`)
)

// normalizeForVerify strips XML patterns that cannot survive a round-trip


@@ 165,6 184,15 @@ func normalizeForVerify(xml string) string {
	xml = rePImageOnly.ReplaceAllString(xml, "$1")
	// Wrap bare text directly after headings in <p>...</p>.
	xml = reBareTextAfterHeading.ReplaceAllString(xml, "$1<p>$2</p>$3")
	// Canonicalize Confluence-assigned task ids — Markdown can't carry them.
	xml = reTaskID.ReplaceAllString(xml, "<ac:task-id>0</ac:task-id>")
	// Drop presentational attributes from <ul>/<ol> openers.
	xml = reListStyleAttr.ReplaceAllString(xml, "<$1>")
	// Strip trailing whitespace before a task-body close.
	xml = reTrailWSTaskBody.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, "</$2>$1")
	return xml
}


M cmd/mdcx/verify_test.go => cmd/mdcx/verify_test.go +37 -0
@@ 171,6 171,43 @@ func TestVerifyRoundTrip_CodeBlockAfterHeading(t *testing.T) {
		"code-block body must survive the round-trip")
}

func TestNormalizeForVerify_TaskID(t *testing.T) {
	// Confluence-assigned task ids are renumbered by the round-trip; verify must
	// ignore the value by zeroing it on both sides.
	in := `<ac:task><ac:task-id>9</ac:task-id><ac:task-status>complete</ac:task-status></ac:task>`
	want := `<ac:task><ac:task-id>0</ac:task-id><ac:task-status>complete</ac:task-status></ac:task>`
	assert.Equal(t, want, normalizeForVerify(in))
}

func TestNormalizeForVerify_ListStyleAttr(t *testing.T) {
	// Markdown can't carry list bullet styling, so presentational attributes on
	// <ul>/<ol> are stripped on both sides.
	assert.Equal(t, `<ul><li>x</li></ul>`,
		normalizeForVerify(`<ul style="list-style-type: square;"><li>x</li></ul>`))
	assert.Equal(t, `<ol><li>x</li></ol>`,
		normalizeForVerify(`<ol style="list-style-type: square;"><li>x</li></ol>`))
	// A bare <ul> is untouched.
	assert.Equal(t, `<ul><li>x</li></ul>`, normalizeForVerify(`<ul><li>x</li></ul>`))
}

func TestNormalizeForVerify_TaskBodyTrailingWS(t *testing.T) {
	// The stored form ends a task body with a stray nbsp after the user link that
	// no Markdown construct reproduces; strip it (regular space and nbsp).
	nbsp := " "
	in := `<ac:task-body><ac:link><ri:user ri:userkey="k"/></ac:link>` + nbsp + `</ac:task-body>`
	want := `<ac:task-body><ac:link><ri:user ri:userkey="k"/></ac:link></ac:task-body>`
	assert.Equal(t, want, normalizeForVerify(in))
}

func TestNormalizeForVerify_EmphasisWhitespace(t *testing.T) {
	// The stored `<strong> x </strong>` and the round-trip ` <strong>x</strong> `
	// must canonicalize to the same shape (whitespace outside the tags).
	stored := `a<strong> x </strong>b`
	roundtrip := `a <strong>x</strong> b`
	assert.Equal(t, normalizeForVerify(roundtrip), normalizeForVerify(stored))
	assert.Equal(t, `a <strong>x</strong> b`, normalizeForVerify(stored))
}

// --- computeDiffOps ---

func TestComputeDiffOps_IdenticalInputs(t *testing.T) {

M confluence/renderer.go => confluence/renderer.go +9 -1
@@ 443,7 443,15 @@ func (r *Renderer) renderText(w util.BufWriter, source []byte, node ast.Node, en
		return ast.WalkContinue, nil
	}
	n := node.(*ast.Text)
	w.WriteString(html.EscapeString(string(n.Segment.Value(source))))
	// goldmark keeps a backslash-escaped run (e.g. `\<table>`) as a single Text
	// node with the backslash still in the segment — its default writer strips
	// escapes at render time, but this renderer emits the raw segment. We unescape
	// only `\<`: that is the one escape xml2md adds (to stop literal angle-bracket
	// text like `box.backup.start(<table>)` being re-parsed as an HTML tag), and
	// limiting it to `<` leaves the deliberate `\|` table-pipe escaping — which
	// must survive verbatim — untouched.
	value := strings.ReplaceAll(string(n.Segment.Value(source)), `\<`, `<`)
	w.WriteString(html.EscapeString(value))
	if n.HardLineBreak() {
		w.WriteString("<br/>")
	} else if n.SoftLineBreak() {

M converter/md2xml_test.go => converter/md2xml_test.go +68 -0
@@ 622,6 622,74 @@ func TestRoundTrip_PageLink(t *testing.T) {
	assert.Contains(t, xmlOutput, "Основной документ:")
}

// TestConfluenceToMarkdown_CommentInHeading guards against the regression
// where an HTML comment that is the sole content of a heading — e.g. a TOC
// macro marker md2xml emits as `<h1><!-- ac:toc macro-id="..." --></h1>` —
// was silently dropped by xml2md. A CommentNode is neither a TextNode nor an
// ElementNode, so it used to fall through to walkChildren (a comment has no
// children) and vanish, collapsing `# <!-- ac:toc ... -->` to a bare `#`.
func TestConfluenceToMarkdown_CommentInHeading(t *testing.T) {
	xmlInput := `<h1><!-- ac:toc macro-id="f9579fa5-2198-4556-92de-04db447accec" --></h1>`

	md, err := ConfluenceToMarkdown(xmlInput)
	require.NoError(t, err)
	assert.Contains(t, md, `# <!-- ac:toc macro-id="f9579fa5-2198-4556-92de-04db447accec" -->`)

	// And the comment survives a full markdown round-trip.
	xmlOutput, err := MarkdownToConfluence([]byte(md))
	require.NoError(t, err)
	mdBack, err := ConfluenceToMarkdown(xmlOutput)
	require.NoError(t, err)
	assert.Equal(t, strings.TrimSpace(md), strings.TrimSpace(mdBack))
}

// TestRoundTrip_LiteralAngleBracketText guards against the catastrophic
// regression where literal angle-bracket text stored as `&lt;table&gt;` (e.g.
// the function signature `box.backup.start(<table>)`) round-tripped into a real
// `<table>` element: xml2md emitted a bare `<table>`, goldmark parsed it as raw
// HTML, and the phantom table swallowed the rest of the document. xml2md now
// backslash-escapes a tag-like `<`, and renderText unescapes it back to `&lt;`.
func TestRoundTrip_LiteralAngleBracketText(t *testing.T) {
	for _, where := range []string{
		`<p>call box.backup.start(&lt;table&gt;) now</p>`,
		`<ul><li>call box.backup.start(&lt;table&gt;) now</li></ul>`,
	} {
		md, err := ConfluenceToMarkdown(where)
		require.NoError(t, err)
		// The '<' must be escaped so goldmark won't parse it as a table tag.
		assert.Contains(t, md, `\<table>`, "input %q", where)

		xmlOut, err := MarkdownToConfluence([]byte(md))
		require.NoError(t, err)
		assert.Contains(t, xmlOut, "box.backup.start(&lt;table&gt;)", "input %q", where)
		assert.NotContains(t, xmlOut, "<table>", "literal text must not become a table element")
	}

	// A non-tag '<' (comparison operator) is left unescaped and still survives.
	md, err := ConfluenceToMarkdown(`<p>t &lt;= time_end</p>`)
	require.NoError(t, err)
	assert.NotContains(t, md, `\<`, "a non-tag '<' should not be escaped")
	xmlOut, err := MarkdownToConfluence([]byte(md))
	require.NoError(t, err)
	assert.Contains(t, xmlOut, "t &lt;= time_end")
}

// TestRoundTrip_EmphasisInteriorSpaces guards against `<strong> x </strong>`
// (whitespace just inside the tag) round-tripping to literal `** x **` asterisks
// — CommonMark does not treat a whitespace-flanked run as emphasis. xml2md now
// moves the spaces outside the markers (` **x** `).
func TestRoundTrip_EmphasisInteriorSpaces(t *testing.T) {
	md, err := ConfluenceToMarkdown(`<p>Вызов<strong> box.backup.start() </strong>будет</p>`)
	require.NoError(t, err)
	assert.Contains(t, md, "**box.backup.start()**", "spaces must move outside the markers")
	assert.NotContains(t, md, "** box.backup.start", "no whitespace just inside the markers")

	xmlOut, err := MarkdownToConfluence([]byte(md))
	require.NoError(t, err)
	assert.Contains(t, xmlOut, "<strong>box.backup.start()</strong>")
	assert.NotContains(t, xmlOut, "**", "asterisks must not leak as literal text")
}

// TestRoundTrip_NestedOrderedList guards against the regression where a
// nested <ol> inside an outer <ol><li> was flattened into a single flat list
// because the markdown was emitted with a 2-space indent (insufficient for

M converter/xml2md.go => converter/xml2md.go +80 -10
@@ 115,6 115,35 @@ func (c *xmlConverter) ensureSingleNewline() {
	}
}

// writeEmphasis renders an inline emphasis element (strong/em/del) wrapping its
// children in `marker`, with any leading/trailing whitespace moved *outside* the
// markers. CommonMark does not recognize `** x **` as emphasis (the run must be
// flanked by non-whitespace), so `<strong> x </strong>` has to become ` **x** `
// — otherwise the asterisks round-trip as literal text. An all-whitespace body
// is emitted verbatim with no markers.
func (c *xmlConverter) writeEmphasis(marker string, n *html.Node, depth int) {
	saved := c.buf
	c.buf = &bytes.Buffer{}
	c.walkChildren(n, depth)
	inner := c.buf.String()
	c.buf = saved

	body := strings.TrimLeft(inner, " \t\r\n")
	lead := inner[:len(inner)-len(body)]
	core := strings.TrimRight(body, " \t\r\n")
	trail := body[len(core):]

	if core == "" {
		c.buf.WriteString(inner)
		return
	}
	c.buf.WriteString(lead)
	c.buf.WriteString(marker)
	c.buf.WriteString(core)
	c.buf.WriteString(marker)
	c.buf.WriteString(trail)
}

func (c *xmlConverter) walkChildren(n *html.Node, depth int) {
	for child := n.FirstChild; child != nil; child = child.NextSibling {
		c.walk(child, depth)


@@ 134,10 163,27 @@ func (c *xmlConverter) walk(n *html.Node, depth int) {
			// but preserve the trimmed content
			text = collapseWhitespace(text)
		}
		// Backslash-escape '<' so literal angle-bracket text (e.g. a function
		// signature like `box.backup.start(<table>)`, stored as `&lt;table&gt;`)
		// is not re-parsed as an HTML tag by goldmark on the way back to XML —
		// an unescaped `<table>` swallows the rest of the document into a phantom
		// table. md2xml's escaper turns the resulting `<` text back into `&lt;`,
		// closing the round-trip.
		text = escapeMarkdownAngle(text)
		c.buf.WriteString(text)
		return
	}

	// Preserve literal HTML comments (e.g. a TOC macro marker that md2xml
	// emitted inside a heading: `<h1><!-- ac:toc macro-id="..." --></h1>`).
	// Without this, the comment node — being neither text nor element — falls
	// through to walkChildren below and is silently dropped, collapsing
	// `# <!-- ac:toc ... -->` to a bare `#` on round-trip.
	if n.Type == html.CommentNode {
		c.buf.WriteString("<!--" + n.Data + "-->")
		return
	}

	if n.Type != html.ElementNode {
		c.walkChildren(n, depth)
		return


@@ 181,17 227,11 @@ func (c *xmlConverter) walk(n *html.Node, depth int) {

	// Inline formatting
	case tag == "strong", tag == "b":
		c.buf.WriteString("**")
		c.walkChildren(n, depth)
		c.buf.WriteString("**")
		c.writeEmphasis("**", n, depth)
	case tag == "em", tag == "i":
		c.buf.WriteString("*")
		c.walkChildren(n, depth)
		c.buf.WriteString("*")
		c.writeEmphasis("*", n, depth)
	case tag == "del", tag == "s":
		c.buf.WriteString("~~")
		c.walkChildren(n, depth)
		c.buf.WriteString("~~")
		c.writeEmphasis("~~", n, depth)
	case tag == "code":
		if !isPrevSiblingCode(n) {
			c.buf.WriteString("`")


@@ 781,7 821,9 @@ func (c *xmlConverter) walkChildrenInline(n *html.Node, depth int) {
				text = strings.TrimRight(text, " ")
			}
			if text != "" {
				c.buf.WriteString(text)
				// Escape tag-like '<' so literal angle-bracket text in list items
				// (e.g. `box.backup.start(<table>)`) is not re-parsed as raw HTML.
				c.buf.WriteString(escapeMarkdownAngle(text))
			}
			continue
		}


@@ 1207,6 1249,34 @@ func collapseWhitespace(s string) string {
	return buf.String()
}

// escapeMarkdownAngle backslash-escapes a '<' that would start an HTML tag
// (i.e. is followed by a letter, '/', '!' or '?') so goldmark keeps it as
// literal text instead of raw HTML — otherwise a `<table>` in body text
// swallows the rest of the document into a phantom table. The matching
// renderText unescape turns `\<` back into `&lt;`, closing the round-trip.
// A lone '<' (as in `a < b` or `<=`) is left alone: goldmark already treats
// it as literal text, so escaping it would only add noise.
func escapeMarkdownAngle(s string) string {
	if !strings.Contains(s, "<") {
		return s
	}
	var buf strings.Builder
	for i := 0; i < len(s); i++ {
		if s[i] == '<' && (i == 0 || s[i-1] != '\\') && i+1 < len(s) && isTagNameStart(s[i+1]) {
			buf.WriteString(`\<`)
			continue
		}
		buf.WriteByte(s[i])
	}
	return buf.String()
}

// isTagNameStart reports whether b can follow '<' to begin an HTML tag,
// comment, or processing instruction that goldmark would parse as raw HTML.
func isTagNameStart(b byte) bool {
	return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '/' || b == '!' || b == '?'
}

// hasTaskStatus checks if a node contains a task-status element.
func hasTaskStatus(n *html.Node) bool {
	for child := n.FirstChild; child != nil; child = child.NextSibling {