From f6c846a1f16d1f6d0045c92a1c0f24429e6b901f Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Mon, 1 Jun 2026 12:23:11 +0300 Subject: [PATCH] fix: preserve inline spacing in fmt and code-block round-trip Three related round-trip/formatting bugs, each pinned by a regression test that fails on the prior code: - fmt dropped the significant space between two inline elements ( ): a lone-space text node was always treated as indentation. Keep it mid-line; drop it only at a block boundary. - fmt's line-wrapper dropped a word's trailing space before a tag (strings.Fields discards it) and broke inside ..., injecting indent that xml2md collapses into a spurious in-span space. Rewrote wrapLine around an atom model with a per-atom spaceBefore flag and keeps an inline code span whole. - verify.normalizeForVerify's reBareTextAfterHeading used \S for its "visible char" token; \S matches '<', so the capture swallowed a following and wrapped it in

...

, splitting a code macro from its body and breaking round-trip for a no-language fenced code block after a heading. Use [^\s<] instead. Full RFC-0010 document now verifies as lossless. --- CLAUDE.md | 7 ++ cmd/mdcx/verify.go | 7 +- cmd/mdcx/verify_test.go | 54 +++++++++++++++ format/pretty.go | 143 +++++++++++++++++++++------------------- format/pretty_test.go | 61 +++++++++++++++++ 5 files changed, 204 insertions(+), 68 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d42ea2f268df4d90c968f3debb0dce1707e386b5..23ba6e5356666a1c9335208a3482d4ef99bb4738 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,6 +12,8 @@ just test-v # verbose tests go test ./converter/... -run TestRoundTrip # run specific tests ``` +**Every bug fix gets a regression test.** Whenever you fix a bug, add a test that reproduces the exact case and fails on the original code (verify it fails before the fix, passes after). This codebase is a web of subtle round-trip traps — see "Round-trip gotchas" below; each was re-broken at least once before a test pinned it. + ## Architecture **mdcx** converts Markdown ↔ Confluence storage-format XML with round-trip fidelity. The binary lives at `cmd/mdcx/`. @@ -63,6 +65,11 @@ 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. +These two live in the formatter / verify layer rather than the converter, but bite the same way: + +- **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`. + ### Verifying real documents `mdcx verify ` formats the input, round-trips it through `xml2md → md2xml`, formats the output, and diffs them. The justfile in `~/data/1` runs this against five real Confluence pages — exercise it after any change to converter or renderer code, since the unit tests can miss multi-feature interactions (e.g. a task list inside a `
      ` inside a table cell). diff --git a/cmd/mdcx/verify.go b/cmd/mdcx/verify.go index 075f3d1baf06c3ec48e30fc56329545842b6140a..5965513e5d172a1faa2b7eab2d31de8b1af01401 100644 --- a/cmd/mdcx/verify.go +++ b/cmd/mdcx/verify.go @@ -124,7 +124,12 @@ var ( // reBareTextAfterHeading wraps bare text appearing directly between a // heading close and a following block tag in

      ...

      . Confluence accepts // both forms but markdown round-trip always paragraphs it. - reBareTextAfterHeading = regexp.MustCompile(`(?s)()([^<]*?\S[^<]*?)(\s*<(?:table|ul|ol|h[1-6]|ac:|p|hr))`) + // + // The "at least one visible char" token is [^\s<], not \S: \S also matches + // '<', so it would let the group swallow a following tag's opening bracket + // (e.g. ) and wrap a structural element in

      ...

      — + // which splits a code macro open from its body and destroys round-trip. + reBareTextAfterHeading = regexp.MustCompile(`(?s)()([^<]*?[^\s<][^<]*?)(\s*<(?:table|ul|ol|h[1-6]|ac:|p|hr))`) ) // normalizeForVerify strips XML patterns that cannot survive a round-trip diff --git a/cmd/mdcx/verify_test.go b/cmd/mdcx/verify_test.go index 6b5fca38ee510aef698420f8cb381085dce8173f..0aa9ecc18368af621af082ffdb46bab8c9aac0c4 100644 --- a/cmd/mdcx/verify_test.go +++ b/cmd/mdcx/verify_test.go @@ -6,6 +6,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "go.bigb.es/confluence-md-utilities/converter" + "go.bigb.es/confluence-md-utilities/format" ) // --- normalizeForVerify --- @@ -117,6 +120,57 @@ func TestNormalizeForVerify_AdjacentCodeMerged(t *testing.T) { } } +func TestNormalizeForVerify_BareTextAfterHeading(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "genuine bare text is wrapped", + input: `

      Title

      bare text
      x
      `, + want: `

      Title

      bare text

      x
      `, + }, + { + // Regression: \S used to match the '<' of the following tag, so the + // capture swallowed and wrapped it in

      ...

      , + // splitting a code macro from its body and breaking round-trip. + name: "structured-macro after heading is left intact", + input: "

      Heading

      \n", + want: "

      Heading

      \n", + }, + { + name: "paragraph after heading is left intact", + input: "

      T

      \n

      already a paragraph

      ", + want: "

      T

      \n

      already a paragraph

      ", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, normalizeForVerify(tt.input)) + }) + } +} + +// TestVerifyRoundTrip_CodeBlockAfterHeading is the end-to-end regression for the +// no-language fenced code block immediately after a heading: it must survive +// xml2md → md2xml with its body intact. +func TestVerifyRoundTrip_CodeBlockAfterHeading(t *testing.T) { + xml := "

      Heading

      \n" + + `` + + `` + + `` + "\n

      After.

      " + norm := normalizeForVerify(xml) + md, err := converter.ConfluenceToMarkdown(norm) + require.NoError(t, err) + rt, err := converter.MarkdownToConfluence([]byte(md)) + require.NoError(t, err) + assert.Equal(t, + format.PrettyXML(norm, " "), + format.PrettyXML(normalizeForVerify(rt), " "), + "code-block body must survive the round-trip") +} + // --- computeDiffOps --- func TestComputeDiffOps_IdenticalInputs(t *testing.T) { diff --git a/format/pretty.go b/format/pretty.go index e0e7a031b5ab79549a22298bb697a4df09331a49..21decd941073495dc35121212c623e46d822d1f5 100644 --- a/format/pretty.go +++ b/format/pretty.go @@ -153,10 +153,13 @@ func PrettyXML(input string, indent string) string { continue } text := collapseWS(tok.raw) - if text == "" || text == " " { + if text == "" { i++ continue } + // A lone space is an indentation artifact only at a block boundary + // (line start); mid-line it sits between two inline elements + // (e.g. ) and is a significant space — keep it. if atLineStart { text = strings.TrimLeft(text, " ") if text == "" { @@ -273,63 +276,38 @@ func wrapLine(line string, maxWidth int) []string { indentStr := line[:len(line)-len(trimmed)] contIndent := indentStr + " " // continuation lines get extra indent - // Split into segments: tags (unsplittable) and text (splittable at spaces) - segments := splitSegments(trimmed) + // Split into atoms: tags and words, never broken internally. Whitespace + // between atoms is collapsed to a single significant space recorded in + // spaceBefore — including space between a word and an adjacent tag + // (e.g. " "), which must survive wrapping. + atoms := splitAtoms(trimmed) var lines []string var cur strings.Builder cur.WriteString(indentStr) curWidth := runeWidth(indentStr) - for _, seg := range segments { - segW := runeWidth(seg) - - if seg == "" { - continue + for _, a := range atoms { + atomW := runeWidth(a.text) + spaceW := 0 + if a.spaceBefore { + spaceW = 1 } - // Tags and non-space text: never break inside - if strings.HasPrefix(seg, "<") { - // If adding this tag exceeds limit and we have content, wrap - if curWidth+segW > maxWidth && curWidth > runeWidth(indentStr) { - lines = append(lines, strings.TrimRight(cur.String(), " ")) - cur.Reset() - cur.WriteString(contIndent) - curWidth = runeWidth(contIndent) - } - cur.WriteString(seg) - curWidth += segW - continue + // Wrap before this atom if it would overflow and the current line + // already holds content beyond its indentation. + if curWidth+spaceW+atomW > maxWidth && curWidth > runeWidth(contIndent) { + lines = append(lines, strings.TrimRight(cur.String(), " ")) + cur.Reset() + cur.WriteString(contIndent) + curWidth = runeWidth(contIndent) + } else if a.spaceBefore && curWidth > runeWidth(indentStr) { + cur.WriteByte(' ') + curWidth++ } - // Text segment: split at word boundaries - words := strings.Fields(seg) - // Preserve leading space if original had one - needSpace := len(seg) > 0 && seg[0] == ' ' - - for _, word := range words { - wordW := runeWidth(word) - spaceW := 0 - if needSpace { - spaceW = 1 - } - - if curWidth+spaceW+wordW > maxWidth && curWidth > runeWidth(contIndent) { - lines = append(lines, strings.TrimRight(cur.String(), " ")) - cur.Reset() - cur.WriteString(contIndent) - curWidth = runeWidth(contIndent) - needSpace = false - } - - if needSpace { - cur.WriteByte(' ') - curWidth++ - } - cur.WriteString(word) - curWidth += wordW - needSpace = true - } + cur.WriteString(a.text) + curWidth += atomW } if cur.Len() > 0 { @@ -345,28 +323,59 @@ func wrapLine(line string, maxWidth int) []string { return lines } -// splitSegments breaks text into alternating tag and text segments. -// E.g. "Hello world end" -> ["Hello ", "", "world", "", " end"] -func splitSegments(s string) []string { - var segs []string +// atom is an unbreakable unit of a line — a tag (<...>) or a word — together +// with whether whitespace separated it from the previous atom. +type atom struct { + text string + spaceBefore bool +} + +// splitAtoms breaks a line into tags and words, recording the inter-atom +// whitespace as a single space flag. E.g. "Hello world end" +// -> {Hello}, { }, {world}, {}, { end}. +func splitAtoms(s string) []atom { + var atoms []atom + pendingSpace := false for len(s) > 0 { - lt := strings.Index(s, "<") - if lt == -1 { - segs = append(segs, s) - break - } - if lt > 0 { - segs = append(segs, s[:lt]) - } - gt := strings.Index(s[lt:], ">") - if gt == -1 { - segs = append(segs, s[lt:]) - break + r := s[0] + switch { + case r == ' ' || r == '\t' || r == '\n' || r == '\r': + pendingSpace = true + s = s[1:] + case r == '<': + end := strings.Index(s, ">") + if end == -1 { + atoms = append(atoms, atom{s, pendingSpace}) + return atoms + } + // Keep an inline code span whole. A line break inside + // ... would inject the continuation indent into the + // code's text content, which xml2md then collapses into a spurious + // space inside the span (e.g. "` app.manifest.lock`"). + if strings.HasPrefix(s, "") || strings.HasPrefix(s, ""); c != -1 { + spanEnd := c + len("") + atoms = append(atoms, atom{s[:spanEnd], pendingSpace}) + pendingSpace = false + s = s[spanEnd:] + continue + } + } + atoms = append(atoms, atom{s[:end+1], pendingSpace}) + pendingSpace = false + s = s[end+1:] + default: + // A word runs until the next space or tag start. + i := strings.IndexAny(s, " \t\n\r<") + if i == -1 { + i = len(s) + } + atoms = append(atoms, atom{s[:i], pendingSpace}) + pendingSpace = false + s = s[i:] } - segs = append(segs, s[lt:lt+gt+1]) - s = s[lt+gt+1:] } - return segs + return atoms } func runeWidth(s string) int { diff --git a/format/pretty_test.go b/format/pretty_test.go index cbfa77d81bf9570ddbc24e8c22246fab68b09d4e..bae201baecd46178694f4e1bacf028044df72fa6 100644 --- a/format/pretty_test.go +++ b/format/pretty_test.go @@ -161,3 +161,64 @@ func TestPrettyXML_LongLinePreservesTagIntegrity(t *testing.T) { assert.NotContains(t, result, ""). Regression for the +// "missing space before inline code" report. +func TestPrettyXML_SpaceBetweenInlineElements(t *testing.T) { + cases := []struct{ input, want string }{ + {`

      Bold word code here.

      `, " "}, + {`

      Link t code here.

      `, " "}, + {`

      Word em code.

      `, " "}, + } + for _, tc := range cases { + result := PrettyXML(tc.input, " ") + assert.Contains(t, result, tc.want, tc.input) + } +} + +// The same significant space must also survive when the line is long enough to +// be wrapped: wrapLine used to drop whitespace that sat between a word/tag and +// an adjacent tag, because strings.Fields discarded trailing space and tags +// ignored any pending space. +func TestPrettyXML_WrapPreservesSpaceBeforeTag(t *testing.T) { + long := strings.Repeat("слово ", 25) // push past the 120-rune wrap width + input := `

      ` + long + `жирный код хвост предложения.

      ` + result := PrettyXML(input, " ") + assert.NotContains(t, result, "") + assert.Contains(t, result, " ") +} + +// The literal reported pattern: a plain word followed by inline code on a line +// long enough to wrap. wrapLine's strings.Fields dropped the word's trailing +// space, gluing it to ("манифеста" instead of "манифеста "). +func TestPrettyXML_WrapPreservesSpaceBeforeCodeAfterWord(t *testing.T) { + long := strings.Repeat("текст ", 25) // push past the 120-rune wrap width + input := `

      ` + long + `манифеста app.manifest.toml и далее.

      ` + result := PrettyXML(input, " ") + assert.NotContains(t, result, "манифеста") + assert.Contains(t, result, "манифеста ") +} + +// A wrap must never fall inside ...: doing so injects the +// continuation indent into the code text, which round-trips as a spurious space +// inside the span. The whole span is kept on one line regardless of where the +// boundary lands. Probes several lengths so the wrap point sweeps across the +// code span's open tag. +func TestPrettyXML_WrapKeepsCodeSpanWhole(t *testing.T) { + for n := 18; n <= 24; n++ { + input := `

      ` + strings.Repeat("word ", n) + `tail app.manifest.lock rest of it.

      ` + result := PrettyXML(input, " ") + for _, line := range strings.Split(result, "\n") { + trimmed := strings.TrimSpace(line) + // No line may start or end mid-span (open without close, or vice versa). + assert.False(t, strings.HasPrefix(trimmed, "app.manifest.lock"), + "code content split onto its own line at n=%d: %q", n, line) + if strings.Contains(line, "") { + assert.Contains(t, line, "", + " opened but not closed on same line at n=%d: %q", n, line) + } + } + } +}