diff --git a/CLAUDE.md b/CLAUDE.md index e027b3e..01ddc7e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,7 @@ Module `github.com/mozilla/markfluence` (`go 1.25`). `main.go` is a shim to `cmd - `internal/pagetree` — `Walk`, the traversal of pages *and folders* under a node, plus `AllDepths`. It is a package rather than command-local because listing a subtree and exporting one (#59) need the identical walk, and its rules must not exist in two copies: siblings arrive from two requests (`/child/page`, `/child/folder`) and are **merged by `extensions.position`**, or the output loses the order Confluence displays; a folder **counts as a level** like a page, which is only reasonable because folders are reported rather than silently traversed; and the walk descends folders even when only pages matter, since a folder may hold the only pages in a subtree. `nodeURL` uses `SiteURL()` — a v1 child row carries `webui` but no `base`. A visited set guards the unbounded case. - `internal/pageref` — `Resolve`, the single page-argument resolver: a numeric id, a Confluence page **or folder** URL (`pagePathRE` matches both `/pages/` and `/folder/`, since `children` takes a folder and a folder URL is what a browser hands you — the id is all it returns, so a command that can only use a page reports its own not-found), or a `.md` file whose frontmatter has a `page_id` (stat'd first, so `123.md` is a file). Every command taking a page uses it. `message.go` also owns the wording for the two ways a *frontmatter* `page_id` is wrong — `NotFoundMessage` (caller supplies the remedy, which differs per command) and `NotNumericMessage` — because `create`, `update`, and `fix` all report them and a reader should recognize the same problem across all three. They return strings, not errors: `create` wraps the text in its typed `pageIDFailure` (which also carries the `--json` fields), the others want a plain error. Anything checking a `page_id` before a request uses `IsDigits`, since the API answers a non-numeric id with a 400 whose body says nothing useful. - `internal/client` — `ConfluenceClient` over `net/http` with basic auth. Built from a `Config` (site URL, cloud ID, username, token) via `New`; it carries **two bases**: `BaseURL()` is where requests go (the gateway when a cloud ID is set) and `SiteURL()` is always the site. Anything a reader sees uses `SiteURL()` — printed page URLs and, critically, the `baseURL` handed to `convert.MdToConfluence`, since rewritten links are published *into* the page. Pages are Confluence **v2**; attachment writes and the user lookup are **v1** (`/wiki/rest/api/...`). A **folder** — the Cloud content type that can parent a page — has its own v2 route, `GetFolderOrNil` against `/wiki/api/v2/folders/{id}`, because every v2 *page* route answers a folder id with 404; enumerating children, if it is ever added, must be v1, since v2 cannot list inside a folder at all and its page-children route silently omits folders ([docs/confluence/folders.md](docs/confluence/folders.md)). Typed `HTTPError`, per-attempt context timeouts, centralized retry/backoff in `send`. `HTTPError.Error()` appends a **hint** for the three auth failures whose status misleads, matched on the response *body* rather than deduced from the status and always **appended** to it, never replacing it. The one that matters: **a rejected credential is a 404 on every v2 route**, so a revoked token used to make `read` answer `page ... not found` about a page that exists. `RejectedCredential` tells it apart by the fact that every genuine v2 404 *names* what it could not find and the auth one does not, `notFound` gates the three `…OrNil` helpers on it so they stop reading it as "absent", and `jsonout.CodeFor` checks it before the status switch so `--json` reports `AUTH` rather than `NOT_FOUND`. A 403 that is not one of the two measured credential phrasings gets no hint, because that is what a genuine permission denial looks like ([docs/confluence/api.md](docs/confluence/api.md#scopes)). **Retry rules**: 429 for any method; 502/503/504 for idempotent methods; **any other 5xx only when the response carries `Retry-After`** — that is how a 500 becomes retryable, and it is why `parseRetryAfter` reports the header's *presence* apart from its delay (`Retry-After: 0` means "retry now", not "no header"). The exponential delay is jittered, a server-supplied `Retry-After` never is. Decisions go to a package-level hook (`SetRetryLogger`, set once in `root.go` beside `ui.SetDebug`) and fire whichever way they went, because `internal/client` prints nothing and a silent twelve-minute retry storm is otherwise indistinguishable from a hang. **A versioned PUT is not as idempotent as its method**: `SetContentProperty` retry-once on top (recovers a lost create-POST response) and `UpdatePage`'s `updateLanded` both exist for the same reason — a write whose response was lost gets re-sent, and the re-sent version is refused. `updateLanded` requires version *and* title *and* body to match what was sent, since a concurrent edit could have produced the version alone and claiming success over someone else's content is worse than a false failure ([docs/confluence/api.md](docs/confluence/api.md)). `SyncAttachments` (skip/update by a SHA-256 recorded in the attachment's comment, alongside the source path so `read` recovers image paths exactly; only the current comment form is parsed — an attachment stamped by a markfluence predating a comment-format change reads as unmanaged and is re-uploaded once, the same as any hand-uploaded file — except that a *recorded path disagreeing with the local source* is an update even when the checksum matches, so a mangled path repairs itself instead of surviving every later publish; a comment with no source recorded at all is not a disagreement. Every text part of the upload form must go through `writeTextField`, never `multipart.Writer.WriteField`, which emits no charset and gets decoded as Latin-1), `_links.next` pagination. **Three pagination schemes, and picking the wrong one truncates silently.** v1 *child/attachment* collections page through the generic `listV1` helper by `start`/`limit` offset, never `_links.next` (absent when the results fit one page, so it cannot terminate a loop); `ListAttachments`, `ListChildPages`, and `ListChildFolders` all go through it. v2 collections page through `listV2` by the cursor in `_links.next`, which is a `/wiki`-prefixed absolute path `resolveNext` handles unchanged; `ListContentProperties` and `SearchPagesByTitle` share it. **`/wiki/rest/api/search` is neither**: it ignores `start` outright, its `next` is context-relative so it needs the `/wiki` prefix `resolveNext` does not add, a short page does *not* mean the end, and `totalSize` can be nonzero against an empty `results` — so `searchCQL` terminates only on a missing `next` and nothing may branch on `totalSize` ([docs/confluence/search.md](docs/confluence/search.md)). `searchCQLBounded` adds a row bound under it (`SearchCQL` is that call with no bound, which is why `find` is unaffected): it asks for `max+1` and reports the surplus as `more`, since `totalSize` cannot supply a count. Full text goes through `SearchText`/`SearchRawCQL`, which return the cleaned `SearchMatch` the way `FindByTitle` returns `TitleMatch` — and **every field of a match comes from the row's `content` object**, because the row-level `title` is HTML-escaped *and* wrapped in `@@@hl@@@` markers where `content.title` is neither. The `excerpt` exists only at row level, so `cleanExcerpt` strips those markers, unescapes once, and collapses to one line — in the client, so the human and `--json` paths cannot disagree about it. `excerpt=highlight` is passed explicitly and **re-attached when following the cursor** (the `next` link carries `cql` and `limit` but not `excerpt`, and `doJSON` appends params with a bare `?`); an unrecognized value there yields an empty excerpt with a 200, so a rename by Atlassian degrades to no excerpts rather than an error. A row with no `content` object is skipped and **counted** — `type = space` answers with hundreds of them, and a silent skip would report a successful empty result. A bare v1 child row already carries `webui`, `status`, and `extensions.position`, so child listing needs no `expand`. `DownloadAttachment` goes through `send` (inheriting retry/backoff) against `_links.download`; **never** add a `CheckRedirect` that forwards headers — it would leak site credentials to Atlassian's media host, which neither needs nor wants them. `config.go` holds `Resolve` and the `.env` reader. Why each of these is shaped this way, with the evidence: [docs/confluence/api.md](docs/confluence/api.md) and [attachments.md](docs/confluence/attachments.md). -- `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, baseURL, spaceKey, version string) (*ConfluencePage, error)`. It parses with goldmark (GFM) and renders through a custom `storageRenderer` registered at priority 100 (below the default HTML=1000 and table=500 renderers) that emits Confluence storage format. `shield.go` renames raw `ac:`/`ri:` tags to colon-free sentinels around the goldmark step so pasted storage passes through; `callouts.go` is an AST transformer + blockquote renderer for GitHub alerts; `aclink.go` is the *inverse* direction's one element with enough shape to need its own file — ``, which the editor writes for every internal link and `MdToConfluence` never emits, so nothing in the regression suite covers it. One rule decides its whole mapping: **convert when the markdown republishes to a link resolving to the same target, pass the storage through when it would not** — so a page link and a space link convert, while a mention (80% of all real usage), an attachment link (only images are uploaded, so a relative href would be dead) and an unresolvable target stay raw, which the shield republishes byte-identical. A page target is a **title, never an id**, so `PageLinkTargets` reports what needs resolving and `StorageOptions.PageLinks` carries the answers back. An `ac:anchor` is **percent-encoded** where `confluenceSlug` output is not: decode it before matching a heading, leave it encoded inside a URL. A same-page anchor recovers its heading from the document rather than inverting the slug, which is impossible — `confluenceSlug` turns both a space and a hyphen into `-`. The survey the mapping rests on, and the `xml.HTMLAutoClose` trap that made `` crash the parser outright (#88), are in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `attachname.go` owns the source-path↔attachment-name mapping (percent-encoding `%`→`%25` then `/`→`%2F`, which is **bijective** — that is what makes the dedupe collision-free and lets `read` recover an image's original path; decode refuses an absolute result, which markfluence never produces; what names Confluence accepts is in [docs/confluence/attachments.md](docs/confluence/attachments.md)); `destination.go` owns the **other** codec, destination↔path (`decodeDestination`/`encodeDestination`), shared by images *and* doc links — a markdown destination is a URL, so decode inbound (**before** `withinRoot`, or an encoded `..%2F` slips the clamp) and encode outbound in `storage_to_md.go` (or `export` emits markdown that no longer parses, and `sourceFor`'s absolute-path refusal is undone by the next read); an undecodable destination is a literal `%` in a filename, not an error; the reasoning is in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `images.go` (resolution stays page-relative like GitHub; the documentation root — cwd — bounds what may be published, and an image above it is `IMAGE BROKEN`), `links.go` (sibling-file scans, GitHub/Confluence slugs, doc-link + anchor rewriting; `docKey` is the single lookup key for both the page and anchor maps. Failure here is **silent** — an unresolved link publishes as a dead relative href with no warning), `tables.go` (the `` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. Layout values, cell alignment, and colors: [docs/confluence/storage-format.md](docs/confluence/storage-format.md)), and `renderer.go` (code macros, text soft-break→space, images, links) do the rest. The `` and `` token substitutions happen **inside** `MdToConfluence`. +- `internal/convert` — the converter (the crux). `MdToConfluence(md *frontmatter.MarkdownFile, baseURL, spaceKey, version string) (*ConfluencePage, error)`. It parses with goldmark (GFM) and renders through a custom `storageRenderer` registered at priority 100 (below the default HTML=1000 and table=500 renderers) that emits Confluence storage format. `shield.go` renames raw `ac:`/`ri:` tags to colon-free sentinels around the goldmark step so pasted storage passes through; `callouts.go` is an AST transformer + blockquote renderer for GitHub alerts; `aclink.go` is the *inverse* direction's one element with enough shape to need its own file — ``, which the editor writes for every internal link and `MdToConfluence` never emits, so nothing in the regression suite covers it. One rule decides its whole mapping: **convert when the markdown republishes to a link resolving to the same target, pass the storage through when it would not** — so a page link and a space link convert, while a mention (80% of all real usage), an attachment link (only images are uploaded, so a relative href would be dead) and an unresolvable target stay raw, which the shield republishes byte-identical. A page target is a **title, never an id**, so `PageLinkTargets` reports what needs resolving and `StorageOptions.PageLinks` carries the answers back. An `ac:anchor` is **percent-encoded** where `confluenceSlug` output is not: decode it before matching a heading, leave it encoded inside a URL. A same-page anchor recovers its heading from the document rather than inverting the slug, which is impossible — `confluenceSlug` turns both a space and a hyphen into `-`. The survey the mapping rests on, and the `xml.HTMLAutoClose` trap that made `` crash the parser outright (#88), are in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `attachname.go` owns the source-path↔attachment-name mapping (percent-encoding `%`→`%25` then `/`→`%2F`, which is **bijective** — that is what makes the dedupe collision-free and lets `read` recover an image's original path; decode refuses an absolute result, which markfluence never produces; what names Confluence accepts is in [docs/confluence/attachments.md](docs/confluence/attachments.md)); `destination.go` owns the **other** codec, destination↔path (`decodeDestination`/`encodeDestination`), shared by images *and* doc links — a markdown destination is a URL, so decode inbound (**before** `withinRoot`, or an encoded `..%2F` slips the clamp) and encode outbound in `storage_to_md.go` (or `export` emits markdown that no longer parses, and `sourceFor`'s absolute-path refusal is undone by the next read); an undecodable destination is a literal `%` in a filename, not an error; the reasoning is in [docs/confluence/links-and-anchors.md](docs/confluence/links-and-anchors.md); `images.go` (resolution stays page-relative like GitHub; the documentation root — cwd — bounds what may be published, and an image above it is `IMAGE BROKEN`), `links.go` (sibling-file scans, GitHub/Confluence slugs, doc-link + anchor rewriting; `docKey` is the single lookup key for both the page and anchor maps. Failure here is **silent** — an unresolved link publishes as a dead relative href with no warning), `tables.go` (the `

` tag, stamped with `data-layout="align-start"` so tables auto-size and left-align — this must stay if column widths are ever emitted, or a `` silently induces a layout; plus cells: an AST transformer consumes a leading `` comment in a cell and `renderTableCell` emits it as `data-highlight-colour`; `storage_to_md.go`'s `cellTexts` reverses this, reading `data-highlight-colour` back into a `bg:` marker (`cellBGNames`, the reverse of `tables.go`'s swatch map — a hex outside the 21 swatches round-trips as the literal hex, and where two names share a hex the British spelling wins, matching Confluence's own `-colour`), and a column's GFM alignment becomes a `

` wrapper **inside** the cell — never the `align` attribute the GFM renderer would emit, which is the one form Confluence discards. Only center and right are emitted: Confluence has no explicit left, so `:---` publishes bare and `read` recovers it as `---`. Since alignment is per-paragraph there and per-column in GFM, `columnSeparators` in `storage_to_md.go` takes each column's most common declared alignment (ties to the first seen) and drops the rest. Rows still fall through to the GFM renderer. Layout values, cell alignment, and colors: [docs/confluence/storage-format.md](docs/confluence/storage-format.md)), and `renderer.go` (code macros, text soft-break→space, images, links) do the rest. The `` and `` token substitutions happen **inside** `MdToConfluence`. `storage_to_md.go`'s `coalesceSplitMarks` repairs a bold/italic span containing a link once Confluence's own editor has touched the page: `MdToConfluence` always nests a mark around a link (`text link`), but ADF carries marks per text run rather than as nested elements, so any save through the editor re-serializes that into two adjacent runs sharing the mark instead (`text link`) — rendered independently that produces `**text **[**link**](url)`, whose closing `**` is preceded by a space and so does not open emphasis under CommonMark's flanking rule at all, not merely losing the styling but coming back with literal asterisks. `coalesceSplitMarks` hoists the mark to wrap the whole run, the only shape markdown can express, and only when the link's *entire* content shares the mark — a link only partly marked is left alone, since there's nothing to hoist. The same split happens for an internal `` in place of `` (verified the same way), whose visible text sits one level deeper inside `ac:link-body` rather than directly on the link element — `isLinkNode`/`linkTextBody` cover both shapes. - `internal/frontmatter` — flat YAML frontmatter parse/quote/`UpdateField`, and the `MarkdownFile` type (`Parse`/`ParseFile`, exported `Filename`/`Content`/`Frontmatter`/`Body`, and `Title`/`PageID`/`Space`/`Parent` accessors that normalize missing/blank/`"null"`). - `internal/pagewidth` — the `page_width` `Width` enum (`narrow`/`wide`/`max`, default `max`), `Declared`, the vocab↔content-property maps, `WidthFromProperties`, and `Apply`/`Read` against the client. Width lives in two content properties and **both** must be written or the reader and the editor disagree: [docs/confluence/page-width.md](docs/confluence/page-width.md). - `internal/schematest` — the `--json` drift guard, and the reason the schema can't fall behind the code. `ValidateEnvelope`/`ValidateError` validate an emitted document against the embedded schema; `document.go` checks the schema *document* instead (`Commands`, plus tests that every name in the `command` enum has an `if/then` branch that constrains `results.items` **and** `summary`). That last one matters because outside a branch the schema says only "results is an array": a command added to the enum without a branch is completely unvalidated, and adding just the enum entry is exactly how a new command's conformance test goes green. `cmd`'s `TestCommandEnumMatchesRegisteredCommands` closes the loop from the other side — every registered subcommand is in the enum or in that test's `noJSONEnvelope` list. Two rules keep all this working: **every result field lives on a typed struct and nothing uses `omitempty`** (so every field always marshals and `additionalProperties:false`/`required` catch an added, renamed, or removed one no matter what a fixture sets — never build a result as a `map[string]any`), and **a conformance test builds its document with the command's own builder** (`failEnvelope`, `jsonResult`) rather than a hand-copied literal, or it validates a copy while the real output drifts. diff --git a/internal/convert/aclink_test.go b/internal/convert/aclink_test.go index 08ee2c8..e559160 100644 --- a/internal/convert/aclink_test.go +++ b/internal/convert/aclink_test.go @@ -76,6 +76,31 @@ func TestACLinkResolvedPage(t *testing.T) { } } +// TestACLinkCoalescesSplitBoldMark covers the same editor-induced mark split +// coalesceSplitMarks repairs for (storage_to_md_test.go), but for an +// internal : bold text followed by a bold internal page link comes +// back from Confluence's editor as two adjacent runs sharing the mark instead +// of one nested element -- text ... +// y -- because the link's visible +// text lives inside ac:link-body, one level deeper than 's. Verified live +// 2026-08-30 the same way as the case: a direct atlas_doc_format PUT with +// the link mark's href pointing at another Confluence page. +func TestACLinkCoalescesSplitBoldMark(t *testing.T) { + storage := `

some text ` + + `y

` + want := "**some text [y](" + pageURL + ")**" + + got, err := convert.StorageToMarkdown(storage, convert.StorageOptions{ + PageLinks: map[convert.PageLinkTarget]string{{Title: "IT 2026 Roadmap"}: pageURL}, + }) + if err != nil { + t.Fatalf("StorageToMarkdown: %v", err) + } + if strings.TrimSpace(got) != want { + t.Errorf("got %q, want %q", strings.TrimSpace(got), want) + } +} + // TestACLinkUnresolvedPageIsPassedThrough is the fallback that keeps a failed or // skipped lookup from silently deleting a link. A markdown link with no // destination would be worse than the storage, which still works. diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 7a3df50..abd4b4b 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -515,12 +515,163 @@ func (r *mdRenderer) renderCallout(n *snode, macro string) string { // renderInlineChildren renders a node's children as a single inline string. func (r *mdRenderer) renderInlineChildren(n *snode) string { var b strings.Builder - for _, k := range n.kids { + for _, k := range coalesceSplitMarks(n.kids) { b.WriteString(r.renderInline(k)) } return strings.TrimSpace(b.String()) } +// formatMarks are the inline formatting tags coalesceSplitMarks may hoist +// across a link boundary. +var formatMarks = map[string]bool{ + "strong": true, "b": true, + "em": true, "i": true, + "del": true, "s": true, "strike": true, +} + +// coalesceSplitMarks merges a formatting run that Confluence's own editor can +// split around a link. ADF (Confluence's native document model) carries marks +// per text run rather than as nested elements, so "**text [link](url)**" -- +// which markfluence always writes nested, as one wrapping both the +// text and the link -- comes back from a page that has since been edited and +// saved in Confluence's editor as two adjacent runs sharing the mark instead: +// text link. Verified +// 2026-08-30 via a direct atlas_doc_format PUT of the unmodified ADF markfluence +// itself had published, which is what the editor does on any save; the same PUT +// with the link's href pointing at another Confluence page produces the +// identical split with in place of (see isLinkNode). Rendered as +// two independent nodes that becomes "**text **[**link**](url)": the closing +// ** is preceded by a space, so CommonMark's flanking rule refuses to treat it +// as emphasis at all -- the markdown comes back not merely unstyled but +// literally reading "**text **". This restores the nested form before +// rendering, the only shape markdown can actually express, by hoisting the +// mark to wrap the whole run including the link and dropping the now-redundant +// inner one. +// +// Merging two adjacent same-tag mark elements outright (mergeMarkRun's first +// case, needed nowhere else) is what lets a third run on either side of the +// link fold into an already-repaired node; it is not itself a repair, since +// "ab" is valid nested markdown either way. +func coalesceSplitMarks(kids []*snode) []*snode { + out := make([]*snode, 0, len(kids)) + for _, k := range kids { + if len(out) > 0 { + if merged := mergeMarkRun(out[len(out)-1], k); merged != nil { + out[len(out)-1] = merged + continue + } + } + out = append(out, k) + } + return out +} + +// isLinkNode reports whether n is a link element coalesceSplitMarks may hoist +// a mark across: a markdown link, or the editor's own internal (used +// for a page, space, or user link -- see aclink.go). +func isLinkNode(n *snode) bool { + return n.name == "a" || n.name == "ac:link" +} + +// linkTextBody returns the node whose children hold a link's visible text -- +// the itself, or an 's -- or nil if it has neither. +// An 's other body spelling, ac:plain-text-link-body, holds CDATA and +// so can never carry a mark element to unwrap. +func linkTextBody(n *snode) *snode { + if n.name == "a" { + return n + } + return findChild(n, "ac:link-body") +} + +// withLinkTextBody returns a copy of link node n with body's children replaced +// by kids -- unwrapping a mark mergeMarkRun is hoisting out of it. body is +// n itself for an , or its child for an , whose +// other children (ri:page, ac:anchor, ...) must survive untouched. +func withLinkTextBody(n, body *snode, kids []*snode) *snode { + if n.name == "a" { + return &snode{name: "a", attrs: n.attrs, kids: kids} + } + newKids := make([]*snode, len(n.kids)) + for i, k := range n.kids { + if k == body { + k = &snode{name: k.name, attrs: k.attrs, kids: kids} + } + newKids[i] = k + } + return &snode{name: n.name, attrs: n.attrs, kids: newKids} +} + +// mergeMarkRun merges two adjacent inline nodes when they carry the same +// formatting mark: either both are the same mark element, or one is a mark and +// the other is a link whose entire visible text is that same mark (the split +// coalesceSplitMarks exists to repair). Returns nil when they don't combine. +func mergeMarkRun(prev, cur *snode) *snode { + switch { + case prev.name == cur.name && formatMarks[prev.name]: + return &snode{name: prev.name, attrs: mergeAttrs(prev.attrs, cur.attrs), kids: concatKids(prev.kids, cur.kids)} + case formatMarks[prev.name] && isLinkNode(cur): + if link := hoistMarkIntoLink(cur, prev.name); link != nil { + return &snode{name: prev.name, attrs: prev.attrs, kids: concatKids(prev.kids, []*snode{link})} + } + case isLinkNode(prev) && formatMarks[cur.name]: + if link := hoistMarkIntoLink(prev, cur.name); link != nil { + return &snode{name: cur.name, attrs: cur.attrs, kids: concatKids([]*snode{link}, cur.kids)} + } + } + return nil +} + +// mergeAttrs unions two attribute maps; a key present in both keeps a's value, +// so merging n adjacent same-tag runs left to right is order-independent. +// nil-safe in both directions, since most snodes carry no attrs at all. +func mergeAttrs(a, b map[string]string) map[string]string { + if len(b) == 0 { + return a + } + out := make(map[string]string, len(a)+len(b)) + for k, v := range b { + out[k] = v + } + for k, v := range a { + out[k] = v + } + return out +} + +// hoistMarkIntoLink strips a redundant mark wrapping the entirety of link's +// visible text, returning the link with that text unwrapped, or nil if the +// link has no text body or is not entirely marked (a link only partly marked +// is left alone: there's nothing correct to hoist). +func hoistMarkIntoLink(link *snode, mark string) *snode { + body := linkTextBody(link) + if body == nil { + return nil + } + inner, ok := unwrapSoleMark(body, mark) + if !ok { + return nil + } + return withLinkTextBody(link, body, inner) +} + +// unwrapSoleMark reports whether n's entire content is a single child element +// carrying the given mark, returning that child's own children -- the link's +// content with the redundant inner mark stripped. +func unwrapSoleMark(n *snode, mark string) ([]*snode, bool) { + if len(n.kids) != 1 || n.kids[0].name != mark { + return nil, false + } + return n.kids[0].kids, true +} + +// concatKids returns a with b appended, without aliasing either's backing array. +func concatKids(a, b []*snode) []*snode { + out := make([]*snode, 0, len(a)+len(b)) + out = append(out, a...) + return append(out, b...) +} + // renderInline renders one inline node. func (r *mdRenderer) renderInline(n *snode) string { if n.name == "" { diff --git a/internal/convert/storage_to_md_test.go b/internal/convert/storage_to_md_test.go index 4125f7d..51dfa17 100644 --- a/internal/convert/storage_to_md_test.go +++ b/internal/convert/storage_to_md_test.go @@ -210,6 +210,54 @@ func TestStorageToMarkdownStripsGeneratedIDs(t *testing.T) { } } +// TestStorageToMarkdownCoalescesSplitMarks checks the repair for a bold (or +// italic) span that Confluence's own editor splits around a link: saving a page +// through the editor stores marks per text run rather than as nested elements, +// so "text link" -- which is all MdToConfluence ever +// writes -- can come back as two adjacent runs sharing the mark instead, +// "text link". Rendered independently +// that produces "**text **[**link**](url)", whose closing ** is preceded by a +// space and so does not open emphasis at all under CommonMark's flanking rule -- +// verified live on 2026-08-30 by PUTting a page's own unmodified +// atlas_doc_format back at it, which is what the editor does on every save. +func TestStorageToMarkdownCoalescesSplitMarks(t *testing.T) { + tests := map[string]struct { + in, want string + }{ + "bold text then bold link": { + in: `

some text x

`, + want: "**some text [x](https://example.com)**\n", + }, + "bold link then bold text": { + in: `

x more text

`, + want: "**[x](https://example.com) more text**\n", + }, + "italic text then italic link": { + in: `

x y

`, + want: "*x [y](https://example.com)*\n", + }, + "adjacent same-mark runs with no link still merge": { + in: `

ab

`, + want: "**ab**\n", + }, + "link only partly bold does not merge": { + in: `

a bc

`, + want: "**a**[b**c**](https://example.com)\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got, err := convert.StorageToMarkdown(tc.in, convert.StorageOptions{}) + if err != nil { + t.Fatalf("StorageToMarkdown: %v", err) + } + if got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + // TestRoundTripPassthrough verifies that the raw-storage passthrough cases // (column layouts and unknown macros) survive markdown -> storage -> markdown // unchanged -- the whole point of emitting them in a form MdToConfluence diff --git a/internal/convert/testdata/storage2md/split-marks/input.storage b/internal/convert/testdata/storage2md/split-marks/input.storage new file mode 100644 index 0000000..a54d5b2 --- /dev/null +++ b/internal/convert/testdata/storage2md/split-marks/input.storage @@ -0,0 +1,4 @@ +

Editor-split bold text and link: some text x.

+

Editor-split italic link then text: x more text.

+

Adjacent same-tag runs with no link nearby: ab.

+

A link only partly bold does not merge: a bc.

diff --git a/internal/convert/testdata/storage2md/split-marks/output.md b/internal/convert/testdata/storage2md/split-marks/output.md new file mode 100644 index 0000000..d60c96f --- /dev/null +++ b/internal/convert/testdata/storage2md/split-marks/output.md @@ -0,0 +1,7 @@ +Editor-split bold text and link: **some text [x](https://example.com)**. + +Editor-split italic link then text: *[x](https://example.com) more text*. + +Adjacent same-tag runs with no link nearby: **ab**. + +A link only partly bold does not merge: **a**[b**c**](https://example.com).