From 5a1ea6ac8acce8f02f14cd91c6692292b797b58c Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 25 Sep 2026 13:44:46 -0400 Subject: [PATCH 1/8] docs: the raw-tables-round-trip plan --- _plans/055_raw-tables-round-trip.md | 414 ++++++++++++++++++++++++++++ 1 file changed, 414 insertions(+) create mode 100644 _plans/055_raw-tables-round-trip.md diff --git a/_plans/055_raw-tables-round-trip.md b/_plans/055_raw-tables-round-trip.md new file mode 100644 index 0000000..88cab83 --- /dev/null +++ b/_plans/055_raw-tables-round-trip.md @@ -0,0 +1,414 @@ +# 055: raw storage tables round-trip + +Answers #55. A table that Markdown cannot express can be written as raw +storage, and publishing already passes it through intact. But `read` and +`export` turn every table into a GFM pipe table and drop what does not fit, +so read → edit → update silently strips a hand-built table. This plan makes +`read` write such a table as raw storage, and documents the escape hatch. + +## What it looks like + +Each example is a table on a page and what `read` writes for it after this +plan. Storage is abbreviated to the parts that decide. + +**1. A table the editor saved, nothing configured: a pipe table.** The +editor's `data-table-width`, `ac:local-id` and default display mode are +ignored (D2). + +``` + + + + +

Service

Owner

auth

SRE

+``` + +```markdown +| Service | Owner | +| --- | --- | +| auth | SRE | +``` + +**2. Colours and a column alignment: still a pipe table**, as today. + +``` +

Service

Errors

+

auth

+

3

+``` + +```markdown +| Service | Errors | +| --- | ---: | +| auth | 3 | +``` + +**3. A layout and column widths: raw.** The table, row and cell tags are +storage; each cell's body is Markdown between blank lines (D4). A colour on a +raw cell stays the `data-highlight-colour` attribute. + +``` + ++++ + + + + + + + + + + +
+ +Service + + + +Owner + +
+ +auth + + + +[SRE runbooks](runbooks.md) + +
+``` + +**4. Merged cells: raw.** `rowspan` and `colspan` stay on the cell. + +``` + + + + + + + + + + + + + +
+ +Q3 results + +
+ +**auth** + + + +99.9% + +
+ +99.8% + +
+``` + +**5. No header row, or a header column: raw.** GFM needs a header row across +the top; today `read` promotes the first row, and the next publish turns its +cells into headers. Written the same way as 3 and 4, with `` or a leading +`` in each row as the page has them. + +**6. Block content in a cell: raw, and the block stays Markdown.** + +```` + + + +Restart with: + +```bash +systemctl restart auth +``` + + + +```` + +**7. An aligned paragraph inside a raw cell** is written as a raw `

` on its +own line, since a Markdown paragraph has no alignment (D4). Its text is then +storage, not Markdown. + +``` + + +

centred

+ + +``` + +**8. A column whose cells disagree on alignment: raw**, where today the most +common alignment wins and the other cells are republished with it. + +## What Confluence does with table markup + +**Verified 2026-09-25** by writing one table per hypothesis to a scratch page +in the personal space, reading back storage and ADF (what the editor and +renderer use, docs/confluence/storage-format.md), then trashing the page. +Rows marked *known* repeat the 2026-08-07 findings already in that document. + +| written | stored | takes effect (ADF) | +|---|---|---| +| `` in the first row | kept | header row | +| `` first in every row | kept | header column | +| two header rows in `` | kept | two header rows | +| `` | kept | an ordinary last row; no footer | +| `` | **tag dropped, its text left loose** | the caption text becomes a paragraph above the table | +| `colspan`, `rowspan` on ``/`` | kept | yes | +| `data-highlight-colour` on a cell (*known*) | kept | cell background | +| `style="background-color: …"` on a cell | kept, as `rgb()` | **no** | +| `class="highlight-blue"` on a cell | kept | no | +| `text-align` on a cell's `

` or the cell (*known*) | kept | paragraph alignment | +| `valign="bottom"` on a cell | kept | **yes**, cell `valign` | +| `style="vertical-align: top;"` on a cell | kept | no | +| `scope="col"` on a `` | kept | no | +| `data-colwidth` on a cell | **dropped** | no | +| `` of px widths (*known*) | kept | column widths; with no `data-layout`, a layout picked by total width | +| `data-layout` `default`, `align-start`, `center`, `wide`, `full-width`, `align-end` (*known*, plus `default`) | kept | the layout | +| `data-table-width` (*known*) | kept | table width | +| `data-table-display-mode="fixed"` | kept | `displayMode: fixed` | +| `data-number-column="true"` | **dropped** | no | +| `class="numberingColumn"` on each row's first cell | kept | **numbered column**, with those cells removed from ADF | +| `class`, `border`, `width`, `style="width: …"` on `` | kept | no (a `style` width induced `layout: default`) | +| a table inside a cell | kept | **not a table**: an uneditable `nested-table` migration extension ("A table in a table cell can't be created or edited in the new editor") | + +So the vocabulary that works is: header rows and columns, `colspan`/`rowspan`, +`data-highlight-colour`, paragraph or cell `text-align`, `valign`, a px (or, +with `data-table-width`, percentage) ``, `data-layout`, +`data-table-width`, `data-table-display-mode`, and a numbered column spelled +as `numberingColumn` cells. Everything else is stored and ignored, dropped, +or -- ` @@ -18,9 +18,9 @@ Service auth - diff --git a/internal/convert/testdata/storage2md/table-list-ids/input.storage b/internal/convert/testdata/storage2md/table-list-ids/input.storage new file mode 100644 index 0000000..955a347 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-list-ids/input.storage @@ -0,0 +1,6 @@ +
` and nested tables -- actively harmful. + +## What was checked + +**Verified 2026-09-25**, offline with `check --show-html` and +`StorageToMarkdown`, and read-only against the live instance. + +- **Publishing works today.** A raw `` carrying `data-layout="center"`, + `data-table-width`, a ``, `rowspan`, `data-highlight-colour`, a + `

`, and a blank line inside comes out intact. + Markdown between blank lines inside a `

`. + +**Not verified:** whether an editor save adds `data-table-width` to a table +markfluence published. It does not matter under D2, which ignores the +attribute either way. + +## Decisions + +**D1. A table is written as a pipe table only when Markdown expresses all of +it.** Otherwise it is written as a raw table (D4). A table that is *nearly* +expressible -- one `rowspan` -- is raw too: degrading one cell is still loss, +and the raw form is exact. + +**D2. What a pipe table may carry.** These are ignored when deciding, because +the editor writes them on tables nobody configured, and treating them as +intent would make every table the editor has saved come back as HTML: + +- `ac:local-id` anywhere (already dropped on the way back); +- `data-table-width`, whatever its value; +- `data-table-display-mode="default"`; +- `data-layout="align-start"`, or no `data-layout` at all; +- `rowspan="1"` and `colspan="1"`. + +What a pipe table expresses, beyond the table itself: + +- `data-highlight-colour` on a cell (a `bg:` marker); +- a column alignment (the delimiter row), from a cell's `style` or its + paragraphs' `text-align`, as today. + +Anything else on `
` is converted; Markdown tight + against the tags stays literal. markfluence stamps no + `data-layout="align-start"` on a raw table. +- **So does the cell form this plan writes** (D4): table, row and cell tags on + their own lines, each cell's body as Markdown between blank lines. A cell + holding bold text, a link, an image and a list published as the storage + the Markdown describes. +- **`read` destroys the same table.** It comes back as a pipe table with the + layout, width, colgroup and `rowspan` gone, and a ragged last row + (`| x |` in a two-column table). +- **Already done by #48 and #54:** `data-highlight-colour` reads back as a + `` marker, and paragraph or cell `text-align` as the + delimiter row. +- **The editor stamps attributes on every table it saves.** Page 2913502220 + (the issue's example, three tables made in the editor) carries + `data-table-width` (1110 on two tables, 778 on the third) on every table, + `ac:local-id` on every table, row and cell, and + `data-table-display-mode="default"` on one. No `
`, ``, ``; +- `rowspan` or `colspan` greater than 1; +- **no header row**, or a header anywhere else: the first row must be all + ``. A `` and +its ``s are serialized raw on their own lines. A nested table inside a +cell goes through the same decision. + +Attributes are written as the page has them, `ac:local-id` dropped as +everywhere else. A raw table keeps its `data-table-width`: D2 ignores it only +for the decision. + +A paragraph inside a raw cell that carries an attribute Markdown cannot hold +-- in practice `text-align` -- is serialized raw on its own line (example 7). +Its text is storage rather than Markdown, which costs editability in that one +paragraph; rendering it as a plain paragraph would drop the alignment, which +is the loss this plan exists to stop. Scoped to raw table cells: the same gap +exists for every aligned paragraph `read` meets (top level, layout cells, +macro bodies all read back unaligned), and closing it everywhere is a separate +change (Not in scope). + +**D5. The Markdown side is a fixed point.** read → publish → read gives the +same Markdown after one cycle, as for layouts. Publishing a raw table writes +it back with the same attributes, so the second read makes the same decision. + +**D6. No warning when a table falls back to raw.** Deferred until someone asks. + +## Implementation + +**`internal/convert/storage_to_md.go`:** + +- `renderTable` asks a new `tableExpressible(n)` first and hands the table to + `renderRawBlock` when it answers no. +- `tableExpressible` checks D2 and D3 in one walk: the allowlisted attributes, + the header shape, the row widths, each cell's children, and each column's + alignments. +- `isContentContainer` gains `th` and `td`, so `renderRawBlock` writes a + cell's body as Markdown. They only occur inside a table, and a table only + reaches `renderRawBlock` through this path. +- `columnSeparators` loses its vote: once D3 guarantees a column agrees, it + reads the column's one alignment. The majority logic and its comment go. +- `renderTable`'s doc comment ("Alignment is not preserved") is wrong since + #48 and is rewritten. + +Nothing changes on the publish side. + +## Documentation + +- **`docs/markdown-file.md`**, under Tables: a "Column alignment" subsection + (missing since #48: `:---:` and `---:`, no explicit left), and a "Tables + Markdown cannot express" subsection: write the table as raw storage, with + the D4 cell convention and an example. Two gotchas from + docs/confluence/storage-format.md go with it: a raw table gets no + `data-layout` from markfluence, and a `` with no `data-layout` + makes Confluence pick one by total width, so a table with column widths + must name its layout; percentage widths need `data-table-width`, px widths + do not. It also says what `read` does: which tables come back as pipe + tables, and that the rest come back raw. The "Raw Confluence storage + format" section links to it. It lists the table markup that works (the + vocabulary under "What Confluence does with table markup"), with + examples 3, 4 and 6 above, and warns against the three harmful shapes: + ``, `rowspan`, `colspan`, no header + row, a header column, a short row, a heading in a cell, a code macro in a + cell, a nested table, a column whose alignments disagree, a numbered + column (`numberingColumn` cells), `valign`, `data-table-display-mode="fixed"`, + and an aligned paragraph inside a raw cell (example 7). Plus one + editor-shaped table (D2's attributes, a colour, an agreeing alignment) that + stays a pipe table. Each example under "What it looks like" is one of + these cases, so the plan's examples are the goldens. +- **`TestRoundTripPassthrough`** gains `raw-table`: read → publish → read is + stable, and the storage keeps the attributes. The existing + `TestRoundTripMarkdownIsAFixedPoint` covers every new case for D5. +- **Forward:** `testdata/regression/raw-storage/main.md` gains a raw table + with blank lines inside, Markdown in a cell, and one cell tight against its + tags, so the publish path's behaviour is pinned. +- **Changed output:** `storage2md/table-alignment` has two columns whose + cells disagree ("Cell form": right, none; "ADF names": start, end), written + to test the vote. It is split: the recovery forms (cell style, paragraph + style, `end`) in columns that agree, which stay a pipe table, and a + disagreeing column in its own case, which goes raw. + `TestRoundTripTableAlignment` is checked against the same split. + +## Not in scope + +- **A warning on fallback** (D6). +- **An empty-header convention.** A table with no header row could be read + as a pipe table with an empty header row, if publishing learned to treat + one as "no header". It changes what existing files mean (an empty header + row publishes empty `` with no `data-layout`, a + ` diff --git a/internal/convert/testdata/storage2md/raw-table-repeated-align/input.storage b/internal/convert/testdata/storage2md/raw-table-repeated-align/input.storage new file mode 100644 index 0000000..b4d3358 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-repeated-align/input.storage @@ -0,0 +1,6 @@ +
`, `` or a cell's `

` makes the +table raw. The list is an allowlist, so an attribute nobody has seen yet is +kept rather than dropped; it gets honed as real pages turn up noise. + +The cost of ignoring `data-table-width`: a table someone resized by hand in the +editor loses the resize on the next publish of its read-back file. The +alternative sends every table the editor has touched back as HTML. + +**D3. What makes a table raw.** + +- any other `data-layout` (`center`, `wide`, `full-width`, `align-end`...); +- a `

` and no other row may hold one. GFM has no table without a header + row, and today `read` promotes the first row, so republishing turns its + ``s into ``s. A header *column* is the same case; +- a row with a different number of cells than the header (today it becomes a + ragged row that GFM pads or truncates); +- **block content in a cell** other than paragraphs and `
    `/`
      `: a + heading, a code block or any other block macro, a nested table, a + blockquote, `
      `, `
      `, a layout. An image or a status macro inside a + paragraph is inline and stays expressible; +- **cells in one column that do not agree on alignment**, counting no + alignment, `left` and `start` as one value (Confluence has no explicit + left, docs/confluence/storage-format.md). Today the most common alignment + wins and the rest are dropped, which republishes an aligned column over + cells that were not. + +The last two are consequences of D1 rather than separate choices; they are +listed because they change today's output (see Tests). + +**D4. The raw form keeps each cell's body as Markdown.** Table, section, row +and cell tags are raw storage, one per line; each `
`/``'s body is +converted to Markdown and set off by blank lines -- the convention `read` +already uses for `ac:layout-cell` and a macro's rich-text body, and the one +publishing already accepts: + +``` + + + + + + + +
+ +**Owner** is [here](https://x) + + + +x + +
+``` + +Longer than verbatim storage, but the text stays editable, and a page link, +an image or a mention keeps its Markdown form instead of becoming an +`` to edit by hand. An empty cell is `
` (its text escapes above the table), a nested table (the editor + cannot edit it), and `style` colours or `class` names (stored and ignored; + use `data-highlight-colour`). +- **`docs/confluence/storage-format.md`**: the probe table above as a new + "Table markup" section with its date, `default` added to the layout list, + the editor's table attributes (page 2913502220), and a line on D2's reading + of them. +- **CLAUDE.md**: the converter bullet's `storage_to_md.go` table sentences + gain the fallback rule and the ignored attributes. + +## Tests + +- **`storage2md` cases**, one per trigger, each read back as a raw table: a + non-default `data-layout`, a `
`s today) and GitHub's preview shows a blank + header. Raw is exact; this waits for evidence that headerless tables are + common enough to matter. +- **A `check` lint** for a raw `
`, or a nested table. The docs warn about all three. +- **Aligned paragraphs outside raw table cells** (D4). `read` drops the + alignment of a paragraph at the top level, in a layout cell or in a macro + body today, and always has. +- **Honouring a hand resize** (`data-table-width` on a pipe table). It would + need a way to write a width in Markdown. From 47d9dca4e1edd5bddae1cd8939b9404b33a1c004 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 25 Sep 2026 13:49:49 -0400 Subject: [PATCH 2/8] feat(convert): read a table Markdown cannot express as raw storage read and export turned every table into a GFM pipe table and dropped whatever did not fit -- a layout, column widths, merged cells, a headerless shape -- so read, edit, update stripped a hand-built table from its page (#55). A table now reads back as a pipe table only when Markdown expresses all of it, decided over an allowlist; otherwise it is written as raw storage with each cell's body as Markdown between blank lines, the convention layout cells already use. The attributes the editor writes on every table it saves (ac:local-id, data-table-width, the default display mode, align-start) are ignored in the decision. A column whose cells disagree on alignment is now raw rather than voted on. The raw-table escape hatch is documented, with the table markup that takes effect in Confluence, verified against the live instance. --- CLAUDE.md | 2 +- docs/confluence/storage-format.md | 49 +++ docs/markdown-file.md | 107 ++++++ internal/convert/storage_to_md.go | 190 +--------- internal/convert/storage_to_md_table.go | 342 ++++++++++++++++++ internal/convert/storage_to_md_test.go | 26 +- .../testdata/regression/raw-storage/main.md | 32 ++ .../regression/raw-storage/test.output | 2 +- .../raw-table-aligned-paragraph/input.storage | 6 + .../raw-table-aligned-paragraph/output.md | 20 + .../raw-table-block-content/input.storage | 6 + .../raw-table-block-content/output.md | 32 ++ .../raw-table-colgroup/input.storage | 7 + .../storage2md/raw-table-colgroup/output.md | 32 ++ .../raw-table-display-fixed/input.storage | 6 + .../raw-table-display-fixed/output.md | 18 + .../raw-table-header-column/input.storage | 6 + .../raw-table-header-column/output.md | 28 ++ .../storage2md/raw-table-layout/input.storage | 7 + .../storage2md/raw-table-layout/output.md | 32 ++ .../storage2md/raw-table-nested/input.storage | 6 + .../storage2md/raw-table-nested/output.md | 20 + .../raw-table-no-header/input.storage | 6 + .../storage2md/raw-table-no-header/output.md | 28 ++ .../raw-table-numbered/input.storage | 6 + .../storage2md/raw-table-numbered/output.md | 24 ++ .../raw-table-short-row/input.storage | 7 + .../storage2md/raw-table-short-row/output.md | 35 ++ .../storage2md/raw-table-spans/input.storage | 7 + .../storage2md/raw-table-spans/output.md | 30 ++ .../storage2md/raw-table-valign/input.storage | 6 + .../storage2md/raw-table-valign/output.md | 30 ++ .../table-alignment-disagree/input.storage | 16 + .../table-alignment-disagree/output.md | 40 ++ .../storage2md/table-alignment/input.storage | 13 +- .../storage2md/table-alignment/output.md | 8 +- .../storage2md/table-editor/input.storage | 12 + .../storage2md/table-editor/output.md | 3 + 38 files changed, 1054 insertions(+), 193 deletions(-) create mode 100644 internal/convert/storage_to_md_table.go create mode 100644 internal/convert/testdata/storage2md/raw-table-aligned-paragraph/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-aligned-paragraph/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-block-content/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-block-content/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-colgroup/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-colgroup/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-display-fixed/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-display-fixed/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-header-column/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-header-column/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-layout/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-layout/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-nested/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-nested/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-no-header/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-no-header/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-numbered/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-numbered/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-short-row/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-short-row/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-spans/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-spans/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-valign/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-valign/output.md create mode 100644 internal/convert/testdata/storage2md/table-alignment-disagree/input.storage create mode 100644 internal/convert/testdata/storage2md/table-alignment-disagree/output.md create mode 100644 internal/convert/testdata/storage2md/table-editor/input.storage create mode 100644 internal/convert/testdata/storage2md/table-editor/output.md diff --git a/CLAUDE.md b/CLAUDE.md index 6f5b836..0877349 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,7 +79,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 `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. 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 that **declares** a `page_id` — in its own frontmatter or in a `pages:` entry for it (#139), resolved through `pagemeta` after discovering the root from the file's own directory (stat'd first, so `123.md` is a file). Every command taking a page uses it, which is why the manifest lookup lives here rather than at the seven call sites: without it the page argument meant one thing to `update` and another to `page-info`/`read`/`children`/`export`/`attachment-*`, so a file `update` could publish could not be named to any of them. A **disagreement** between the two locations is fatal here — it is the question being asked — while a **malformed** `markfluence.yaml` is not: a project file this resolver never consults must not make `page-info 123` fail, and the commands that bound reads by the root report it themselves. `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` and `update` report both and `check` reports the non-numeric one, and a reader should recognize the same problem across all of them. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. A **move** is `MovePage` (`move.go`), always the v1 `PUT /content/{id}/move/{position}/{targetId}` with `append` (under a page or folder) or `after` (after the last top-level page, the only way to the top of a space): the v1 route leaves the page version alone where a v2 `parentId` change bumps it, and v2 silently ignores a null `parentId`, so it cannot reach the top at all ([docs/confluence/api.md](docs/confluence/api.md#moving-a-page)). There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. 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. Every attachment file is read through `LocalAttachment.Open`, twice — once for `planAttachments`' checksum, once by `uploadAttachment`, which reads the file whole and takes the comment's checksum from *those* bytes, since the file may have changed in between and a comment misdescribing its content reads as up to date on the next publish. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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. `credentials.go` holds the credentials file's own functions — `CredentialsPath`, `ReadCredentials` (the rules `Resolve` uses, minus the permission warning, for a caller about to rewrite the file; one read returning a `CredentialsFile` of values, the lines a rewrite drops, and the mode), `WriteCredentials` (temp-file-and-rename at `0600`, writing through a symbolic link, quoting a value exactly when `readDotenv` would not read it back unchanged, then reading the result back and comparing), `DisplayPath`, and `FetchCloudID`, the unauthenticated `tenant_info` request, which bypasses `send` because `send` always sets basic auth. The setting names (`URLVar`/`UsernameVar`/`TokenVar`/`CloudIDVar`), `CredentialsDoc` and `LooseMode` are exported so `credentials-init` copies none of them. `HTTPError.ScopeMismatch`/`SiteRejectedAuth`, beside `RejectedCredential`, are the shapes `hint` matches, exported for `credentials-init`. `config.go` holds `Resolve` and the env-file reader (`loadDotenv`, which warns, over `readDotenv`, which does not), plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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, root *project.Root, index *linkindex.Index, baseURL, spaceKey string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in Markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `MentionMarkdown` is the shared builder for a mention's whole Markdown line, exported because `user-find` (#143) prints exactly it and a second copy there would be a second place to get the `@` marker and name escaping right. `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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); Plain text used as a Markdown link's text goes through `escapeLinkText` (`\`, `[`, `]`), applied to the *raw* sources only — a page title, a space key, an anchor, a display name — and via `inlineTextForLink` to a body whose every descendant is a text node. Never to already-rendered output: an `ac:link-body` holding markup has been converted to Markdown already, and escaping it yields a literal `\*\*bold\*\*`. Both directions are tested, because a fix at either extreme passes one and fails the other. `attachname.go` owns the source-path→attachment-name mapping, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. 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` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `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. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

` 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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`/``, a span of 1, cell children that are paragraphs, lists or inline markup, and every cell in a column agreeing on alignment (no alignment, `left` and `start` are one value) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Four attributes are **ignored** rather than honoured because the editor writes them on every table it saves (measured on a real page, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id`, `data-table-width` whatever its value (the known cost: a hand resize is lost when a read-back table republishes), `data-table-display-mode="default"`, and `data-layout="align-start"`; honouring them would send every editor-saved table back as HTML. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

` | kept | two header rows | +| `` | kept | an ordinary last row; no footer | +| `of measured pixel widths on an align-start table on any save. Both are now ignored, the only on align-start, so a markfluence table stays a pipe table after an edit in Confluence. A hand resize writes the same and is lost on the next publish. From the review, each reproduced first: loose inline content in a raw cell is one paragraph rather than a block per element, an empty paragraph in a raw cell stays

, loose text in a table wrapper is kept, an unknown text-align value keeps a table raw, a paragraph's own alignment beats its cell's, an empty cell does not vote on its column's alignment, the tags allowed loose in a pipe cell are those read can render, and renderRawBlock no longer renders a raw cell twice. --- CLAUDE.md | 2 +- _plans/055_raw-tables-round-trip.md | 25 ++ docs/confluence/storage-format.md | 30 ++- docs/markdown-file.md | 17 +- internal/convert/storage_to_md.go | 12 +- internal/convert/storage_to_md_table.go | 216 ++++++++++++++---- internal/convert/storage_to_md_test.go | 2 +- .../raw-table-cell-content/input.storage | 8 + .../raw-table-cell-content/output.md | 52 +++++ .../raw-table-colgroup/input.storage | 2 +- .../storage2md/raw-table-colgroup/output.md | 2 +- .../raw-table-unknown-align/input.storage | 6 + .../raw-table-unknown-align/output.md | 18 ++ .../table-alignment-edges/input.storage | 8 + .../table-alignment-edges/output.md | 5 + .../table-browser-saved/input.storage | 1 + .../storage2md/table-browser-saved/output.md | 5 + 17 files changed, 353 insertions(+), 58 deletions(-) create mode 100644 internal/convert/testdata/storage2md/raw-table-cell-content/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-cell-content/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-unknown-align/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-unknown-align/output.md create mode 100644 internal/convert/testdata/storage2md/table-alignment-edges/input.storage create mode 100644 internal/convert/testdata/storage2md/table-alignment-edges/output.md create mode 100644 internal/convert/testdata/storage2md/table-browser-saved/input.storage create mode 100644 internal/convert/testdata/storage2md/table-browser-saved/output.md diff --git a/CLAUDE.md b/CLAUDE.md index 0877349..462e483 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,7 +79,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 `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. 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 that **declares** a `page_id` — in its own frontmatter or in a `pages:` entry for it (#139), resolved through `pagemeta` after discovering the root from the file's own directory (stat'd first, so `123.md` is a file). Every command taking a page uses it, which is why the manifest lookup lives here rather than at the seven call sites: without it the page argument meant one thing to `update` and another to `page-info`/`read`/`children`/`export`/`attachment-*`, so a file `update` could publish could not be named to any of them. A **disagreement** between the two locations is fatal here — it is the question being asked — while a **malformed** `markfluence.yaml` is not: a project file this resolver never consults must not make `page-info 123` fail, and the commands that bound reads by the root report it themselves. `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` and `update` report both and `check` reports the non-numeric one, and a reader should recognize the same problem across all of them. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. A **move** is `MovePage` (`move.go`), always the v1 `PUT /content/{id}/move/{position}/{targetId}` with `append` (under a page or folder) or `after` (after the last top-level page, the only way to the top of a space): the v1 route leaves the page version alone where a v2 `parentId` change bumps it, and v2 silently ignores a null `parentId`, so it cannot reach the top at all ([docs/confluence/api.md](docs/confluence/api.md#moving-a-page)). There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. 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. Every attachment file is read through `LocalAttachment.Open`, twice — once for `planAttachments`' checksum, once by `uploadAttachment`, which reads the file whole and takes the comment's checksum from *those* bytes, since the file may have changed in between and a comment misdescribing its content reads as up to date on the next publish. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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. `credentials.go` holds the credentials file's own functions — `CredentialsPath`, `ReadCredentials` (the rules `Resolve` uses, minus the permission warning, for a caller about to rewrite the file; one read returning a `CredentialsFile` of values, the lines a rewrite drops, and the mode), `WriteCredentials` (temp-file-and-rename at `0600`, writing through a symbolic link, quoting a value exactly when `readDotenv` would not read it back unchanged, then reading the result back and comparing), `DisplayPath`, and `FetchCloudID`, the unauthenticated `tenant_info` request, which bypasses `send` because `send` always sets basic auth. The setting names (`URLVar`/`UsernameVar`/`TokenVar`/`CloudIDVar`), `CredentialsDoc` and `LooseMode` are exported so `credentials-init` copies none of them. `HTTPError.ScopeMismatch`/`SiteRejectedAuth`, beside `RejectedCredential`, are the shapes `hint` matches, exported for `credentials-init`. `config.go` holds `Resolve` and the env-file reader (`loadDotenv`, which warns, over `readDotenv`, which does not), plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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, root *project.Root, index *linkindex.Index, baseURL, spaceKey string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in Markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `MentionMarkdown` is the shared builder for a mention's whole Markdown line, exported because `user-find` (#143) prints exactly it and a second copy there would be a second place to get the `@` marker and name escaping right. `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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); Plain text used as a Markdown link's text goes through `escapeLinkText` (`\`, `[`, `]`), applied to the *raw* sources only — a page title, a space key, an anchor, a display name — and via `inlineTextForLink` to a body whose every descendant is a text node. Never to already-rendered output: an `ac:link-body` holding markup has been converted to Markdown already, and escaping it yields a literal `\*\*bold\*\*`. Both directions are tested, because a fix at either extreme passes one and fails the other. `attachname.go` owns the source-path→attachment-name mapping, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. 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` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `tables.go` (the `

`/`` as content containers so each cell's body stays Markdown between blank lines (an aligned `

` in a raw cell stays storage, since a Markdown paragraph has no alignment). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
` in the first row | kept | header row | +| `` first in every row | kept | header column | +| two header rows in `
` | **tag dropped, its text left loose** | the caption text becomes a paragraph above the table | +| `colspan`, `rowspan` on `
`/`` | kept | yes | +| `style="background-color: …"` on a cell | kept, as `rgb()` | **no** | +| `class="highlight-blue"` on a cell | kept | no | +| `valign="bottom"` on a cell | kept | **yes**, cell `valign` | +| `style="vertical-align: top;"` on a cell | kept | no | +| `scope="col"` on a `` | kept | no | +| `data-colwidth` on a cell | **dropped** | no | +| `data-table-display-mode="fixed"` | kept | `displayMode: fixed` | +| `data-number-column="true"` | **dropped** | no | +| `class="numberingColumn"` on each row's first cell | kept | **numbered column**, those cells removed from ADF | +| `class`, `border`, `width`, `style="width: …"` on `` | kept | no (the `style` width induced `layout: default`) | +| a table inside a cell | kept | **not a table**: a `nested-table` migration extension, "A table in a table cell can't be created or edited in the new editor" | + +Colour, alignment, layout and widths are in their own sections here. So the +markup that works is header rows and columns, `colspan`/`rowspan`, +`data-highlight-colour`, `text-align`, `valign`, a ``, +`data-layout`, `data-table-width`, `data-table-display-mode`, and a numbered +column spelled as `numberingColumn` cells. Everything else is stored and +ignored, or dropped; ``. + +So those attributes say nothing about what an author chose. `read` ignores +them when deciding whether a table can be a GFM table (#55, +`internal/convert/storage_to_md_table.go`), along with +`data-layout="align-start"` and a span of 1. `data-table-width` is the costly +one: it is also what a hand resize records, and a resized table that reads +back as GFM loses the resize on its next publish. + ## Cell background colors `data-highlight-colour` on a `` with a `` for each column | column widths | +| `data-layout` on the table: `align-start`, `align-end`, `center`, `default`, `wide`, or `full-width` | the layout of the table | +| `data-table-width` on the table, in pixels | the width of the table | +| `data-table-display-mode="fixed"` on the table | the fixed display mode of the editor | +| `class="numberingColumn"` on the first cell of each row | a numbered column | + +Details: + +- markfluence does not add `data-layout="align-start"` to a raw table, as it + does to a GFM table. You control all the attributes. +- If a table has a `` and no `data-layout`, Confluence picks a layout + from the total width of the columns. Wide columns make the table + `full-width`. Thus, if you give column widths, also give a `data-layout`. +- Column widths in pixels always work. Column widths in percent work only if + the table also has `data-table-width`. +- Do not use ` contains a is column widths, a a row Confluence + // renders as an ordinary one but that a pipe table would + // republish without the tag; anything else is unknown. + return pipeTable{}, false + } + } + if len(trs) == 0 { + return pipeTable{}, true + } + + var t pipeTable + for i, tr := range trs { + cells, ok := rowCells(tr) + if !ok || len(cells) == 0 { + return pipeTable{}, false + } + // GFM has no table without a header row, and no header anywhere but + // the top: the first row must be all + @@ -12,13 +13,15 @@ - + + - - - - + + + + +
` and a nested table do harm. + +### What the editor writes on a table + +**Verified 2026-09-25** on page 2913502220, three tables made in the editor: +every table carries `data-table-width` (1110 on two, 778 on the third) and +`ac:local-id`, every row and cell carries `ac:local-id`, and one table carries +`data-table-display-mode="default"`. None has a `
`/`` sets a cell background. It reaches ADF diff --git a/docs/markdown-file.md b/docs/markdown-file.md index c87e9f4..74d74b8 100644 --- a/docs/markdown-file.md +++ b/docs/markdown-file.md @@ -321,6 +321,110 @@ line, and a table row cannot do that. Thus you cannot use it here. `read` and `export` get back the same tags. They do not change them to anything else. +#### Column alignment + +To align a column, use the delimiter row of GFM: `:---:` centers a column and +`---:` aligns it to the right. + +```markdown +| Service | Errors | +| ------- | -----: | +| auth | 3 | +``` + +Confluence has no explicit left alignment. Left is its default. Thus `:---` +publishes the same as `---`, and `read` gives back `---`. + +#### Tables that Markdown cannot express + +A GFM table cannot express some things that a Confluence table can: column +widths, a layout other than the default, merged cells, a table with no header +row, a header column, or a code block or a heading in a cell. For these, write +the table as raw storage format. markfluence publishes it with no change. See +[Raw Confluence storage format](#raw-confluence-storage-format). + +Put each table, row, and cell tag on its own line. Put a blank line before and +after the content of a cell. Then markfluence converts the content of the cell +as Markdown. Without the blank lines, the content is storage format, and +markfluence does not convert it. + +``` + ++++ + + + + + + + + + + + + +
+ +Q3 results + +
+ +**auth** is [up](https://status.example.com) + + + +99.9% + +
+ +99.8% + +
+``` + +This is the table markup that has an effect in Confluence: + +| markup | effect | +| --- | --- | +| `
` cells in the first row | a header row | +| a `` cell first in each row | a header column | +| `colspan` and `rowspan` on a cell | merged cells | +| `data-highlight-colour="#rrggbb"` on a cell | the background color of the cell | +| `style="text-align: center;"` or `right` on a cell, or on a `

` in a cell | alignment | +| `valign` on a cell, such as `valign="bottom"` | vertical alignment | +| `

`. Confluence removes the tag and puts its text in a + paragraph above the table. +- Do not put a table in a table cell. The Confluence editor cannot edit a + nested table. +- Colors in `style` or `class` do nothing. Use `data-highlight-colour`. + +`read` and `export` give back a GFM table when GFM can express the whole +table. Otherwise, they give back a raw table in the form above, with the +content of each cell as Markdown. A paragraph with an alignment stays storage +format, because a Markdown paragraph has no alignment. `read` ignores some +attributes that the Confluence editor adds to every table that it saves, such +as `data-table-width`. Thus a table with a width that someone changed in the +editor comes back as a GFM table, and it loses that width when you publish it +again. + ### GitHub alerts GitHub alerts become Confluence panels in the color that GitHub gives them. The @@ -557,3 +661,6 @@ Right column. Storage markup in a fenced code block stays literal. markfluence does not activate it. + +A table uses the same conventions. See +[Tables that Markdown cannot express](#tables-that-markdown-cannot-express). diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 2d4c3b5..2303dbc 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -373,177 +373,6 @@ func (r *mdRenderer) renderListItem(li *snode, cont string) string { return item } -// renderTable renders a table as a GFM pipe table. Alignment is not preserved. -func (r *mdRenderer) renderTable(n *snode) string { - var rows []*snode - var header *snode - var walk func(*snode) - walk = func(x *snode) { - for _, k := range x.kids { - switch k.name { - case "thead", "tbody": - walk(k) - case "tr": - if header == nil && len(rows) == 0 && rowHasHeaderCell(k) { - header = k - } else { - rows = append(rows, k) - } - } - } - } - walk(n) - if header == nil { - if len(rows) == 0 { - return "" - } - header, rows = rows[0], rows[1:] - } - - head := r.cellTexts(header) - var b strings.Builder - b.WriteString("| " + strings.Join(head, " | ") + " |\n") - b.WriteString("| " + strings.Join(columnSeparators(header, rows, len(head)), " | ") + " |") - for _, row := range rows { - b.WriteString("\n| " + strings.Join(r.cellTexts(row), " | ") + " |") - } - return b.String() -} - -// alignSeparators are the GFM delimiter cells, keyed by the alignment recovered -// from storage. -var alignSeparators = map[string]string{ - "left": ":---", - "center": ":---:", - "right": "---:", -} - -// textAlignRE pulls the value out of a text-align declaration anywhere in a style -// attribute. -var textAlignRE = regexp.MustCompile(`(?i)text-align\s*:\s*([a-z]+)`) - -// whitespaceRunRE collapses a run of whitespace to a single space in collapse -// below. Not the same concern as linkindex's identically-shaped regexp (that -// one collapses a run to a hyphen, for a Confluence anchor slug); duplicated -// rather than imported, since sharing it would couple this file's general -// text-collapsing to an unrelated package over one regexp literal. -var whitespaceRunRE = regexp.MustCompile(`\s+`) - -// columnSeparators builds the delimiter row, recovering each column's alignment -// from its cells. -// -// Confluence aligns a paragraph while GFM aligns a column, so a column whose -// cells disagree cannot be represented: the most common alignment wins and the -// rest are dropped. Cells that declare nothing do not vote -- a single centered -// cell in a column of plain ones still centers the column, which is the only -// reading that survives the round trip at all. -func columnSeparators(header *snode, rows []*snode, cols int) []string { - counts := make([]map[string]int, cols) - order := make([]map[string]int, cols) - seen := 0 - for _, tr := range append([]*snode{header}, rows...) { - if tr == nil { - continue - } - i := 0 - for _, c := range tr.kids { - if c.name != "th" && c.name != "td" { - continue - } - if i >= cols { - break - } - if a := cellAlignment(c); a != "" { - if counts[i] == nil { - counts[i], order[i] = map[string]int{}, map[string]int{} - } - counts[i][a]++ - if _, ok := order[i][a]; !ok { - seen++ - order[i][a] = seen - } - } - i++ - } - } - - seps := make([]string, cols) - for i := range seps { - seps[i] = "---" - best := "" - for a, n := range counts[i] { - // Ties go to whichever alignment appeared first, so the delimiter row - // does not depend on map iteration order. - if best == "" || n > counts[i][best] || (n == counts[i][best] && order[i][a] < order[i][best]) { - best = a - } - } - if sep, ok := alignSeparators[best]; ok { - seps[i] = sep - } - } - return seps -} - -// cellAlignment reports the alignment stored on one cell, normalized to the GFM -// vocabulary. Confluence's own form is a text-align on a paragraph inside the -// cell, which is what markfluence writes; the cell-level form works too and is -// read for the same reason. "start"/"end" are ADF's names for left/right and turn -// up in hand-edited storage. -func cellAlignment(c *snode) string { - styles := []string{c.attrs["style"]} - for _, k := range c.kids { - if k.name == "p" { - styles = append(styles, k.attrs["style"]) - } - } - for _, s := range styles { - m := textAlignRE.FindStringSubmatch(s) - if m == nil { - continue - } - switch strings.ToLower(m[1]) { - case "left", "start": - return "left" - case "center": - return "center" - case "right", "end": - return "right" - } - } - return "" -} - -// rowHasHeaderCell reports whether a
. -func rowHasHeaderCell(tr *snode) bool { - for _, c := range tr.kids { - if c.name == "th" { - return true - } - } - return false -} - -// cellTexts renders a row's cells to inline strings with pipes escaped, -// prefixed with a bg: marker for a cell carrying a background color. -func (r *mdRenderer) cellTexts(tr *snode) []string { - var cells []string - for _, c := range tr.kids { - if c.name == "th" || c.name == "td" { - text := r.renderCellLines(c) - if marker := cellBGMarkerComment(c); marker != "" { - if text == "" { - text = marker - } else { - text = marker + " " + text - } - } - cells = append(cells, text) - } - } - return cells -} - // renderCellLines renders a table cell's content as a single physical line. // Confluence writes one

per line when a cell holds more than one -- // hitting Enter inside a cell in the editor starts a new

, not a
-- and @@ -959,6 +788,13 @@ func imageTitle(attrs map[string]string) string { // --- helpers ----------------------------------------------------------------- +// whitespaceRunRE collapses a run of whitespace to a single space in collapse +// below. Not the same concern as linkindex's identically-shaped regexp (that +// one collapses a run to a hyphen, for a Confluence anchor slug); duplicated +// rather than imported, since sharing it would couple this file's general +// text-collapsing to an unrelated package over one regexp literal. +var whitespaceRunRE = regexp.MustCompile(`\s+`) + // collapse replaces every run of whitespace with a single space (reusing the // package's whitespace regexp). func collapse(s string) string { @@ -1071,7 +907,11 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { // Content container: raw tags around a Markdown body. if isContentContainer(n.name) { - if md := strings.Join(r.blockStrings(n.kids, ""), "\n\n"); md != "" { + blocks := r.blockStrings(n.kids, "") + if n.name == "th" || n.name == "td" { + blocks = r.rawCellBlocks(n) + } + if md := strings.Join(blocks, "\n\n"); md != "" { return open + "\n\n" + md + "\n\n" + closeTag } return open + closeTag @@ -1098,10 +938,12 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { // isContentContainer reports whether an element holds block content that should // be converted to Markdown rather than serialized raw, so a passed-through -// wrapper keeps an editable body. +// wrapper keeps an editable body. A table cell is one because a table only +// reaches renderRawBlock when Markdown cannot express it (renderTable), and its +// cells' text should stay as editable as a layout cell's. func isContentContainer(name string) bool { switch name { - case "ac:rich-text-body", "ac:layout-cell", "ac:adf-content": + case "ac:rich-text-body", "ac:layout-cell", "ac:adf-content", "th", "td": return true } return false diff --git a/internal/convert/storage_to_md_table.go b/internal/convert/storage_to_md_table.go new file mode 100644 index 0000000..2162295 --- /dev/null +++ b/internal/convert/storage_to_md_table.go @@ -0,0 +1,342 @@ +package convert + +import ( + "regexp" + "strings" +) + +// A table reads back as a GFM pipe table only when Markdown expresses all of +// it, and as raw storage otherwise (#55, _plans/055). The pipe table used to be +// unconditional, so a page's layout, column widths, merged cells or a +// headerless shape were dropped on read, and the next publish stripped them +// from the page. A table that is nearly expressible is raw too: degrading one +// cell is still loss, and the raw form is exact. +// +// What counts is an allowlist, so an attribute nobody has seen yet keeps the +// table raw rather than being dropped. A few attributes are ignored because the +// editor writes them on tables nobody configured (page 2913502220, measured +// 2026-09-25): honouring them would send every table the editor has saved back +// as HTML. docs/confluence/storage-format.md has what each attribute does. + +// renderTable renders a table as a GFM pipe table when tableShape says Markdown +// can hold it, and as raw storage with Markdown cell bodies otherwise. +func (r *mdRenderer) renderTable(n *snode) string { + shape, ok := tableShape(n) + if !ok { + return r.renderRawBlock(n) + } + if shape.header == nil { + return "" + } + head := r.cellTexts(shape.header) + seps := make([]string, len(shape.aligns)) + for i, a := range shape.aligns { + seps[i] = alignSeparators[a] + } + var b strings.Builder + b.WriteString("| " + strings.Join(head, " | ") + " |\n") + b.WriteString("| " + strings.Join(seps, " | ") + " |") + for _, row := range shape.rows { + b.WriteString("\n| " + strings.Join(r.cellTexts(row), " | ") + " |") + } + return b.String() +} + +// alignSeparators are the GFM delimiter cells, keyed by the alignment recovered +// from storage. There is no left: Confluence cannot state one, so a left column +// and an unaligned one are the same column (storage-format.md). +var alignSeparators = map[string]string{ + "": "---", + "center": ":---:", + "right": "---:", +} + +// pipeTable is a table Markdown can hold: one header row of

, body rows of +// , every row as wide as the header, and one alignment per column. +type pipeTable struct { + header *snode + rows []*snode + aligns []string +} + +// tableShape reports whether a pipe table expresses n exactly, and its parts +// when it does. A table with no rows at all is expressible as nothing, which +// is what the pipe renderer has always written for one. +func tableShape(n *snode) (pipeTable, bool) { + if !allowedAttrs(n, tableAttrOK) { + return pipeTable{}, false + } + var trs []*snode + for _, k := range n.kids { + switch k.name { + case "": + if strings.TrimSpace(k.text) != "" { + return pipeTable{}, false + } + case "thead", "tbody": + if !allowedAttrs(k, idOnly) { + return pipeTable{}, false + } + for _, tr := range k.kids { + switch { + case tr.name == "tr": + trs = append(trs, tr) + case tr.name != "" || strings.TrimSpace(tr.text) != "": + return pipeTable{}, false + } + } + case "tr": + trs = append(trs, k) + default: + // A
and no other row may hold + // one, or the next publish turns s into s (or back). + want := "td" + if i == 0 { + want = "th" + t.header = tr + t.aligns = make([]string, len(cells)) + } else { + t.rows = append(t.rows, tr) + } + if len(cells) != len(t.aligns) { + return pipeTable{}, false + } + for col, c := range cells { + if c.name != want || !cellExpressible(c) { + return pipeTable{}, false + } + a, ok := cellAlign(c) + if !ok { + return pipeTable{}, false + } + // GFM aligns a column, Confluence a paragraph, so a column whose + // cells disagree cannot be written without republishing some of + // them with an alignment they did not have. + if i == 0 { + t.aligns[col] = a + } else if a != t.aligns[col] { + return pipeTable{}, false + } + } + } + return t, true +} + +// rowCells returns a row's cells, or false when the row carries an attribute +// or a child a pipe table cannot hold. +func rowCells(tr *snode) ([]*snode, bool) { + if !allowedAttrs(tr, idOnly) { + return nil, false + } + var cells []*snode + for _, c := range tr.kids { + switch c.name { + case "th", "td": + cells = append(cells, c) + case "": + if strings.TrimSpace(c.text) != "" { + return nil, false + } + default: + return nil, false + } + } + return cells, true +} + +// cellExpressible reports whether a cell's attributes and children fit in a +// pipe table cell: a colour, an alignment, and content that is paragraphs, +// lists or inline markup. A heading, a code block or any other block macro, a +// nested table and the rest need more than one physical line. +func cellExpressible(c *snode) bool { + if !allowedAttrs(c, cellAttrOK) { + return false + } + for _, k := range c.kids { + switch { + case k.name == "": + case k.name == "p": + if !allowedAttrs(k, paragraphAttrOK) { + return false + } + case k.name == "ul", k.name == "ol", cellInline[k.name]: + default: + return false + } + } + return true +} + +// cellInline are the elements that may sit directly in a cell, outside any +// paragraph, and still render as the cell's inline text. A macro is not among +// them: directly in a cell it is usually a block one. +var cellInline = map[string]bool{ + "ac:image": true, "ac:link": true, "a": true, "strong": true, "b": true, "em": true, + "i": true, "code": true, "del": true, "s": true, "strike": true, "br": true, + "span": true, "u": true, "sub": true, "sup": true, "ac:emoticon": true, "time": true, +} + +// allowedAttrs reports whether ok accepts every attribute of n. +func allowedAttrs(n *snode, ok func(name, value string) bool) bool { + for k, v := range n.attrs { + if !ok(k, v) { + return false + } + } + return true +} + +// idOnly accepts only the server-generated id, which read drops everywhere. +func idOnly(name, _ string) bool { return name == "ac:local-id" } + +// tableAttrOK accepts what the editor writes on a table nobody configured. +// data-table-width is ignored whatever its value: the editor writes one on +// every table it saves, and a hand resize is lost with it on the next publish. +// Any other layout or display mode is a choice somebody made. +func tableAttrOK(name, value string) bool { + switch name { + case "ac:local-id", "data-table-width": + return true + case "data-layout": + return value == "align-start" + case "data-table-display-mode": + return value == "default" + } + return false +} + +// cellAttrOK accepts a cell's id, its colour (a bg: marker), a span of one, +// and a style that says nothing but its alignment. +func cellAttrOK(name, value string) bool { + switch name { + case "ac:local-id", "data-highlight-colour": + return true + case "rowspan", "colspan": + return strings.TrimSpace(value) == "1" + case "style": + return onlyTextAlign(value) + } + return false +} + +// paragraphAttrOK accepts a cell paragraph's id and a style that says nothing +// but its alignment. +func paragraphAttrOK(name, value string) bool { + switch name { + case "ac:local-id": + return true + case "style": + return onlyTextAlign(value) + } + return false +} + +// textAlignDeclRE matches one text-align declaration in a style attribute. +var textAlignDeclRE = regexp.MustCompile(`(?i)text-align\s*:\s*[a-z-]+\s*;?`) + +// onlyTextAlign reports whether a style attribute holds no declaration but +// text-align, the one a pipe table carries (as its delimiter row). +func onlyTextAlign(style string) bool { + return strings.Trim(textAlignDeclRE.ReplaceAllString(style, ""), " \t\n;") == "" +} + +// textAlignRE pulls the value out of a text-align declaration anywhere in a style +// attribute. +var textAlignRE = regexp.MustCompile(`(?i)text-align\s*:\s*([a-z]+)`) + +// cellAlign reports the one alignment a cell's content carries, normalized to +// the delimiter row's vocabulary, or false when its paragraphs disagree -- a +// cell with one centred line and one plain one has no column alignment that +// republishes it unchanged. +// +// Confluence's own form is a text-align on each paragraph, which is what +// markfluence writes; a text-align on the cell works too and covers every +// paragraph in it. "start"/"end" are ADF's names for left/right and turn up in +// hand-edited storage, and left is no alignment at all, since Confluence has +// no explicit left (storage-format.md). +func cellAlign(c *snode) (string, bool) { + cell := styleAlign(c.attrs["style"]) + found, seen := cell, false + for _, k := range c.kids { + if k.name != "p" { + continue + } + a := styleAlign(k.attrs["style"]) + if a == "" { + a = cell + } + if seen && a != found { + return "", false + } + found, seen = a, true + } + return found, true +} + +// styleAlign reads a text-align declaration from a style attribute. +func styleAlign(style string) string { + m := textAlignRE.FindStringSubmatch(style) + if m == nil { + return "" + } + switch strings.ToLower(m[1]) { + case "center": + return "center" + case "right", "end": + return "right" + } + return "" +} + +// rawCellBlocks renders a raw table cell's body as Markdown blocks, except a +// paragraph carrying an attribute Markdown cannot hold -- in practice an +// alignment -- which stays storage on its own line, since a Markdown paragraph +// would drop it. That costs the paragraph's editability, not its alignment. +func (r *mdRenderer) rawCellBlocks(c *snode) []string { + var out []string + for _, k := range c.kids { + if k.name == "p" && !allowedAttrs(k, idOnly) { + out = append(out, serialize(k)) + continue + } + out = append(out, r.blockStrings([]*snode{k}, "")...) + } + return out +} + +// cellTexts renders a row's cells to inline strings with pipes escaped, +// prefixed with a bg: marker for a cell carrying a background color. +func (r *mdRenderer) cellTexts(tr *snode) []string { + var cells []string + for _, c := range tr.kids { + if c.name == "th" || c.name == "td" { + text := r.renderCellLines(c) + if marker := cellBGMarkerComment(c); marker != "" { + if text == "" { + text = marker + } else { + text = marker + " " + text + } + } + cells = append(cells, text) + } + } + return cells +} diff --git a/internal/convert/storage_to_md_test.go b/internal/convert/storage_to_md_test.go index ab6e956..2c6bdd4 100644 --- a/internal/convert/storage_to_md_test.go +++ b/internal/convert/storage_to_md_test.go @@ -213,7 +213,9 @@ func TestRoundTripTableCellBG(t *testing.T) { // renders as the two-trailing-spaces hard break valid in ordinary block // content but not inside a single table row. func TestStorageToMarkdownJoinsMultilineCells(t *testing.T) { - in := `` + + // The header row keeps the table a pipe table: with no header row it would + // read back raw (#55), which is not what this is about. + in := `
` + `` + `` + `` + @@ -221,8 +223,8 @@ func TestStorageToMarkdownJoinsMultilineCells(t *testing.T) { // the absence of one, and must survive rather than being silently dropped. `` + `
abcd

line one

line two

mid-line
break

plain, no wrapper issue

line one

line three

` - want := "| line one
line two | mid-line
break | plain, no wrapper issue" + - " | line one

line three |\n| --- | --- | --- | --- |\n" + want := "| a | b | c | d |\n| --- | --- | --- | --- |\n" + + "| line one
line two | mid-line
break | plain, no wrapper issue | line one

line three |\n" got, err := convert.StorageToMarkdown(in, convert.StorageOptions{}) if err != nil { @@ -271,14 +273,14 @@ func TestStorageToMarkdownPassesThroughListsInCells(t *testing.T) { // would corrupt the href with a backslash that has no meaning inside a // quoted attribute -- a URL, unlike table text, has no escaping syntax at // all, so this must come back byte-identical. - in := `` + + in := `
` + `` + `` + `` + `
abc
  • one
  • two
  1. a

  2. b

` - want := "|
  • one
  • two
|
  1. a

  2. b

|" + - ` |` + "\n" + - "| --- | --- | --- |\n" + want := "| a | b | c |\n| --- | --- | --- |\n" + + "|
  • one
  • two
|
  1. a

  2. b

|" + + ` |` + "\n" got, err := convert.StorageToMarkdown(in, convert.StorageOptions{}) if err != nil { @@ -378,7 +380,15 @@ func TestStorageToMarkdownCoalescesSplitMarks(t *testing.T) { // are passthrough: a case whose output is ordinary Markdown is covered by its // own golden. func TestRoundTripPassthrough(t *testing.T) { - for _, name := range []string{"layout", "unknown-macros", "excerpt", "aclink", "adf-panel"} { + for _, name := range []string{ + "layout", "unknown-macros", "excerpt", "aclink", "adf-panel", + // Every table that reads back raw (#55): the raw form must publish to + // the same table, or read -> edit -> update would still strip it. + "raw-table-layout", "raw-table-colgroup", "raw-table-spans", "raw-table-no-header", + "raw-table-header-column", "raw-table-short-row", "raw-table-block-content", + "raw-table-nested", "raw-table-numbered", "raw-table-valign", "raw-table-display-fixed", + "raw-table-aligned-paragraph", "table-alignment-disagree", + } { t.Run(name, func(t *testing.T) { src, err := os.ReadFile(filepath.Join(storage2mdDir, name, "output.md")) if err != nil { diff --git a/internal/convert/testdata/regression/raw-storage/main.md b/internal/convert/testdata/regression/raw-storage/main.md index 396e3b9..eb0157e 100644 --- a/internal/convert/testdata/regression/raw-storage/main.md +++ b/internal/convert/testdata/regression/raw-storage/main.md @@ -27,6 +27,38 @@ Right column with a list: +A raw table passes through with every attribute, including a blank line +inside it. A cell body set off by blank lines is Markdown; one tight against +its tags stays literal: + + ++++ + + + + + + + + + + + + + +
+ +Q3 **results** + +
+ +**auth** is [up](https://example.net) + +

tight **not markdown**

99.8%
+ Storage format inside a code fence stays literal: ``` diff --git a/internal/convert/testdata/regression/raw-storage/test.output b/internal/convert/testdata/regression/raw-storage/test.output index fabeb6f..c58cd39 100644 --- a/internal/convert/testdata/regression/raw-storage/test.output +++ b/internal/convert/testdata/regression/raw-storage/test.output @@ -1,7 +1,7 @@ { "attachments": [], "broken": [], - "html": "

Raw Confluence Storage

\n

A pasted status macro passes straight through:

\n\nGreen\nDone\n\n

A layout whose cells contain markdown that still gets converted:

\n\n\n\n

Left column with bold and a link.

\n
\n\n

Right column with a list:

\n
    \n
  • one
  • \n
  • two
  • \n
\n
\n
\n
\n

Storage format inside a code fence stays literal:

\n]]>", + "html": "

Raw Confluence Storage

\n

A pasted status macro passes straight through:

\n\nGreen\nDone\n\n

A layout whose cells contain markdown that still gets converted:

\n\n\n\n

Left column with bold and a link.

\n
\n\n

Right column with a list:

\n
    \n
  • one
  • \n
  • two
  • \n
\n
\n
\n
\n

A raw table passes through with every attribute, including a blank line inside it. A cell body set off by blank lines is Markdown; one tight against its tags stays literal:

\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
\n

Q3 results

\n
\n

auth is up

\n

tight **not markdown**

99.8%
\n

Storage format inside a code fence stays literal:

\n]]>", "warnings": [], "mentions": [] } diff --git a/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/input.storage b/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/input.storage new file mode 100644 index 0000000..14dfe94 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/input.storage @@ -0,0 +1,6 @@ + + + + + +

Note

First line

centred line

diff --git a/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/output.md b/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/output.md new file mode 100644 index 0000000..3834adf --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-aligned-paragraph/output.md @@ -0,0 +1,20 @@ + + + + + + + + + +
+ +Note + +
+ +*First* line + +

centred line

+ +
diff --git a/internal/convert/testdata/storage2md/raw-table-block-content/input.storage b/internal/convert/testdata/storage2md/raw-table-block-content/input.storage new file mode 100644 index 0000000..4ee140d --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-block-content/input.storage @@ -0,0 +1,6 @@ + + + + + +

Step

How

Restart

Restart with:

bash
diff --git a/internal/convert/testdata/storage2md/raw-table-block-content/output.md b/internal/convert/testdata/storage2md/raw-table-block-content/output.md new file mode 100644 index 0000000..c683699 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-block-content/output.md @@ -0,0 +1,32 @@ + + + + + + + + + + + +
+ +Step + + + +How + +
+ +### Restart + + + +Restart with: + +```bash +systemctl restart auth +``` + +
diff --git a/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage b/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage new file mode 100644 index 0000000..e649a52 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage @@ -0,0 +1,7 @@ + + + + + + +

Key

Value

a

b

diff --git a/internal/convert/testdata/storage2md/raw-table-colgroup/output.md b/internal/convert/testdata/storage2md/raw-table-colgroup/output.md new file mode 100644 index 0000000..61cf0ea --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-colgroup/output.md @@ -0,0 +1,32 @@ + ++++ + + + + + + + + + + +
+ +Key + + + +Value + +
+ +a + + + +b + +
diff --git a/internal/convert/testdata/storage2md/raw-table-display-fixed/input.storage b/internal/convert/testdata/storage2md/raw-table-display-fixed/input.storage new file mode 100644 index 0000000..d3a0214 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-display-fixed/input.storage @@ -0,0 +1,6 @@ + + + + + +

A

a

diff --git a/internal/convert/testdata/storage2md/raw-table-display-fixed/output.md b/internal/convert/testdata/storage2md/raw-table-display-fixed/output.md new file mode 100644 index 0000000..1188180 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-display-fixed/output.md @@ -0,0 +1,18 @@ + + + + + + + + + +
+ +A + +
+ +a + +
diff --git a/internal/convert/testdata/storage2md/raw-table-header-column/input.storage b/internal/convert/testdata/storage2md/raw-table-header-column/input.storage new file mode 100644 index 0000000..5e79427 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-header-column/input.storage @@ -0,0 +1,6 @@ + + + + + +

Owner

SRE

Pager

auth-oncall

diff --git a/internal/convert/testdata/storage2md/raw-table-header-column/output.md b/internal/convert/testdata/storage2md/raw-table-header-column/output.md new file mode 100644 index 0000000..cfe070d --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-header-column/output.md @@ -0,0 +1,28 @@ + + + + + + + + + + + +
+ +Owner + + + +SRE + +
+ +Pager + + + +auth-oncall + +
diff --git a/internal/convert/testdata/storage2md/raw-table-layout/input.storage b/internal/convert/testdata/storage2md/raw-table-layout/input.storage new file mode 100644 index 0000000..c934c67 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-layout/input.storage @@ -0,0 +1,7 @@ + + + + + + +

Service

Owner

auth

SRE runbooks

diff --git a/internal/convert/testdata/storage2md/raw-table-layout/output.md b/internal/convert/testdata/storage2md/raw-table-layout/output.md new file mode 100644 index 0000000..ba36c42 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-layout/output.md @@ -0,0 +1,32 @@ + ++++ + + + + + + + + + + +
+ +Service + + + +Owner + +
+ +auth + + + +[SRE runbooks](https://example.com/runbooks) + +
diff --git a/internal/convert/testdata/storage2md/raw-table-nested/input.storage b/internal/convert/testdata/storage2md/raw-table-nested/input.storage new file mode 100644 index 0000000..c2f2308 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-nested/input.storage @@ -0,0 +1,6 @@ + + + + + +

Outer

Inner

x

diff --git a/internal/convert/testdata/storage2md/raw-table-nested/output.md b/internal/convert/testdata/storage2md/raw-table-nested/output.md new file mode 100644 index 0000000..49b62cc --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-nested/output.md @@ -0,0 +1,20 @@ + + + + + + + + + +
+ +Outer + +
+ +| Inner | +| --- | +| x | + +
diff --git a/internal/convert/testdata/storage2md/raw-table-no-header/input.storage b/internal/convert/testdata/storage2md/raw-table-no-header/input.storage new file mode 100644 index 0000000..71d14fc --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-no-header/input.storage @@ -0,0 +1,6 @@ + + + + + +

Key

Value

a

b

diff --git a/internal/convert/testdata/storage2md/raw-table-no-header/output.md b/internal/convert/testdata/storage2md/raw-table-no-header/output.md new file mode 100644 index 0000000..7607d5a --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-no-header/output.md @@ -0,0 +1,28 @@ + + + + + + + + + + + +
+ +Key + + + +Value + +
+ +a + + + +b + +
diff --git a/internal/convert/testdata/storage2md/raw-table-numbered/input.storage b/internal/convert/testdata/storage2md/raw-table-numbered/input.storage new file mode 100644 index 0000000..8832c6f --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-numbered/input.storage @@ -0,0 +1,6 @@ + + + + + +

Task

1

Write it

diff --git a/internal/convert/testdata/storage2md/raw-table-numbered/output.md b/internal/convert/testdata/storage2md/raw-table-numbered/output.md new file mode 100644 index 0000000..100dfb8 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-numbered/output.md @@ -0,0 +1,24 @@ + + + + + + + + + + + +
+ +Task + +
+ +1 + + + +Write it + +
diff --git a/internal/convert/testdata/storage2md/raw-table-short-row/input.storage b/internal/convert/testdata/storage2md/raw-table-short-row/input.storage new file mode 100644 index 0000000..544a24b --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-short-row/input.storage @@ -0,0 +1,7 @@ + + + + + + +

A

B

a

b

x

diff --git a/internal/convert/testdata/storage2md/raw-table-short-row/output.md b/internal/convert/testdata/storage2md/raw-table-short-row/output.md new file mode 100644 index 0000000..6a39a1c --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-short-row/output.md @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + +
+ +A + + + +B + +
+ +a + + + +b + +
+ +x + +
diff --git a/internal/convert/testdata/storage2md/raw-table-spans/input.storage b/internal/convert/testdata/storage2md/raw-table-spans/input.storage new file mode 100644 index 0000000..7b28b5f --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-spans/input.storage @@ -0,0 +1,7 @@ + + + + + + +

Q3 results

auth

99.9%

99.8%

diff --git a/internal/convert/testdata/storage2md/raw-table-spans/output.md b/internal/convert/testdata/storage2md/raw-table-spans/output.md new file mode 100644 index 0000000..dc3feac --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-spans/output.md @@ -0,0 +1,30 @@ + + + + + + + + + + + + + +
+ +Q3 results + +
+ +**auth** + + + +99.9% + +
+ +99.8% + +
diff --git a/internal/convert/testdata/storage2md/raw-table-valign/input.storage b/internal/convert/testdata/storage2md/raw-table-valign/input.storage new file mode 100644 index 0000000..0ec89ff --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-valign/input.storage @@ -0,0 +1,6 @@ + + + + + +

A

B

low

line one

line two

diff --git a/internal/convert/testdata/storage2md/raw-table-valign/output.md b/internal/convert/testdata/storage2md/raw-table-valign/output.md new file mode 100644 index 0000000..9ae7495 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-valign/output.md @@ -0,0 +1,30 @@ + + + + + + + + + + + +
+ +A + + + +B + +
+ +low + + + +line one + +line two + +
diff --git a/internal/convert/testdata/storage2md/table-alignment-disagree/input.storage b/internal/convert/testdata/storage2md/table-alignment-disagree/input.storage new file mode 100644 index 0000000..b9b5b41 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-alignment-disagree/input.storage @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + +

Service

Errors

auth

3

billing

none reported

diff --git a/internal/convert/testdata/storage2md/table-alignment-disagree/output.md b/internal/convert/testdata/storage2md/table-alignment-disagree/output.md new file mode 100644 index 0000000..9f63d53 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-alignment-disagree/output.md @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + +
+ +Service + + + +

Errors

+ +
+ +auth + + + +

3

+ +
+ +billing + + + +none reported + +
diff --git a/internal/convert/testdata/storage2md/table-alignment/input.storage b/internal/convert/testdata/storage2md/table-alignment/input.storage index fa33758..4b2ec1c 100644 --- a/internal/convert/testdata/storage2md/table-alignment/input.storage +++ b/internal/convert/testdata/storage2md/table-alignment/input.storage @@ -5,6 +5,7 @@

Paragraph form

Cell form

ADF names

No left

a

b

c

d

d

e

e

f

g

h

f

g

h

i

j
diff --git a/internal/convert/testdata/storage2md/table-alignment/output.md b/internal/convert/testdata/storage2md/table-alignment/output.md index 170b3bc..dfeea13 100644 --- a/internal/convert/testdata/storage2md/table-alignment/output.md +++ b/internal/convert/testdata/storage2md/table-alignment/output.md @@ -1,4 +1,4 @@ -| Plain | Paragraph form | Cell form | ADF names | -| --- | :---: | ---: | ---: | -| a | b | c | d | -| e | f | g | h | +| Plain | Paragraph form | Cell form | ADF names | No left | +| --- | :---: | ---: | ---: | --- | +| a | b | c | d | e | +| f | g | h | i | j | diff --git a/internal/convert/testdata/storage2md/table-editor/input.storage b/internal/convert/testdata/storage2md/table-editor/input.storage new file mode 100644 index 0000000..c407eab --- /dev/null +++ b/internal/convert/testdata/storage2md/table-editor/input.storage @@ -0,0 +1,12 @@ + + + + + + + + + + + +

Service

Errors

auth

3

diff --git a/internal/convert/testdata/storage2md/table-editor/output.md b/internal/convert/testdata/storage2md/table-editor/output.md new file mode 100644 index 0000000..27bddb9 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-editor/output.md @@ -0,0 +1,3 @@ +| Service | Errors | +| --- | ---: | +| auth | 3 | From 2707b86299a2d13f4e4806dc1e68971faf19b8be Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 25 Sep 2026 15:06:03 -0400 Subject: [PATCH 3/8] fix: address the raw-tables-round-trip review A markfluence table saved in the browser editor read back raw: the editor puts a bare local-id on every paragraph and a
` 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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`/``, a span of 1, cell children that are paragraphs, lists or inline markup, and every cell in a column agreeing on alignment (no alignment, `left` and `start` are one value) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Four attributes are **ignored** rather than honoured because the editor writes them on every table it saves (measured on a real page, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id`, `data-table-width` whatever its value (the known cost: a hand resize is lost when a read-back table republishes), `data-table-display-mode="default"`, and `data-layout="align-start"`; honouring them would send every editor-saved table back as HTML. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

`/`` as content containers so each cell's body stays Markdown between blank lines (an aligned `

` in a raw cell stays storage, since a Markdown paragraph has no alignment). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
` 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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`, a span of 1, cell children that are paragraphs, lists or inline markup, a `text-align` only of a known value, and every non-empty cell in a column agreeing on alignment (no alignment, `left`, `start` and `justify` are one value, and a paragraph's own declaration beats its cell's) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Some markup is **ignored** rather than honoured because the browser editor writes it on every table it saves (measured on a markfluence table saved in the browser, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id` and the bare `local-id` it puts on every paragraph, `data-table-width` whatever its value, `data-table-display-mode="default"`, `data-layout="align-start"`, and — on an `align-start` table only — a `` of pixel widths, which the editor measures and writes on any save. Honouring them would turn every markfluence table someone edits in Confluence into HTML on the next `read`. The known cost: a hand resize writes the same `data-table-width` and ``, so it is lost when a read-back table republishes. A `` on any other layout keeps the table raw, since `default`/`full-width` are visible layouts markfluence never writes. In a raw cell, `rawCellBlocks` groups loose inline content into one paragraph, keeps an empty paragraph as `

`, and strips the bare `local-id` from a paragraph it has to serialize; `renderRawBlock` keeps loose text in a wrapper as a line rather than dropping it as whitespace. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

` of measured pixel widths summing to `data-table-width`, so it + read back raw. Both are now ignored: `local-id` everywhere, the + `` only on an `align-start` table. A resize writes the same shape + (only its sum stops matching `data-table-width`, seen once), so a resized + column is lost on the next publish -- chosen over trusting the sum. + `data-layout="default"` and `full-width`, common on tables markfluence did + not write (a 261-table sample), stay reasons to go raw: they are visible + layouts. +- **From the code review, each reproduced first:** loose inline content in a + raw cell is one paragraph, not a block per element; an empty paragraph in + a raw cell stays `

`; loose text in a table wrapper is kept; a + `text-align` of an unknown value keeps a table raw (`justify` has no effect + and is accepted); a paragraph's own alignment beats its cell's; an empty + cell does not vote on its column's alignment; the tags allowed loose in a + pipe cell are only those `read` can render. Inline tags `read` cannot + render inside a paragraph (`

`s) still keeps a table raw: in the sample, ignoring it would + change 3 of 109 old tables. diff --git a/docs/confluence/storage-format.md b/docs/confluence/storage-format.md index da73fde..291d91c 100644 --- a/docs/confluence/storage-format.md +++ b/docs/confluence/storage-format.md @@ -238,12 +238,34 @@ every table carries `data-table-width` (1110 on two, 778 on the third) and `ac:local-id`, every row and cell carries `ac:local-id`, and one table carries `data-table-display-mode="default"`. None has a ``. +**Verified 2026-09-25** on page 3109814418, a table `create` published and +then saved in the browser editor. An edit elsewhere on the page, leaving the +table alone, added `ac:local-id` to the table, rows and cells, a bare +`local-id` to every paragraph, `data-table-width="229"`, and a `` of +pixel widths (72, 97, 60) that sum to it -- widths the editor measured, on the +unchanged `data-layout="align-start"`. Colours and alignments were untouched. +Dragging one column border then changed the `` widths (72, 154, 48) and +left `data-table-width` at 229. A save through the API instead (the page's ADF +`PUT` back) added none of this. + So those attributes say nothing about what an author chose. `read` ignores them when deciding whether a table can be a GFM table (#55, -`internal/convert/storage_to_md_table.go`), along with -`data-layout="align-start"` and a span of 1. `data-table-width` is the costly -one: it is also what a hand resize records, and a resized table that reads -back as GFM loses the resize on its next publish. +`internal/convert/storage_to_md_table.go`), along with a span of 1 -- and a +`` of pixel widths only on an `align-start` table, which is what +keeps a markfluence table a GFM table after an editor save. Two are costly: +`data-table-width` and that `` are also what a hand resize records, +so a resized table that reads back as GFM loses the resize on its next +publish. The only sign of a resize is a `` sum that no longer matches +`data-table-width`, seen once and not relied on. + +**Tables markfluence did not write are mostly different.** Of 152 tables on +50 pages edited since June 2026 (a CQL sample, 2026-09-25), 83 carry +`data-layout="default"`, 27 `full-width`, and 35 a ``; 27 read back +as GFM. Of 109 tables on 50 pages last edited before 2019, 6 do, and the rest +mostly for structure GFM cannot hold -- no header row (48), merged cells (33), +block content in a cell. `read` keeps a layout other than `align-start` as a +reason to write a table raw: it is visible, and republishing a GFM table would +replace it with `align-start`. ## Cell background colors diff --git a/docs/markdown-file.md b/docs/markdown-file.md index 74d74b8..02db5f4 100644 --- a/docs/markdown-file.md +++ b/docs/markdown-file.md @@ -419,11 +419,18 @@ Details: `read` and `export` give back a GFM table when GFM can express the whole table. Otherwise, they give back a raw table in the form above, with the content of each cell as Markdown. A paragraph with an alignment stays storage -format, because a Markdown paragraph has no alignment. `read` ignores some -attributes that the Confluence editor adds to every table that it saves, such -as `data-table-width`. Thus a table with a width that someone changed in the -editor comes back as a GFM table, and it loses that width when you publish it -again. +format, because a Markdown paragraph has no alignment. + +A table that markfluence published stays a GFM table after someone edits the +page in Confluence. The Confluence editor adds attributes to every table that +it saves: IDs, a `data-table-width`, and, on a table with the `align-start` +layout that markfluence uses, a `` with the column widths that it +measured. `read` ignores these. The editor writes the same `` when +someone changes a column width by hand. Thus that change is lost when you +publish the file again, and the columns fit their content again. (Column +widths that add up to more than the page give the table a horizontal scroll +bar, so this is often what you want.) A table with any other layout and a +`` comes back as a raw table. ### GitHub alerts diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 2303dbc..96bc2c5 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -907,9 +907,11 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { // Content container: raw tags around a Markdown body. if isContentContainer(n.name) { - blocks := r.blockStrings(n.kids, "") + var blocks []string if n.name == "th" || n.name == "td" { blocks = r.rawCellBlocks(n) + } else { + blocks = r.blockStrings(n.kids, "") } if md := strings.Join(blocks, "\n\n"); md != "" { return open + "\n\n" + md + "\n\n" + closeTag @@ -925,7 +927,13 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { var parts []string for _, k := range n.kids { if k.name == "" { - continue // drop inter-tag whitespace + // Whitespace between tags is layout; anything else is content a + // wrapper happens to hold loose, and passing it through is what + // the raw form is for. + if t := strings.TrimSpace(k.text); t != "" { + parts = append(parts, xmlTextEscape(t)) + } + continue } if isContentContainer(k.name) || hasElementChild(k) { parts = append(parts, r.renderRawBlock(k)) diff --git a/internal/convert/storage_to_md_table.go b/internal/convert/storage_to_md_table.go index 2162295..a9b7468 100644 --- a/internal/convert/storage_to_md_table.go +++ b/internal/convert/storage_to_md_table.go @@ -87,10 +87,14 @@ func tableShape(n *snode) (pipeTable, bool) { } case "tr": trs = append(trs, k) + case "colgroup": + if n.attrs["data-layout"] != "align-start" || !pixelColgroup(k) { + return pipeTable{}, false + } default: - // A is column widths, a a row Confluence - // renders as an ordinary one but that a pipe table would - // republish without the tag; anything else is unknown. + // A is a row Confluence renders as an ordinary one but + // that a pipe table would republish without the tag; anything + // else is unknown. return pipeTable{}, false } } @@ -99,6 +103,7 @@ func tableShape(n *snode) (pipeTable, bool) { } var t pipeTable + var voted []bool for i, tr := range trs { cells, ok := rowCells(tr) if !ok || len(cells) == 0 { @@ -112,6 +117,7 @@ func tableShape(n *snode) (pipeTable, bool) { want = "th" t.header = tr t.aligns = make([]string, len(cells)) + voted = make([]bool, len(cells)) } else { t.rows = append(t.rows, tr) } @@ -128,10 +134,14 @@ func tableShape(n *snode) (pipeTable, bool) { } // GFM aligns a column, Confluence a paragraph, so a column whose // cells disagree cannot be written without republishing some of - // them with an alignment they did not have. - if i == 0 { - t.aligns[col] = a - } else if a != t.aligns[col] { + // them with an alignment they did not have. An empty cell has + // nothing to align, so it does not vote: republished, it gains an + // empty aligned paragraph, which reads back the same. + switch { + case emptyCell(c): + case !voted[col]: + t.aligns[col], voted[col] = a, true + case a != t.aligns[col]: return pipeTable{}, false } } @@ -161,6 +171,60 @@ func rowCells(tr *snode) ([]*snode, bool) { return cells, true } +// pixelColgroup reports whether a holds nothing but pixel column +// widths, which is what the browser editor writes on an align-start table +// whenever it saves one -- the widths it measured, summing to the +// data-table-width it writes beside them (page 3109814418, 2026-09-25, a +// markfluence table saved without touching the table). A resize writes the +// same shape, so ignoring it loses a resized column's width on the next +// publish; nothing tells the two apart but a sum Atlassian does not document. +// On any other layout a keeps the table raw: a table in the +// default or full-width layout is not one markfluence wrote. +func pixelColgroup(g *snode) bool { + if !allowedAttrs(g, idOnly) { + return false + } + for _, c := range g.kids { + switch { + case c.name == "col": + if len(c.kids) > 0 || !allowedAttrs(c, pixelWidth) { + return false + } + case c.name != "" || strings.TrimSpace(c.text) != "": + return false + } + } + return true +} + +// pixelWidthRE matches the style the editor writes on a . +var pixelWidthRE = regexp.MustCompile(`^\s*width:\s*[0-9]+(\.[0-9]+)?px;?\s*$`) + +// pixelWidth accepts a 's id and a pixel width. +func pixelWidth(name, value string) bool { + return isID(name) || (name == "style" && pixelWidthRE.MatchString(value)) +} + +// emptyCell reports whether a cell holds no content: no text, and no element +// but empty paragraphs. +func emptyCell(c *snode) bool { + for _, k := range c.kids { + switch k.name { + case "": + if strings.TrimSpace(k.text) != "" { + return false + } + case "p": + if !emptyCell(k) { + return false + } + default: + return false + } + } + return true +} + // cellExpressible reports whether a cell's attributes and children fit in a // pipe table cell: a colour, an alignment, and content that is paragraphs, // lists or inline markup. A heading, a code block or any other block macro, a @@ -185,12 +249,12 @@ func cellExpressible(c *snode) bool { } // cellInline are the elements that may sit directly in a cell, outside any -// paragraph, and still render as the cell's inline text. A macro is not among -// them: directly in a cell it is usually a block one. +// paragraph, and still render as the cell's inline text: the ones renderInline +// has a Markdown form for. A macro is not among them: directly in a cell it is +// usually a block one. var cellInline = map[string]bool{ "ac:image": true, "ac:link": true, "a": true, "strong": true, "b": true, "em": true, "i": true, "code": true, "del": true, "s": true, "strike": true, "br": true, - "span": true, "u": true, "sub": true, "sup": true, "ac:emoticon": true, "time": true, } // allowedAttrs reports whether ok accepts every attribute of n. @@ -203,20 +267,25 @@ func allowedAttrs(n *snode, ok func(name, value string) bool) bool { return true } -// idOnly accepts only the server-generated id, which read drops everywhere. -func idOnly(name, _ string) bool { return name == "ac:local-id" } +// isID reports whether an attribute is a server-generated id. The editor writes +// ac:local-id on tables, rows and cells, and a bare local-id on every +// paragraph (page 3109814418); Confluence regenerates both on publish. +func isID(name string) bool { return name == "ac:local-id" || name == "local-id" } + +// idOnly accepts only a server-generated id. +func idOnly(name, _ string) bool { return isID(name) } // tableAttrOK accepts what the editor writes on a table nobody configured. // data-table-width is ignored whatever its value: the editor writes one on // every table it saves, and a hand resize is lost with it on the next publish. // Any other layout or display mode is a choice somebody made. func tableAttrOK(name, value string) bool { - switch name { - case "ac:local-id", "data-table-width": + switch { + case isID(name), name == "data-table-width": return true - case "data-layout": + case name == "data-layout": return value == "align-start" - case "data-table-display-mode": + case name == "data-table-display-mode": return value == "default" } return false @@ -225,12 +294,12 @@ func tableAttrOK(name, value string) bool { // cellAttrOK accepts a cell's id, its colour (a bg: marker), a span of one, // and a style that says nothing but its alignment. func cellAttrOK(name, value string) bool { - switch name { - case "ac:local-id", "data-highlight-colour": + switch { + case isID(name), name == "data-highlight-colour": return true - case "rowspan", "colspan": + case name == "rowspan", name == "colspan": return strings.TrimSpace(value) == "1" - case "style": + case name == "style": return onlyTextAlign(value) } return false @@ -239,17 +308,20 @@ func cellAttrOK(name, value string) bool { // paragraphAttrOK accepts a cell paragraph's id and a style that says nothing // but its alignment. func paragraphAttrOK(name, value string) bool { - switch name { - case "ac:local-id": + switch { + case isID(name): return true - case "style": + case name == "style": return onlyTextAlign(value) } return false } -// textAlignDeclRE matches one text-align declaration in a style attribute. -var textAlignDeclRE = regexp.MustCompile(`(?i)text-align\s*:\s*[a-z-]+\s*;?`) +// textAlignDeclRE matches one text-align declaration in a style attribute, +// with a value the delimiter row can carry or that has no effect in Confluence +// (justify is stored and ignored, storage-format.md). Any other value keeps the +// style unmatched, and so the table raw. +var textAlignDeclRE = regexp.MustCompile(`(?i)text-align\s*:\s*(left|start|center|right|end|justify)\s*(;|$)`) // onlyTextAlign reports whether a style attribute holds no declaration but // text-align, the one a pipe table carries (as its delimiter row). @@ -272,14 +344,16 @@ var textAlignRE = regexp.MustCompile(`(?i)text-align\s*:\s*([a-z]+)`) // hand-edited storage, and left is no alignment at all, since Confluence has // no explicit left (storage-format.md). func cellAlign(c *snode) (string, bool) { - cell := styleAlign(c.attrs["style"]) + cell, _ := styleAlign(c.attrs["style"]) found, seen := cell, false for _, k := range c.kids { if k.name != "p" { continue } - a := styleAlign(k.attrs["style"]) - if a == "" { + // A paragraph's own declaration wins over the cell's, as in CSS, so a + // left paragraph in a centred cell is left. + a, declared := styleAlign(k.attrs["style"]) + if !declared { a = cell } if seen && a != found { @@ -290,35 +364,91 @@ func cellAlign(c *snode) (string, bool) { return found, true } -// styleAlign reads a text-align declaration from a style attribute. -func styleAlign(style string) string { +// styleAlign reads a text-align declaration from a style attribute, and +// whether there was one: left, start and justify are declarations of no +// alignment. +func styleAlign(style string) (string, bool) { m := textAlignRE.FindStringSubmatch(style) if m == nil { - return "" + return "", false } switch strings.ToLower(m[1]) { case "center": - return "center" + return "center", true case "right", "end": - return "right" + return "right", true } - return "" + return "", true } -// rawCellBlocks renders a raw table cell's body as Markdown blocks, except a -// paragraph carrying an attribute Markdown cannot hold -- in practice an -// alignment -- which stays storage on its own line, since a Markdown paragraph -// would drop it. That costs the paragraph's editability, not its alignment. +// rawCellBlocks renders a raw table cell's body as Markdown blocks. Three +// things differ from blockStrings, each because the raw form must lose +// nothing: +// +// - text and inline elements sitting directly in the cell, outside any +// paragraph, are one paragraph, not a block apiece; +// - an empty paragraph -- a deliberate blank line, Enter twice in the editor +// -- stays as

, unless the cell holds nothing else; +// - a paragraph carrying an attribute Markdown cannot hold, in practice an +// alignment, stays storage on its own line, which costs that paragraph's +// editability rather than its alignment. func (r *mdRenderer) rawCellBlocks(c *snode) []string { - var out []string + if emptyCell(c) { + return nil + } + var run []*snode + var blocks []string + flush := func() { + if s := strings.TrimSpace(r.renderInlineChildren(&snode{kids: run})); s != "" { + blocks = append(blocks, s) + } + run = nil + } for _, k := range c.kids { - if k.name == "p" && !allowedAttrs(k, idOnly) { - out = append(out, serialize(k)) + switch { + case k.name == "" || cellInline[k.name] || rawCellInline[k.name]: + run = append(run, k) continue + case k.name == "p" && emptyCell(k): + flush() + blocks = append(blocks, "

") + case k.name == "p" && !allowedAttrs(k, idOnly): + flush() + blocks = append(blocks, serialize(withoutIDs(k))) + default: + flush() + blocks = append(blocks, r.blockStrings([]*snode{k}, "")...) + } + } + flush() + return blocks +} + +// rawCellInline are inline elements beyond cellInline that group into a raw +// cell's loose paragraph rather than becoming a block of their own. read has no +// Markdown for them and renders their text (or nothing), the same as in any +// paragraph; that is an older gap than this one. +var rawCellInline = map[string]bool{ + "span": true, "u": true, "sub": true, "sup": true, "time": true, "ac:emoticon": true, +} + +// withoutIDs is n with the server-generated ids removed from it and everything +// under it. attrString drops ac:local-id already; the bare local-id the editor +// writes on a paragraph would otherwise be copied into the Markdown. +func withoutIDs(n *snode) *snode { + c := &snode{name: n.name, text: n.text} + if n.attrs != nil { + c.attrs = map[string]string{} + for k, v := range n.attrs { + if !isID(k) { + c.attrs[k] = v + } } - out = append(out, r.blockStrings([]*snode{k}, "")...) } - return out + for _, k := range n.kids { + c.kids = append(c.kids, withoutIDs(k)) + } + return c } // cellTexts renders a row's cells to inline strings with pipes escaped, diff --git a/internal/convert/storage_to_md_test.go b/internal/convert/storage_to_md_test.go index 2c6bdd4..15f7189 100644 --- a/internal/convert/storage_to_md_test.go +++ b/internal/convert/storage_to_md_test.go @@ -387,7 +387,7 @@ func TestRoundTripPassthrough(t *testing.T) { "raw-table-layout", "raw-table-colgroup", "raw-table-spans", "raw-table-no-header", "raw-table-header-column", "raw-table-short-row", "raw-table-block-content", "raw-table-nested", "raw-table-numbered", "raw-table-valign", "raw-table-display-fixed", - "raw-table-aligned-paragraph", "table-alignment-disagree", + "raw-table-aligned-paragraph", "table-alignment-disagree", "raw-table-cell-content", "raw-table-unknown-align", } { t.Run(name, func(t *testing.T) { src, err := os.ReadFile(filepath.Join(storage2mdDir, name, "output.md")) diff --git a/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage b/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage new file mode 100644 index 0000000..c842a71 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage @@ -0,0 +1,8 @@ +

`/`` as content containers so each cell's body stays Markdown between blank lines (an aligned `

` in a raw cell stays storage, since a Markdown paragraph has no alignment). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
+ +loose text in the section + + + + +

Loose

Blank line

Aligned

a b c

one

three

centred

x

diff --git a/internal/convert/testdata/storage2md/raw-table-cell-content/output.md b/internal/convert/testdata/storage2md/raw-table-cell-content/output.md new file mode 100644 index 0000000..94745c3 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-cell-content/output.md @@ -0,0 +1,52 @@ + + +loose text in the section + + + + + + + + + + + + + + + + +
+ +Loose + + + +Blank line + + + +Aligned + +
+ +a **b** c + + + +one + +

+ +three + +

+ +

centred

+ +
+ +x + +
diff --git a/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage b/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage index e649a52..4d8a6ec 100644 --- a/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage +++ b/internal/convert/testdata/storage2md/raw-table-colgroup/input.storage @@ -1,4 +1,4 @@ - +
diff --git a/internal/convert/testdata/storage2md/raw-table-colgroup/output.md b/internal/convert/testdata/storage2md/raw-table-colgroup/output.md index 61cf0ea..cadd580 100644 --- a/internal/convert/testdata/storage2md/raw-table-colgroup/output.md +++ b/internal/convert/testdata/storage2md/raw-table-colgroup/output.md @@ -1,4 +1,4 @@ -

Key

Value

+
diff --git a/internal/convert/testdata/storage2md/raw-table-unknown-align/input.storage b/internal/convert/testdata/storage2md/raw-table-unknown-align/input.storage new file mode 100644 index 0000000..51bb4ae --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-unknown-align/input.storage @@ -0,0 +1,6 @@ +
+ + + + +

A

a

diff --git a/internal/convert/testdata/storage2md/raw-table-unknown-align/output.md b/internal/convert/testdata/storage2md/raw-table-unknown-align/output.md new file mode 100644 index 0000000..c66af21 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-unknown-align/output.md @@ -0,0 +1,18 @@ + + + + + + + + + +
+ +A + +
+ +

a

+ +
diff --git a/internal/convert/testdata/storage2md/table-alignment-edges/input.storage b/internal/convert/testdata/storage2md/table-alignment-edges/input.storage new file mode 100644 index 0000000..71abed2 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-alignment-edges/input.storage @@ -0,0 +1,8 @@ + + + + + + + +

Count

Left wins

Justified

3

a

b

cd
ef
diff --git a/internal/convert/testdata/storage2md/table-alignment-edges/output.md b/internal/convert/testdata/storage2md/table-alignment-edges/output.md new file mode 100644 index 0000000..82c43ca --- /dev/null +++ b/internal/convert/testdata/storage2md/table-alignment-edges/output.md @@ -0,0 +1,5 @@ +| Count | Left wins | Justified | +| ---: | --- | --- | +| 3 | a | b | +| | c | d | +| | e | f | diff --git a/internal/convert/testdata/storage2md/table-browser-saved/input.storage b/internal/convert/testdata/storage2md/table-browser-saved/input.storage new file mode 100644 index 0000000..72ac698 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-browser-saved/input.storage @@ -0,0 +1 @@ +

Service

Status

Errors

auth

up

3

billing

down
since noon

12

search

ok

0

diff --git a/internal/convert/testdata/storage2md/table-browser-saved/output.md b/internal/convert/testdata/storage2md/table-browser-saved/output.md new file mode 100644 index 0000000..e40f5ae --- /dev/null +++ b/internal/convert/testdata/storage2md/table-browser-saved/output.md @@ -0,0 +1,5 @@ +| Service | Status | Errors | +| --- | :---: | ---: | +| auth | up | 3 | +| billing | down
since noon | 12 | +| search | ok | 0 | From 2fb0fa1d5eb47ba5c66fc5efaab83695d6766dfd Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 25 Sep 2026 15:20:46 -0400 Subject: [PATCH 4/8] fix: address the second raw-tables-round-trip review Each finding was reproduced before fixing. A cell's shared paragraph alignment now moves onto the cell as a text-align style, so its paragraphs stay Markdown: kept as storage, an aligned paragraph kept an image as , which is never uploaded when an exported tree is published to new pages. A nested table in a raw cell stays raw, a paragraph that renders to nothing stays storage, two text-align declarations or a list in an aligned column keep a table raw, the bare local-id is dropped everywhere, and an element holding loose text is written whole rather than a line per child. --- CLAUDE.md | 2 +- _plans/055_raw-tables-round-trip.md | 16 ++- docs/markdown-file.md | 7 +- internal/convert/storage_to_md.go | 33 +++-- internal/convert/storage_to_md_table.go | 136 +++++++++++++----- internal/convert/storage_to_md_test.go | 2 + .../raw-mixed-content/input.storage | 1 + .../storage2md/raw-mixed-content/output.md | 3 + .../raw-table-aligned-image/input.storage | 6 + .../raw-table-aligned-image/output.md | 20 +++ .../raw-table-cell-content/input.storage | 1 - .../raw-table-cell-content/output.md | 5 +- .../raw-table-list-aligned/input.storage | 6 + .../raw-table-list-aligned/output.md | 20 +++ .../raw-table-loose-text/input.storage | 7 + .../storage2md/raw-table-loose-text/output.md | 7 + .../storage2md/raw-table-nested/output.md | 21 ++- .../raw-table-repeated-align/input.storage | 6 + .../raw-table-repeated-align/output.md | 18 +++ .../input.storage | 6 + .../raw-table-textless-paragraphs/output.md | 32 +++++ .../table-alignment-disagree/output.md | 8 +- .../storage2md/table-list-ids/input.storage | 6 + .../storage2md/table-list-ids/output.md | 3 + 24 files changed, 314 insertions(+), 58 deletions(-) create mode 100644 internal/convert/testdata/storage2md/raw-mixed-content/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-mixed-content/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-aligned-image/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-aligned-image/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-list-aligned/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-list-aligned/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-loose-text/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-loose-text/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-repeated-align/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-repeated-align/output.md create mode 100644 internal/convert/testdata/storage2md/raw-table-textless-paragraphs/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-textless-paragraphs/output.md create mode 100644 internal/convert/testdata/storage2md/table-list-ids/input.storage create mode 100644 internal/convert/testdata/storage2md/table-list-ids/output.md diff --git a/CLAUDE.md b/CLAUDE.md index 462e483..d8b47e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,7 +79,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 `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. 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 that **declares** a `page_id` — in its own frontmatter or in a `pages:` entry for it (#139), resolved through `pagemeta` after discovering the root from the file's own directory (stat'd first, so `123.md` is a file). Every command taking a page uses it, which is why the manifest lookup lives here rather than at the seven call sites: without it the page argument meant one thing to `update` and another to `page-info`/`read`/`children`/`export`/`attachment-*`, so a file `update` could publish could not be named to any of them. A **disagreement** between the two locations is fatal here — it is the question being asked — while a **malformed** `markfluence.yaml` is not: a project file this resolver never consults must not make `page-info 123` fail, and the commands that bound reads by the root report it themselves. `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` and `update` report both and `check` reports the non-numeric one, and a reader should recognize the same problem across all of them. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. A **move** is `MovePage` (`move.go`), always the v1 `PUT /content/{id}/move/{position}/{targetId}` with `append` (under a page or folder) or `after` (after the last top-level page, the only way to the top of a space): the v1 route leaves the page version alone where a v2 `parentId` change bumps it, and v2 silently ignores a null `parentId`, so it cannot reach the top at all ([docs/confluence/api.md](docs/confluence/api.md#moving-a-page)). There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. 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. Every attachment file is read through `LocalAttachment.Open`, twice — once for `planAttachments`' checksum, once by `uploadAttachment`, which reads the file whole and takes the comment's checksum from *those* bytes, since the file may have changed in between and a comment misdescribing its content reads as up to date on the next publish. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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. `credentials.go` holds the credentials file's own functions — `CredentialsPath`, `ReadCredentials` (the rules `Resolve` uses, minus the permission warning, for a caller about to rewrite the file; one read returning a `CredentialsFile` of values, the lines a rewrite drops, and the mode), `WriteCredentials` (temp-file-and-rename at `0600`, writing through a symbolic link, quoting a value exactly when `readDotenv` would not read it back unchanged, then reading the result back and comparing), `DisplayPath`, and `FetchCloudID`, the unauthenticated `tenant_info` request, which bypasses `send` because `send` always sets basic auth. The setting names (`URLVar`/`UsernameVar`/`TokenVar`/`CloudIDVar`), `CredentialsDoc` and `LooseMode` are exported so `credentials-init` copies none of them. `HTTPError.ScopeMismatch`/`SiteRejectedAuth`, beside `RejectedCredential`, are the shapes `hint` matches, exported for `credentials-init`. `config.go` holds `Resolve` and the env-file reader (`loadDotenv`, which warns, over `readDotenv`, which does not), plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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, root *project.Root, index *linkindex.Index, baseURL, spaceKey string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in Markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `MentionMarkdown` is the shared builder for a mention's whole Markdown line, exported because `user-find` (#143) prints exactly it and a second copy there would be a second place to get the `@` marker and name escaping right. `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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); Plain text used as a Markdown link's text goes through `escapeLinkText` (`\`, `[`, `]`), applied to the *raw* sources only — a page title, a space key, an anchor, a display name — and via `inlineTextForLink` to a body whose every descendant is a text node. Never to already-rendered output: an `ac:link-body` holding markup has been converted to Markdown already, and escaping it yields a literal `\*\*bold\*\*`. Both directions are tested, because a fix at either extreme passes one and fails the other. `attachname.go` owns the source-path→attachment-name mapping, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. 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` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `
` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`, a span of 1, cell children that are paragraphs, lists or inline markup, a `text-align` only of a known value, and every non-empty cell in a column agreeing on alignment (no alignment, `left`, `start` and `justify` are one value, and a paragraph's own declaration beats its cell's) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Some markup is **ignored** rather than honoured because the browser editor writes it on every table it saves (measured on a markfluence table saved in the browser, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id` and the bare `local-id` it puts on every paragraph, `data-table-width` whatever its value, `data-table-display-mode="default"`, `data-layout="align-start"`, and — on an `align-start` table only — a `` of pixel widths, which the editor measures and writes on any save. Honouring them would turn every markfluence table someone edits in Confluence into HTML on the next `read`. The known cost: a hand resize writes the same `data-table-width` and ``, so it is lost when a read-back table republishes. A `` on any other layout keeps the table raw, since `default`/`full-width` are visible layouts markfluence never writes. In a raw cell, `rawCellBlocks` groups loose inline content into one paragraph, keeps an empty paragraph as `

`, and strips the bare `local-id` from a paragraph it has to serialize; `renderRawBlock` keeps loose text in a wrapper as a line rather than dropping it as whitespace. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

`/`` as content containers so each cell's body stays Markdown between blank lines (an aligned `

` in a raw cell stays storage, since a Markdown paragraph has no alignment). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
` 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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`, a span of 1, cell children that are paragraphs, lists or inline markup, a `text-align` only of a known value, and every non-empty cell in a column agreeing on alignment (no alignment, `left`, `start` and `justify` are one value, and a paragraph's own declaration beats its cell's) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Some markup is **ignored** rather than honoured because the browser editor writes it on every table it saves (measured on a markfluence table saved in the browser, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id` and the bare `local-id` it puts on every paragraph, `data-table-width` whatever its value, `data-table-display-mode="default"`, `data-layout="align-start"`, and — on an `align-start` table only — a `` of pixel widths, which the editor measures and writes on any save. Honouring them would turn every markfluence table someone edits in Confluence into HTML on the next `read`. The known cost: a hand resize writes the same `data-table-width` and ``, so it is lost when a read-back table republishes. A `` on any other layout keeps the table raw, since `default`/`full-width` are visible layouts markfluence never writes. In a raw cell, `rawCellBlocks` groups loose inline content into one paragraph, keeps an empty paragraph as `

` and one that renders to nothing (a `
`, a `

`; `renderRawBlock` writes any element holding loose text whole (`hasLooseText`), since one line per child would add whitespace to it. The bare `local-id` is in `droppedAttrs` beside `ac:local-id`. A list in an aligned column keeps a table raw: the pipe cell would publish it inside the aligned `

`. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

`s) still keeps a table raw: in the sample, ignoring it would change 3 of 109 old tables. +- **From the second review, each reproduced first:** a cell's shared + paragraph alignment moves onto the cell as `style="text-align: …"`, so its + paragraphs stay Markdown -- an aligned paragraph kept as storage kept its + image as ``, which is never uploaded when an exported tree is + published to new pages; a paragraph stays storage only when a cell's + paragraphs disagree. A nested table in a raw cell stays raw. A paragraph + that renders to nothing (`
`, `
it did not have. func (r *mdRenderer) rawCellBlocks(c *snode) []string { if emptyCell(c) { return nil @@ -414,7 +491,17 @@ func (r *mdRenderer) rawCellBlocks(c *snode) []string { blocks = append(blocks, "

") case k.name == "p" && !allowedAttrs(k, idOnly): flush() - blocks = append(blocks, serialize(withoutIDs(k))) + blocks = append(blocks, serialize(k)) + case k.name == "p": + flush() + if s := r.blockStrings([]*snode{k}, ""); len(s) > 0 && strings.TrimSpace(strings.Join(s, "")) != "" { + blocks = append(blocks, s...) + } else { + blocks = append(blocks, serialize(k)) + } + case k.name == "table": + flush() + blocks = append(blocks, r.renderRawBlock(k)) default: flush() blocks = append(blocks, r.blockStrings([]*snode{k}, "")...) @@ -432,25 +519,6 @@ var rawCellInline = map[string]bool{ "span": true, "u": true, "sub": true, "sup": true, "time": true, "ac:emoticon": true, } -// withoutIDs is n with the server-generated ids removed from it and everything -// under it. attrString drops ac:local-id already; the bare local-id the editor -// writes on a paragraph would otherwise be copied into the Markdown. -func withoutIDs(n *snode) *snode { - c := &snode{name: n.name, text: n.text} - if n.attrs != nil { - c.attrs = map[string]string{} - for k, v := range n.attrs { - if !isID(k) { - c.attrs[k] = v - } - } - } - for _, k := range n.kids { - c.kids = append(c.kids, withoutIDs(k)) - } - return c -} - // cellTexts renders a row's cells to inline strings with pipes escaped, // prefixed with a bg: marker for a cell carrying a background color. func (r *mdRenderer) cellTexts(tr *snode) []string { diff --git a/internal/convert/storage_to_md_test.go b/internal/convert/storage_to_md_test.go index 15f7189..25bb312 100644 --- a/internal/convert/storage_to_md_test.go +++ b/internal/convert/storage_to_md_test.go @@ -388,6 +388,8 @@ func TestRoundTripPassthrough(t *testing.T) { "raw-table-header-column", "raw-table-short-row", "raw-table-block-content", "raw-table-nested", "raw-table-numbered", "raw-table-valign", "raw-table-display-fixed", "raw-table-aligned-paragraph", "table-alignment-disagree", "raw-table-cell-content", "raw-table-unknown-align", + "raw-table-loose-text", "raw-table-list-aligned", "raw-table-textless-paragraphs", "raw-table-repeated-align", + "raw-mixed-content", } { t.Run(name, func(t *testing.T) { src, err := os.ReadFile(filepath.Join(storage2mdDir, name, "output.md")) diff --git a/internal/convert/testdata/storage2md/raw-mixed-content/input.storage b/internal/convert/testdata/storage2md/raw-mixed-content/input.storage new file mode 100644 index 0000000..2ba35ce --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-mixed-content/input.storage @@ -0,0 +1 @@ +

Hello,x.

diff --git a/internal/convert/testdata/storage2md/raw-mixed-content/output.md b/internal/convert/testdata/storage2md/raw-mixed-content/output.md new file mode 100644 index 0000000..a5fab42 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-mixed-content/output.md @@ -0,0 +1,3 @@ + +

Hello,x.

+
diff --git a/internal/convert/testdata/storage2md/raw-table-aligned-image/input.storage b/internal/convert/testdata/storage2md/raw-table-aligned-image/input.storage new file mode 100644 index 0000000..b30c7d6 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-aligned-image/input.storage @@ -0,0 +1,6 @@ +
`/`` as content containers so each cell's body stays Markdown between blank lines (a Markdown paragraph has no alignment, so `hoistCellAlign` moves a cell's shared paragraph alignment onto the cell as `style="text-align: …"`, a form Confluence honours, and an aligned `

` stays storage only when a cell's paragraphs disagree — storage would also keep an image in it as ``, which is never uploaded to a new page). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
@@ -437,3 +438,16 @@ Nothing changes on the publish side. only in tables, and are left alone. Old-editor markup (`class="wrapped"`, empty `
`. If the +paragraphs in a cell have different alignments, an aligned paragraph stays +storage format. A table in a raw cell also stays a raw table. A table that markfluence published stays a GFM table after someone edits the page in Confluence. The Confluence editor adds attributes to every table that diff --git a/internal/convert/storage_to_md.go b/internal/convert/storage_to_md.go index 96bc2c5..3136c49 100644 --- a/internal/convert/storage_to_md.go +++ b/internal/convert/storage_to_md.go @@ -873,8 +873,9 @@ func serialize(n *snode) string { // droppedAttrs are server-generated per-instance ids that are noise in the output // and that Confluence regenerates on publish, so passthrough serialization omits -// them. -var droppedAttrs = map[string]bool{"ac:macro-id": true, "ac:local-id": true} +// them. The editor writes the bare local-id on every paragraph inside a table, +// and on ADF content. +var droppedAttrs = map[string]bool{"ac:macro-id": true, "ac:local-id": true, "local-id": true} // attrString renders an element's attributes (sorted, XML-escaped) as a leading- // space attribute list, dropping the server-generated ids in droppedAttrs. @@ -902,6 +903,9 @@ func attrString(attrs map[string]string) string { // (expand, panel, …), keeping their bodies readable while the structure and // parameters survive verbatim. func (r *mdRenderer) renderRawBlock(n *snode) string { + if n.name == "th" || n.name == "td" { + n = hoistCellAlign(n) + } open := "<" + n.name + attrString(n.attrs) + ">" closeTag := "" @@ -921,19 +925,19 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { if len(n.kids) == 0 { return "<" + n.name + attrString(n.attrs) + " />" } + // Text in a wrapper, beside its elements, is content rather than layout, + // and one line per child would add whitespace to it: written whole, the + // element is exact. + if hasLooseText(n) { + return serialize(n) + } // Wrapper: one line per element child; a child with element children (or a // content container) recurses, a leaf child is serialized raw inline. var parts []string for _, k := range n.kids { if k.name == "" { - // Whitespace between tags is layout; anything else is content a - // wrapper happens to hold loose, and passing it through is what - // the raw form is for. - if t := strings.TrimSpace(k.text); t != "" { - parts = append(parts, xmlTextEscape(t)) - } - continue + continue // inter-tag whitespace; hasLooseText caught anything else } if isContentContainer(k.name) || hasElementChild(k) { parts = append(parts, r.renderRawBlock(k)) @@ -944,6 +948,17 @@ func (r *mdRenderer) renderRawBlock(n *snode) string { return open + "\n" + strings.Join(parts, "\n") + "\n" + closeTag } +// hasLooseText reports whether n holds text other than whitespace directly, +// beside or instead of elements. +func hasLooseText(n *snode) bool { + for _, k := range n.kids { + if k.name == "" && strings.TrimSpace(k.text) != "" { + return true + } + } + return false +} + // isContentContainer reports whether an element holds block content that should // be converted to Markdown rather than serialized raw, so a passed-through // wrapper keeps an editable body. A table cell is one because a table only diff --git a/internal/convert/storage_to_md_table.go b/internal/convert/storage_to_md_table.go index a9b7468..e3d76dc 100644 --- a/internal/convert/storage_to_md_table.go +++ b/internal/convert/storage_to_md_table.go @@ -146,6 +146,17 @@ func tableShape(n *snode) (pipeTable, bool) { } } } + // A list cannot sit in an aligned column: the pipe cell publishes it inside + // the column's aligned

, where a list is invalid, and the next read loses + // it. + for _, tr := range append([]*snode{t.header}, t.rows...) { + cells, _ := rowCells(tr) + for col, c := range cells { + if t.aligns[col] != "" && (findChild(c, "ul") != nil || findChild(c, "ol") != nil) { + return pipeTable{}, false + } + } + } return t, true } @@ -320,19 +331,20 @@ func paragraphAttrOK(name, value string) bool { // textAlignDeclRE matches one text-align declaration in a style attribute, // with a value the delimiter row can carry or that has no effect in Confluence // (justify is stored and ignored, storage-format.md). Any other value keeps the -// style unmatched, and so the table raw. +// style unmatched, and so the table raw. It is the one definition of a +// declaration read understands: onlyTextAlign validates with it and styleAlign +// reads with it. var textAlignDeclRE = regexp.MustCompile(`(?i)text-align\s*:\s*(left|start|center|right|end|justify)\s*(;|$)`) -// onlyTextAlign reports whether a style attribute holds no declaration but -// text-align, the one a pipe table carries (as its delimiter row). +// onlyTextAlign reports whether a style attribute holds no declaration but a +// single text-align, the one a pipe table carries (as its delimiter row). Two +// are refused rather than resolved: CSS takes the last, and a style that says +// it twice was not written by an editor. func onlyTextAlign(style string) bool { - return strings.Trim(textAlignDeclRE.ReplaceAllString(style, ""), " \t\n;") == "" + return len(textAlignDeclRE.FindAllString(style, -1)) <= 1 && + strings.Trim(textAlignDeclRE.ReplaceAllString(style, ""), " \t\n;") == "" } -// textAlignRE pulls the value out of a text-align declaration anywhere in a style -// attribute. -var textAlignRE = regexp.MustCompile(`(?i)text-align\s*:\s*([a-z]+)`) - // cellAlign reports the one alignment a cell's content carries, normalized to // the delimiter row's vocabulary, or false when its paragraphs disagree -- a // cell with one centred line and one plain one has no column alignment that @@ -368,7 +380,7 @@ func cellAlign(c *snode) (string, bool) { // whether there was one: left, start and justify are declarations of no // alignment. func styleAlign(style string) (string, bool) { - m := textAlignRE.FindStringSubmatch(style) + m := textAlignDeclRE.FindStringSubmatch(style) if m == nil { return "", false } @@ -381,17 +393,82 @@ func styleAlign(style string) (string, bool) { return "", true } -// rawCellBlocks renders a raw table cell's body as Markdown blocks. Three -// things differ from blockStrings, each because the raw form must lose -// nothing: +// hoistCellAlign returns a raw cell with its paragraphs' shared alignment moved +// onto the cell, so the paragraphs can stay Markdown: a Markdown paragraph has +// no alignment, and one written as storage keeps its images and links as +// storage too -- an image that is never uploaded when the file is published to +// a new page. A text-align on the cell is a form Confluence honours +// (storage-format.md, verified 2026-08-07), and the next read keeps it, since a +// raw cell's attributes are written as they are. +// +// Only when every piece of content is a paragraph declaring the same alignment +// and nothing else in its style, and the cell's own style is at most a +// text-align: loose text or a list would otherwise gain an alignment it did not +// have. Otherwise the cell is returned unchanged and rawCellBlocks writes an +// aligned paragraph as storage. +func hoistCellAlign(c *snode) *snode { + if !onlyTextAlign(c.attrs["style"]) { + return c + } + var decl string + for _, k := range c.kids { + switch { + case k.name == "" && strings.TrimSpace(k.text) == "": + case k.name == "p" && emptyCell(k): + case k.name == "p" && allowedAttrs(k, paragraphAttrOK): + m := textAlignDeclRE.FindString(k.attrs["style"]) + if m == "" || (decl != "" && !strings.EqualFold(normalizeDecl(m), decl)) { + return c + } + decl = normalizeDecl(m) + default: + return c + } + } + if decl == "" { + return c + } + out := &snode{name: c.name, attrs: map[string]string{}, kids: make([]*snode, len(c.kids))} + for k, v := range c.attrs { + out.attrs[k] = v + } + out.attrs["style"] = decl + for i, k := range c.kids { + out.kids[i] = k + if k.name == "p" && k.attrs["style"] != "" { + p := &snode{name: "p", attrs: map[string]string{}, kids: k.kids} + for a, v := range k.attrs { + if a != "style" { + p.attrs[a] = v + } + } + out.kids[i] = p + } + } + return out +} + +// normalizeDecl spells a matched text-align declaration the way markfluence +// writes one. +func normalizeDecl(m string) string { + return "text-align: " + strings.ToLower(textAlignDeclRE.FindStringSubmatch(m)[1]) + ";" +} + +// rawCellBlocks renders a raw table cell's body as Markdown blocks. What +// differs from blockStrings is each because the raw form must lose nothing: // // - text and inline elements sitting directly in the cell, outside any // paragraph, are one paragraph, not a block apiece; // - an empty paragraph -- a deliberate blank line, Enter twice in the editor // -- stays as

, unless the cell holds nothing else; +// - a paragraph that renders to nothing although it holds something (a +//
, a date read has no Markdown for) stays storage; // - a paragraph carrying an attribute Markdown cannot hold, in practice an -// alignment, stays storage on its own line, which costs that paragraph's -// editability rather than its alignment. +// alignment hoistCellAlign could not move to the cell, stays storage on +// its own line, which costs that paragraph's editability rather than its +// alignment; +// - a table stays raw. A nested table that fits GFM would otherwise be +// republished with markfluence's layout and a

+ + + + +

Diagram

Figure 1

diff --git a/internal/convert/testdata/storage2md/raw-table-aligned-image/output.md b/internal/convert/testdata/storage2md/raw-table-aligned-image/output.md new file mode 100644 index 0000000..3737337 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-aligned-image/output.md @@ -0,0 +1,20 @@ + + + + + + + + + +
+ +Diagram + +
+ +![](d.png) + +Figure 1 + +
diff --git a/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage b/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage index c842a71..7d34b9a 100644 --- a/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage +++ b/internal/convert/testdata/storage2md/raw-table-cell-content/input.storage @@ -1,6 +1,5 @@ -loose text in the section diff --git a/internal/convert/testdata/storage2md/raw-table-cell-content/output.md b/internal/convert/testdata/storage2md/raw-table-cell-content/output.md index 94745c3..03cf90c 100644 --- a/internal/convert/testdata/storage2md/raw-table-cell-content/output.md +++ b/internal/convert/testdata/storage2md/raw-table-cell-content/output.md @@ -1,6 +1,5 @@

Loose

Blank line

Aligned

a b c

one

three

centred

x

-loose text in the section diff --git a/internal/convert/testdata/storage2md/raw-table-list-aligned/input.storage b/internal/convert/testdata/storage2md/raw-table-list-aligned/input.storage new file mode 100644 index 0000000..989cc4e --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-list-aligned/input.storage @@ -0,0 +1,6 @@ +
@@ -33,9 +32,9 @@ one three - + -

centred

+centred
+ + + + +

Choices

pick one

  • x

diff --git a/internal/convert/testdata/storage2md/raw-table-list-aligned/output.md b/internal/convert/testdata/storage2md/raw-table-list-aligned/output.md new file mode 100644 index 0000000..740ec3f --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-list-aligned/output.md @@ -0,0 +1,20 @@ + + + + + + + + + +
+ +Choices + +
+ +

pick one

+ +- x + +
diff --git a/internal/convert/testdata/storage2md/raw-table-loose-text/input.storage b/internal/convert/testdata/storage2md/raw-table-loose-text/input.storage new file mode 100644 index 0000000..0475f49 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-loose-text/input.storage @@ -0,0 +1,7 @@ + + + +loose text beside the rows + + +

A

a

diff --git a/internal/convert/testdata/storage2md/raw-table-loose-text/output.md b/internal/convert/testdata/storage2md/raw-table-loose-text/output.md new file mode 100644 index 0000000..0475f49 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-loose-text/output.md @@ -0,0 +1,7 @@ + + + +loose text beside the rows + + +

A

a

diff --git a/internal/convert/testdata/storage2md/raw-table-nested/output.md b/internal/convert/testdata/storage2md/raw-table-nested/output.md index 49b62cc..4c69a9a 100644 --- a/internal/convert/testdata/storage2md/raw-table-nested/output.md +++ b/internal/convert/testdata/storage2md/raw-table-nested/output.md @@ -10,9 +10,24 @@ Outer
-| Inner | -| --- | -| x | + + + + + + + + + +
+ +Inner + +
+ +x + +
+ + + + +

A

a

diff --git a/internal/convert/testdata/storage2md/raw-table-repeated-align/output.md b/internal/convert/testdata/storage2md/raw-table-repeated-align/output.md new file mode 100644 index 0000000..06e46b6 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-repeated-align/output.md @@ -0,0 +1,18 @@ + + + + + + + + + +
+ +

A

+ +
+ +a + +
diff --git a/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/input.storage b/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/input.storage new file mode 100644 index 0000000..2736a6f --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/input.storage @@ -0,0 +1,6 @@ + + + + + +

Blank line

Date

a


b

diff --git a/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/output.md b/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/output.md new file mode 100644 index 0000000..4391a32 --- /dev/null +++ b/internal/convert/testdata/storage2md/raw-table-textless-paragraphs/output.md @@ -0,0 +1,32 @@ + + + + + + + + + + + +
+ +Blank line + + + +Date + +
+ +a + +


+ +b + +
+ +

+ +
diff --git a/internal/convert/testdata/storage2md/table-alignment-disagree/output.md b/internal/convert/testdata/storage2md/table-alignment-disagree/output.md index 9f63d53..e2e38ea 100644 --- a/internal/convert/testdata/storage2md/table-alignment-disagree/output.md +++ b/internal/convert/testdata/storage2md/table-alignment-disagree/output.md @@ -6,9 +6,9 @@ Service -
+ -

Errors

+Errors
+ -

3

+3
+ + + + +

Values

  • open

diff --git a/internal/convert/testdata/storage2md/table-list-ids/output.md b/internal/convert/testdata/storage2md/table-list-ids/output.md new file mode 100644 index 0000000..4501803 --- /dev/null +++ b/internal/convert/testdata/storage2md/table-list-ids/output.md @@ -0,0 +1,3 @@ +| Values | +| --- | +|

| From fb1d9a2d3b63dc51a2e5bf323c374388616ae2e6 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Fri, 25 Sep 2026 15:54:08 -0400 Subject: [PATCH 5/8] test(convert): a property test over tables, and what it found table_property_test.go generates tables from a seed and checks that each reads without error, gives Markdown that is a fixed point after one publish, and publishes back to the same model of what takes effect in Confluence. The model ignores exactly the differences read may make, so that list is written down once. 3000 seeds run in every make test, a fuzz target runs more, and a corpus test runs the checks over real tables named by MF_TABLE_CORPUS. What it found, now fixed: a "|" in a list in a pipe cell broke the table, so such a table reads raw; a
beside a list in a pipe cell published as a blank line; a trailing empty paragraph, or one of only
s, was lost in a pipe cell, so it reads raw; loose text beside an aligned paragraph was read as aligned; and an empty paragraph counted as disagreeing with its cell's alignment. docs/markdown-file.md wrongly said a multi-line cell publishes back to paragraphs; it publishes line breaks, which look the same. --- CLAUDE.md | 4 +- _plans/055_raw-tables-round-trip.md | 17 + docs/markdown-file.md | 3 +- internal/convert/storage_to_md.go | 13 +- internal/convert/storage_to_md_table.go | 66 +- internal/convert/storage_to_md_test.go | 25 +- internal/convert/table_property_test.go | 679 ++++++++++++++++++ .../raw-table-list-pipe/input.storage | 6 + .../storage2md/raw-table-list-pipe/output.md | 18 + 9 files changed, 805 insertions(+), 26 deletions(-) create mode 100644 internal/convert/table_property_test.go create mode 100644 internal/convert/testdata/storage2md/raw-table-list-pipe/input.storage create mode 100644 internal/convert/testdata/storage2md/raw-table-list-pipe/output.md diff --git a/CLAUDE.md b/CLAUDE.md index d8b47e6..8d8e664 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,7 +79,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 `WalkSpace` (the same traversal seeded from a space's root pages, via `client.ListSpaceRootPages`) and `AllDepths`. Both go through one `walker`, so the depth rule and the visited guard exist in a single copy. 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 that **declares** a `page_id` — in its own frontmatter or in a `pages:` entry for it (#139), resolved through `pagemeta` after discovering the root from the file's own directory (stat'd first, so `123.md` is a file). Every command taking a page uses it, which is why the manifest lookup lives here rather than at the seven call sites: without it the page argument meant one thing to `update` and another to `page-info`/`read`/`children`/`export`/`attachment-*`, so a file `update` could publish could not be named to any of them. A **disagreement** between the two locations is fatal here — it is the question being asked — while a **malformed** `markfluence.yaml` is not: a project file this resolver never consults must not make `page-info 123` fail, and the commands that bound reads by the root report it themselves. `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` and `update` report both and `check` reports the non-numeric one, and a reader should recognize the same problem across all of them. 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)). Page **status** — the title lozenge — is v1 only and lives in `state.go` (`PageState`/`AvailableStates`/`SetPageState`, plus `StateVocabulary`, which keeps the space's statuses and the caller's own custom ones in separate fields because only the first are valid for a file); v2 carries no state field on a page in any form and there is no expansion that adds one, so a lozenge is one extra request per page, always. `AvailableStates` must be asked about the page the status is going on — its answer varies by caller *and* page, and it needs edit permission on that page. A **move** is `MovePage` (`move.go`), always the v1 `PUT /content/{id}/move/{position}/{targetId}` with `append` (under a page or folder) or `after` (after the last top-level page, the only way to the top of a space): the v1 route leaves the page version alone where a v2 `parentId` change bumps it, and v2 silently ignores a null `parentId`, so it cannot reach the top at all ([docs/confluence/api.md](docs/confluence/api.md#moving-a-page)). There is deliberately **no `ClearPageState`**: the `DELETE` route exists, but nothing can reach it until a clearing spelling does, and an unused write method is a loaded gun. 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`. **Two error types on the request path, and one predicate for them**: an `*HTTPError` once a response has a status, an unexported `requestError` when there is none (a transport failure, a request that would not build, a body that would not decode), and `FromRequest` answers whether an error is either. That is what lets a caller tell a server failure from a local one — `jsonout.CodeOr(err, fallback)` is the whole point of it, since `CodeFor` alone reports every non-`HTTPError` as `NETWORK` and so turns `no title given` into a network problem (#133). The rule is deliberately scoped to the request: `DownloadAttachment` writing to the caller's writer, `uploadAttachment` opening the caller's file, and `Resolve` reading the environment stay untyped, because tagging them would misreport an unreadable file as a network failure. The wrapper carries no message of its own, so `Error()` is the inner text verbatim and nothing a reader sees changed. 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. Every attachment file is read through `LocalAttachment.Open`, twice — once for `planAttachments`' checksum, once by `uploadAttachment`, which reads the file whole and takes the comment's checksum from *those* bytes, since the file may have changed in between and a comment misdescribing its content reads as up to date on the next publish. **Four 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` (whose loop is `walkV2`, shared rather than copied so a counting caller can stream — a second implementation of v2 paging is how one of them comes to terminate on a short page), 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. **`/wiki/rest/api/space` is a fifth, and the one that punishes the obvious choice**: it pages by `start`/`limit` offset exactly as the child collections do, and **a short page is not the end** — asked for 250 from `start=0` it answered 200, and `start=200` then answered 250 more, against 525 spaces. `listV1` stops on that short page, so `WalkSpaceOperations` has its own loop terminating on an **empty** page, advancing by rows *returned* rather than by the limit asked for, bounded by `maxSpacePages` since an empty page is the only end signal offset paging has here. The first version of the probe that found this trusted the short page and reported 200 spaces with total confidence ([users.md](docs/confluence/users.md)). **`/wiki/rest/api/search/user` is a fourth**, and the one that looks most like an existing scheme while not being it: it pages by `start`/`limit` offset exactly as `listV1` does, so `listV1` is the obvious home for it and is a trap — the route **caps a page at 100 rows while echoing back whatever limit was requested** (101, 250 and 500 all answer 100), and `listV1` asks for `v1PageSize = 250` and reads a short page as the end of the collection, so it would truncate every result set past 100 with no error at all. `SearchUsers` lives in its own `users.go` with `userPageSize = 100` and the measurement beside it for that reason, and `TestPageCapDoesNotTruncate` is the regression. It also carries `maxUserPages`, `searchCQLBounded`'s guard for the same hazard reached a different way: a short page is the *only* end signal offset paging here has, so a server that clamped `start` — or ignored it the way `/wiki/rest/api/search` ignores it outright — would return a full page forever and an unbounded walk would collect rows until it ran out of memory. Its `totalSize` is a *third* kind of wrong: not absent like v1's and not an estimate like `/search`'s, but the row count of the page just fetched, so `limit=3` answers 3 and `limit=500` answers 100 against 304 real matches. `user.go` holds the two identity routes (`CurrentUser`, `UserInfo` — both `read:confluence-user`, both seeing a deactivated account the directory cannot) and `WalkSpaceOperations`; `space.go` holds `GetSpace` (one v1 request answering identity, the caller's own operations, description, labels and the homepage *with its title*), `SpaceStateSettings` (space-admin only, so a 403 that is not a rejected credential is `(nil, nil)` rather than an error) and `WalkSpacePages`. Both space routes decode the space `id` as a `json.Number`: v1 reports it as a **number** where every v2 route reports a string, and `homepage.id` in the same response is a string. 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. `credentials.go` holds the credentials file's own functions — `CredentialsPath`, `ReadCredentials` (the rules `Resolve` uses, minus the permission warning, for a caller about to rewrite the file; one read returning a `CredentialsFile` of values, the lines a rewrite drops, and the mode), `WriteCredentials` (temp-file-and-rename at `0600`, writing through a symbolic link, quoting a value exactly when `readDotenv` would not read it back unchanged, then reading the result back and comparing), `DisplayPath`, and `FetchCloudID`, the unauthenticated `tenant_info` request, which bypasses `send` because `send` always sets basic auth. The setting names (`URLVar`/`UsernameVar`/`TokenVar`/`CloudIDVar`), `CredentialsDoc` and `LooseMode` are exported so `credentials-init` copies none of them. `HTTPError.ScopeMismatch`/`SiteRejectedAuth`, beside `RejectedCredential`, are the shapes `hint` matches, exported for `credentials-init`. `config.go` holds `Resolve` and the env-file reader (`loadDotenv`, which warns, over `readDotenv`, which does not), plus the **permission warning** (#136): a *regular* credentials file or `--env-file` reachable by anyone but its owner (`mode.Perm()&0o077`; a pipe from `--env-file <(pass show …)` reports 0440 and no chmod can fix it) *and* containing `CONFLUENCE_TOKEN` earns a warning naming the file, its mode, and the `chmod`. Both halves matter — a file holding only the URL and username leaks nothing, and a warning that fires on a file with no secret in it is how one becomes something people scroll past. It stats rather than lstats (a link's own `0777` would cry wolf over a `0600` target), lives in `loadDotenv` because that is the one function both the credentials file and `--env-file` pass through, and reaches the reader through `SetSecurityWarner` for the same reason `SetRetryLogger` exists — wired to `cmd/root.go`'s `reportSecurityWarning`, which prints it (human mode) *and* records it via `jsonout.AddWarning`, since stderr under `--json` is a schema-validated document with no room for a stray line. A group/world-*writable* file with no token in it is knowingly **not** covered: the same-source rule means a URL rewritten there can no longer be paired with a token from somewhere else, and the cloud ID follows the URL. 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, root *project.Root, index *linkindex.Index, baseURL, spaceKey string) (*ConfluencePage, error)`. `root` bounds which images and parent references may be read (S1/S2) and is what an image's recorded `Source` is relative to; `index` is the tree-wide link/anchor index for `root` (`internal/linkindex.Build`), built once and shared across every file converted under it rather than rebuilt per conversion — both are discovered/built by the caller (`internal/project`/`internal/linkindex`), which is why this package stays client-free. 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; `mention.go` owns the user-mention mapping in both directions (#91): a mention is 80% of all `` usage, and it converts to `[@Display Name](https://home.atlassian.com/people/{accountId})`. Three things decide its shape, each measured rather than reasoned. **The URL is Atlassian Home, not the site** — Confluence's own renderer still emits `{site}/wiki/people/{id}`, which no longer resolves usefully in a browser, so a mention in Markdown names *no site*, needs nothing from configuration, and is therefore recognisable by `check` with no client at all. **Matching is on the path, ignoring host and query**, because several spellings of one target circulate (the Home URL, the modal's `?cloudId=` copy, the `/o/{orgId}` redirect, both Confluence forms, root-relative) and none of `cloudId`/`ref`/the org segment identifies the person — only the id does, and `ri:user` stores nothing else. **The `@` on the link text is the marker**, load-bearing rather than decoration: the URL cannot tell "mention this person" from "link to their profile", so without it anyone writing the second would silently get the first. The account id is **not** pattern-validated (two shapes are live on one instance, so a pattern tight enough for one rejects the other) and `ri:local-id` is never emitted (a mention carrying only the id resolves to the same person, verified via ADF). `MentionMarkdown` is the shared builder for a mention's whole Markdown line, exported because `user-find` (#143) prints exactly it and a second copy there would be a second place to get the `@` marker and name escaping right. `ConfluencePage.Mentions` reports the ids the *forward* direction emitted so the caller can warn about one that names nobody — the `Attachments` arrangement, and necessary because Confluence accepts any id and renders `@Unlicensed user` rather than failing, and the profile URL 200s either way. An unresolvable mention still renders as a link, `[@Unlicensed user](…)` — that wording mirrors Confluence because the only ids reaching it are the ones the page labels that way: a **deactivated account resolves normally** and keeps its name (measured across every mention on a real page — 18 of them, six departed, all 200, returning e.g. `Mark Reid (Deactivated)`), so a departed colleague never takes that branch. Name resolution is `pagedoc.UserCache`, a per-run cross-page cache, and the tri-state is the part to preserve: `client.LookupUser` separates a name from `ErrNoSuchUser` from an unaskable question, `StorageOptions.UserNames` carries that as name / `""` / absent, and only a **confirmed** absence renders the placeholder. Flattening those would write a fabricated name over a real one the moment a VPN dropped mid-export, across a whole tree, into a file that then looks authoritative — which is also why the cache remembers a 404 but not a timeout (one is an answer, the other is not) and why `MentionWarnings` warns only about a confirmed absence; `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 and a space link convert, and 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); Plain text used as a Markdown link's text goes through `escapeLinkText` (`\`, `[`, `]`), applied to the *raw* sources only — a page title, a space key, an anchor, a display name — and via `inlineTextForLink` to a body whose every descendant is a text node. Never to already-rendered output: an `ac:link-body` holding markup has been converted to Markdown already, and escaping it yields a literal `\*\*bold\*\*`. Both directions are tested, because a fix at either extreme passes one and fails the other. `attachname.go` owns the source-path→attachment-name mapping, which is now the path's **base name** and nothing else (#59/`_plans/029`): the name is the attachment's identity, so an encoded path moved the name every time the file moved and orphaned the old attachment, and the path is recorded in the comment anyway. The mapping is therefore lossy, and what the bijection used to buy is an explicit refusal — two assets in one document whose base names agree return a typed `NameCollisionError` from `MdToConfluence`, which is a *failure* and not a `Broken` entry, since nothing blocks a publish on `Broken`. `check` catches that error and reports it as `Broken` anyway, because there it is a document defect like a dead link rather than a converter failure. A stored name is never interpreted in the other direction either: `sourceFor` reads the recorded path or uses the name verbatim. 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` (GitHub/Confluence slugs, doc-link + anchor rewriting against `internal/linkindex`'s tree-wide index; `resolveDocKey` resolves a destination to the index's root-relative key and reports `escapes` — a purely lexical check on the *query* side, since the index itself needs no clamp: an escaping key can never be in it, built by walking downward from root). A doc-link target is one of four severities, #42: missing entirely or escaping root is **Broken** (`LINK BROKEN: … (not found|outside the documentation root)`) and replaces the whole `` element — tags and visible text alike — with that literal message, matching `images.go`'s precedent for a missing image (`renderLink` needs a small per-node flag, `linkBrokenText`, since goldmark still invokes a container node's renderer on the matching leaving call regardless of `WalkSkipChildren` on entering, and there is no `` to write in the broken case); existing on disk with no `page_id` yet is unchanged — a **warning**, the normal state of an unpublished tree; a `#fragment` matching no heading on an otherwise-resolving target also **warns**, gated on `linkindex.Index.FileExists` so a missing/escaping target isn't double-reported. `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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`, a span of 1, cell children that are paragraphs, lists or inline markup, a `text-align` only of a known value, and every non-empty cell in a column agreeing on alignment (no alignment, `left`, `start` and `justify` are one value, and a paragraph's own declaration beats its cell's) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Some markup is **ignored** rather than honoured because the browser editor writes it on every table it saves (measured on a markfluence table saved in the browser, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id` and the bare `local-id` it puts on every paragraph, `data-table-width` whatever its value, `data-table-display-mode="default"`, `data-layout="align-start"`, and — on an `align-start` table only — a `` of pixel widths, which the editor measures and writes on any save. Honouring them would turn every markfluence table someone edits in Confluence into HTML on the next `read`. The known cost: a hand resize writes the same `data-table-width` and ``, so it is lost when a read-back table republishes. A `` on any other layout keeps the table raw, since `default`/`full-width` are visible layouts markfluence never writes. In a raw cell, `rawCellBlocks` groups loose inline content into one paragraph, keeps an empty paragraph as `

` and one that renders to nothing (a `
`, a `

`; `renderRawBlock` writes any element holding loose text whole (`hasLooseText`), since one line per child would add whitespace to it. The bare `local-id` is in `droppedAttrs` beside `ac:local-id`. A list in an aligned column keeps a table raw: the pipe cell would publish it inside the aligned `

`. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

`/`` as content containers so each cell's body stays Markdown between blank lines (a Markdown paragraph has no alignment, so `hoistCellAlign` moves a cell's shared paragraph alignment onto the cell as `style="text-align: …"`, a form Confluence honours, and an aligned `

` stays storage only when a cell's paragraphs disagree — storage would also keep an image in it as ``, which is never uploaded to a new page). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
` 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_table.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 `---`. Rows still fall through to the GFM renderer. **The way back writes a pipe table only when Markdown expresses the whole table** (#55, `_plans/055`), and otherwise the table as raw storage through `renderRawBlock`, with `

`, a span of 1, cell children that are paragraphs, lists or inline markup, a `text-align` only of a known value, and every non-empty cell in a column agreeing on alignment (no alignment, `left`, `start` and `justify` are one value, and a paragraph's own declaration beats its cell's) — since alignment is per-paragraph in Confluence and per-column in GFM, a column that disagrees could not be written without republishing cells with an alignment they lacked; this replaced a majority vote that did exactly that. Some markup is **ignored** rather than honoured because the browser editor writes it on every table it saves (measured on a markfluence table saved in the browser, [storage-format.md](docs/confluence/storage-format.md#what-the-editor-writes-on-a-table)): `ac:local-id` and the bare `local-id` it puts on every paragraph, `data-table-width` whatever its value, `data-table-display-mode="default"`, `data-layout="align-start"`, and — on an `align-start` table only — a `` of pixel widths, which the editor measures and writes on any save. Honouring them would turn every markfluence table someone edits in Confluence into HTML on the next `read`. The known cost: a hand resize writes the same `data-table-width` and ``, so it is lost when a read-back table republishes. A `` on any other layout keeps the table raw, since `default`/`full-width` are visible layouts markfluence never writes. In a raw cell, `rawCellBlocks` groups loose inline content into one paragraph, keeps an empty paragraph as `

` and one that renders to nothing (a `
`, a `

`; `renderRawBlock` writes any element holding loose text whole (`hasLooseText`), since one line per child would add whitespace to it. The bare `local-id` is in `droppedAttrs` beside `ac:local-id`. A list in an aligned column keeps a table raw: the pipe cell would publish it inside the aligned `

`. So does a list holding a `|` anywhere (unescaped it splits the row; escaped, the backslash survives into an `href`), a paragraph of nothing but `
`s, and an empty paragraph at either end of a cell. `renderCellLines` puts no `
` beside a list, which ends its line by being a block. In `cellAlign`, loose inline content has the cell's alignment and an empty paragraph has none to disagree with. A multi-line cell is one `

` per line — Enter in the editor starts a new `

`, it does not insert a `
` — so `renderCellLines` in `storage_to_md.go` joins sibling `

` children with a literal `
` rather than nothing: a GFM table row is exactly one physical line, so a real newline isn't an option, and the same substitution catches a bare mid-line `
` (Shift+Enter) that would otherwise render as the two-space hard break valid in ordinary block content but not inside a table row. A `

`/`` as content containers so each cell's body stays Markdown between blank lines (a Markdown paragraph has no alignment, so `hoistCellAlign` moves a cell's shared paragraph alignment onto the cell as `style="text-align: …"`, a form Confluence honours, and an aligned `

` stays storage only when a cell's paragraphs disagree — storage would also keep an image in it as ``, which is never uploaded to a new page). `tableShape` decides, over an **allowlist**, so an attribute nobody has seen keeps a table raw rather than being dropped: one header row of `

` and nothing but `` below it (GFM has no headerless table, and promoting the first row turned ``s into ``s on the next publish), every row as wide as the header, no `
` + + // A list holding a "|" -- in its text or an href -- cannot be a pipe cell + // at all: unescaped the "|" splits the row, and escaped it survives into + // the href as a literal backslash. Such a table reads back raw instead + // (#55, storage2md/raw-table-list-pipe). + in := `
abc
` + `` + `` + - `` + `
ab
  • one
  • two
  1. a

  2. b

` - want := "| a | b | c |\n| --- | --- | --- |\n" + - "|
  • one
  • two
|
  1. a

  2. b

|" + - `
|` + "\n" + want := "| a | b |\n| --- | --- |\n" + + "|
  • one
  • two
|
  1. a

  2. b

|\n" got, err := convert.StorageToMarkdown(in, convert.StorageOptions{}) if err != nil { @@ -301,8 +296,8 @@ func TestStorageToMarkdownPassesThroughListsInCells(t *testing.T) { if err != nil { t.Fatalf("MdToConfluence: %v", err) } - if !strings.Contains(page.HTML, `
  • one
  • two
`) { - t.Errorf("published storage lost the list:\n%s", page.HTML) + if !strings.Contains(page.HTML, `
  • one
  • two
`) { + t.Errorf("published storage lost the list, or the table:\n%s", page.HTML) } } @@ -389,7 +384,7 @@ func TestRoundTripPassthrough(t *testing.T) { "raw-table-nested", "raw-table-numbered", "raw-table-valign", "raw-table-display-fixed", "raw-table-aligned-paragraph", "table-alignment-disagree", "raw-table-cell-content", "raw-table-unknown-align", "raw-table-loose-text", "raw-table-list-aligned", "raw-table-textless-paragraphs", "raw-table-repeated-align", - "raw-mixed-content", + "raw-mixed-content", "raw-table-list-pipe", } { t.Run(name, func(t *testing.T) { src, err := os.ReadFile(filepath.Join(storage2mdDir, name, "output.md")) diff --git a/internal/convert/table_property_test.go b/internal/convert/table_property_test.go new file mode 100644 index 0000000..8406a3d --- /dev/null +++ b/internal/convert/table_property_test.go @@ -0,0 +1,679 @@ +package convert + +import ( + "fmt" + "math/rand/v2" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/mozilla/markfluence/internal/frontmatter" + "github.com/mozilla/markfluence/internal/linkindex" + "github.com/mozilla/markfluence/internal/project" +) + +// The table property test (#55, _plans/055): generate tables from a seed, read +// each one, publish the Markdown, and check three things -- nothing errors, the +// Markdown is a fixed point, and nothing that takes effect in Confluence is +// lost. Two code reviews each found edge cases in the rules that decide between +// a pipe table and a raw one; this tries the shapes nobody thought of. +// +// The third check compares a model of each table (tableModel) rather than its +// storage, because storage legitimately changes: ids are regenerated, a pipe +// table gains markfluence's layout and a , a raw cell's shared alignment +// moves onto the cell. What the model ignores is the list of differences read +// is allowed to make, written down in one place. + +// propertySeeds is how many tables the ordinary test run tries. The fuzz +// target below runs as many more as it is given time for. +const propertySeeds = 3000 + +func TestTablePropertyRoundTrip(t *testing.T) { + env := newPropertyEnv(t) + for seed := range uint64(propertySeeds) { + if msg := env.check(seed); msg != "" { + t.Fatal(msg) + } + } +} + +// FuzzTableProperty runs the same check over seeds the fuzzer chooses: +// go test ./internal/convert -run '^$' -fuzz FuzzTableProperty -fuzztime 60s +func FuzzTableProperty(f *testing.F) { + f.Add(uint64(0)) + f.Fuzz(func(t *testing.T, seed uint64) { + if msg := newPropertyEnv(t).check(seed); msg != "" { + t.Fatal(msg) + } + }) +} + +// TestTablePropertyCorpus runs the check over real tables: a file of storage +// fragments, one table per record separated by a line holding only "\x00", +// named by MF_TABLE_CORPUS. Skipped without it; the corpus is page content and +// is never committed. +func TestTablePropertyCorpus(t *testing.T) { + path := os.Getenv("MF_TABLE_CORPUS") + if path == "" { + t.Skip("MF_TABLE_CORPUS not set") + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + env := newPropertyEnv(t) + failed := 0 + tables := strings.Split(string(data), "\n\x00\n") + for i, s := range tables { + if strings.TrimSpace(s) == "" { + continue + } + if msg := env.checkStorage(fmt.Sprintf("corpus table %d", i), s); msg != "" { + failed++ + t.Error(msg) + } + } + t.Logf("%d tables, %d failed", len(tables), failed) +} + +// propertyEnv is a documentation root holding the image the generator refers +// to, so a published image is not IMAGE BROKEN. +type propertyEnv struct { + t testing.TB + dir string + root *project.Root + index *linkindex.Index +} + +func newPropertyEnv(t testing.TB) *propertyEnv { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "d.png"), []byte("png"), 0o644); err != nil { + t.Fatal(err) + } + root, err := project.FromPath(dir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = root.FS.Close() }) + idx, err := linkindex.Build(root) + if err != nil { + t.Fatal(err) + } + return &propertyEnv{t: t, dir: dir, root: root, index: idx} +} + +func (e *propertyEnv) check(seed uint64) string { + g := &tableGen{r: rand.New(rand.NewPCG(seed, 0x55))} + return e.checkStorage(fmt.Sprintf("seed %d", seed), g.table(0)) +} + +// checkStorage returns "" when storage passes all three checks, and a report +// naming what failed otherwise. +func (e *propertyEnv) checkStorage(name, storage string) string { + fail := func(what string, parts ...string) string { + return fmt.Sprintf("%s: %s\n--- storage ---\n%s\n%s", name, what, storage, strings.Join(parts, "\n")) + } + md, err := StorageToMarkdown(storage, StorageOptions{}) + if err != nil { + return fail("read: " + err.Error()) + } + published, err := e.publish(md) + if err != nil { + return fail("publish: "+err.Error(), "--- markdown ---", md) + } + again, err := StorageToMarkdown(published, StorageOptions{}) + if err != nil { + return fail("second read: "+err.Error(), "--- markdown ---", md, "--- published ---", published) + } + if again != md { + return fail("the Markdown is not a fixed point", + "--- markdown ---", md, "--- published ---", published, "--- read again ---", again) + } + before, err := modelOf(storage) + if err != nil { + return fail("model: " + err.Error()) + } + after, err := modelOf(published) + if err != nil { + return fail("model of published: " + err.Error()) + } + if before != after { + return fail("publishing the Markdown changed the table", + "--- markdown ---", md, "--- published ---", published, + "--- model before ---", before, "--- model after ---", after) + } + return "" +} + +func (e *propertyEnv) publish(md string) (string, error) { + f, err := frontmatter.Parse(filepath.Join(e.dir, "main.md"), md) + if err != nil { + return "", err + } + page, err := MdToConfluence(f, e.root, e.index, "https://wiki.example.net", "ENG") + if err != nil { + return "", err + } + if len(page.Broken) > 0 { + return "", fmt.Errorf("broken: %v", page.Broken) + } + return page.HTML, nil +} + +// --- the model ----------------------------------------------------------------- + +// modelOf renders every table in storage as a canonical string of what takes +// effect in Confluence (docs/confluence/storage-format.md), ignoring what read +// may legitimately change: +// +// - server-generated ids; +// - data-table-width, and data-table-display-mode="default"; +// - no data-layout, which reads as align-start (D2); +// - a of pixel widths on an align-start table (the editor's +// measurement, which read ignores); +// - thead/tbody/tfoot: Confluence renders every row the same way; +// - a span of 1; +// - where an alignment is written: each paragraph's effective alignment is +// modelled, so a cell style and a paragraph style that say the same are +// the same, and left, start and justify are no alignment; +// - loose inline content in a cell, which is a paragraph; +// - a cell holding nothing but empty paragraphs, which is an empty cell; +// - b/i/s/strike, which are strong/em/del. +func modelOf(storage string) (string, error) { + root, err := parseStorage(storage) + if err != nil { + return "", err + } + var b strings.Builder + var walk func(n *snode) + walk = func(n *snode) { + for _, k := range n.kids { + if k.name == "table" { + modelTable(&b, k, "") + continue + } + walk(k) + } + } + walk(root) + return b.String(), nil +} + +func modelTable(b *strings.Builder, t *snode, indent string) { + attrs := map[string]string{} + for k, v := range t.attrs { + attrs[k] = v + } + layout := attrs["data-layout"] + delete(attrs, "data-table-width") + if attrs["data-table-display-mode"] == "default" { + delete(attrs, "data-table-display-mode") + } + if layout == "" { + attrs["data-layout"] = "align-start" + } + fmt.Fprintf(b, "%stable%s\n", indent, modelAttrs(attrs)) + for _, sec := range t.kids { + switch sec.name { + case "colgroup": + if layout == "align-start" && pixelColgroup(sec) { + continue + } + var cols []string + for _, c := range sec.kids { + if c.name == "col" { + cols = append(cols, "col"+modelAttrs(c.attrs)) + } + } + fmt.Fprintf(b, "%s colgroup %s\n", indent, strings.Join(cols, " ")) + case "thead", "tbody", "tfoot": + for _, tr := range sec.kids { + if tr.name == "tr" { + modelRow(b, tr, indent+" ") + } + } + case "tr": + modelRow(b, sec, indent+" ") + } + } +} + +func modelRow(b *strings.Builder, tr *snode, indent string) { + fmt.Fprintf(b, "%srow%s\n", indent, modelAttrs(tr.attrs)) + for _, c := range tr.kids { + if c.name != "th" && c.name != "td" { + continue + } + attrs := map[string]string{} + for k, v := range c.attrs { + attrs[k] = v + } + for _, k := range []string{"rowspan", "colspan"} { + if strings.TrimSpace(attrs[k]) == "1" { + delete(attrs, k) + } + } + if v, ok := attrs["data-highlight-colour"]; ok { + attrs["data-highlight-colour"] = strings.ToLower(v) + } + cellAlign, _ := styleAlign(attrs["style"]) + if onlyTextAlign(attrs["style"]) { + delete(attrs, "style") + } + fmt.Fprintf(b, "%s %s%s\n", indent, c.name, modelAttrs(attrs)) + for _, blk := range modelBlocks(c, cellAlign, indent+" ") { + b.WriteString(blk) + } + } +} + +// modelBlocks models a cell's content as blocks. An empty cell -- nothing but +// empty paragraphs -- has none. +func modelBlocks(c *snode, cellAlign, indent string) []string { + if emptyCell(c) { + return nil + } + var out []string + var run []*snode + flush := func() { + if s := modelInline(run); strings.TrimSpace(s) != "" { + out = append(out, fmt.Sprintf("%sp[%s] %s\n", indent, cellAlign, strings.TrimSpace(s))) + } + run = nil + } + for _, k := range c.kids { + switch k.name { + case "", "strong", "b", "em", "i", "code", "del", "s", "strike", "a", "ac:image", "br", "ac:link": + run = append(run, k) + case "p": + flush() + a, declared := styleAlign(k.attrs["style"]) + if !declared { + a = cellAlign + } + text := strings.TrimSpace(modelInline(k.kids)) + if strings.Trim(text, "⏎") == "" { + a = "" // an empty paragraph's alignment shows nothing + } + out = append(out, fmt.Sprintf("%sp[%s] %s\n", indent, a, text)) + case "ul", "ol": + flush() + out = append(out, indent+modelList(k)+"\n") + case "table": + flush() + var b strings.Builder + modelTable(&b, k, indent) + out = append(out, b.String()) + case "ac:structured-macro": + flush() + out = append(out, fmt.Sprintf("%smacro %s %s %q\n", indent, k.attrs["ac:name"], + macroParam(k, "language"), textContent(findChild(k, "ac:plain-text-body")))) + default: + flush() + out = append(out, fmt.Sprintf("%s%s %s\n", indent, k.name, strings.TrimSpace(modelInline(k.kids)))) + } + } + flush() + return joinParagraphs(out, indent) +} + +// joinParagraphs joins adjacent paragraphs of one alignment with a line break. +// A pipe cell holds its lines on one row joined by
, which publishes as +// line breaks in one paragraph where the page had a paragraph per line: the +// two look the same in a cell, and markfluence has always mapped them so. +func joinParagraphs(blocks []string, indent string) []string { + var out []string + for _, blk := range blocks { + if n := len(out); n > 0 { + prev, cur := out[n-1], blk + if pa, pt, ok := paraParts(prev, indent); ok { + if ca, ct, ok := paraParts(cur, indent); ok { + // An empty line has no alignment of its own to disagree with. + switch { + case strings.Trim(pt, "⏎") == "": + pa = ca + case strings.Trim(ct, "⏎") == "": + ca = pa + } + if pa == ca { + out[n-1] = fmt.Sprintf("%sp[%s] %s\n", indent, pa, pt+"⏎"+ct) + continue + } + } + } + } + out = append(out, blk) + } + return out +} + +// paraParts splits a modelled paragraph into its alignment and text. +func paraParts(blk, indent string) (string, string, bool) { + rest, ok := strings.CutPrefix(blk, indent+"p[") + if !ok { + return "", "", false + } + align, text, ok := strings.Cut(strings.TrimSuffix(rest, "\n"), "] ") + return align, text, ok +} + +func macroParam(m *snode, name string) string { + for _, k := range m.kids { + if k.name == "ac:parameter" && k.attrs["ac:name"] == name { + return textContent(k) + } + } + return "" +} + +func modelList(l *snode) string { + var items []string + for _, li := range l.kids { + if li.name == "li" { + var parts []string + for _, k := range li.kids { + if k.name == "p" { + parts = append(parts, strings.TrimSpace(modelInline(k.kids))) + } else { + parts = append(parts, strings.TrimSpace(modelInline([]*snode{k}))) + } + } + items = append(items, strings.Join(parts, " ")) + } + } + return l.name + "[" + strings.Join(items, "; ") + "]" +} + +var inlineAlias = map[string]string{"b": "strong", "i": "em", "s": "del", "strike": "del"} + +// modelInline renders inline content with its marks, collapsing whitespace. +func modelInline(kids []*snode) string { + var b strings.Builder + for _, k := range kids { + switch k.name { + case "": + b.WriteString(k.text) + case "br": + b.WriteString("⏎") + case "a": + fmt.Fprintf(&b, "%s", k.attrs["href"], modelInline(k.kids)) + case "ac:image": + f := "" + if att := findChild(k, "ri:attachment"); att != nil { + f = att.attrs["ri:filename"] + } + fmt.Fprintf(&b, "", f) + default: + name := k.name + if a, ok := inlineAlias[name]; ok { + name = a + } + fmt.Fprintf(&b, "<%s>%s", name, modelInline(k.kids), name) + } + } + s := whitespaceRunRE.ReplaceAllString(b.String(), " ") + // Whitespace beside a line break shows nothing, and publishing writes a + // newline after every
. + return strings.ReplaceAll(strings.ReplaceAll(s, " ⏎", "⏎"), "⏎ ", "⏎") +} + +func modelAttrs(attrs map[string]string) string { + var keys []string + for k := range attrs { + if !isID(k) && !droppedAttrs[k] { + keys = append(keys, k) + } + } + sort.Strings(keys) + var b strings.Builder + for _, k := range keys { + fmt.Fprintf(&b, " %s=%q", k, normalizeWidths(attrs[k])) + } + return b.String() +} + +var pointZeroRE = regexp.MustCompile(`(\d)\.0px`) + +// normalizeWidths reads 300px and 300.0px as one width: Confluence rewrites +// the first as the second on write. +func normalizeWidths(v string) string { return pointZeroRE.ReplaceAllString(v, "${1}px") } + +// --- the generator --------------------------------------------------------------- + +// 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 (