Skip to content

fix(converter): emit > **MARKER** form to close admonition round-trip#136

Merged
pchuri merged 2 commits into
mainfrom
fix/admonition-roundtrip-multi-paragraph
Apr 29, 2026
Merged

fix(converter): emit > **MARKER** form to close admonition round-trip#136
pchuri merged 2 commits into
mainfrom
fix/admonition-roundtrip-multi-paragraph

Conversation

@pchuri
Copy link
Copy Markdown
Owner

@pchuri pchuri commented Apr 29, 2026

Description

Closes #135.

storageToMarkdown emitted the bare [!info] shorthand for callout macros, but the bare form has no explicit body terminator. A blank line inside a multi-paragraph body is therefore indistinguishable from the body ending, so re-uploading downloaded markdown silently dropped every paragraph after the first — content escaped the macro on round-trip.

Reproduction (pre-fix)

Author writes the README-recommended form:

> **INFO**
> foo
>
> bar

markdownToStorage correctly puts both paragraphs inside <ac:rich-text-body>.
storageToMarkdown then collapses to:

[!info]
foo

bar

Re-uploading hits lib/macro-converter.js's preprocessor (?=\n\s*\n|...) lookahead, which treats the blank line as the end of the body. <p>bar</p> lands outside the macro.

Fix

Switch the storage→markdown output to the > **MARKER** blockquote form that README already documents as the canonical authored shape. The > prefix gives the body an explicit boundary, so markdownToStorage — which already handles this form correctly via the blockquote handler (PR #132) — preserves every paragraph on re-upload.

markdownToStorage still accepts the bare [!marker] form on input for backwards compatibility with already-downloaded files.

The three near-duplicate per-marker handlers (info / warning / note) collapse into a single CALLOUT_MARKERS loop, matching the pattern already used elsewhere in this file (PRs #132 and #134).

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

Testing

  • Tests pass locally with my changes (391 passing, was 385)
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

6 new tests in a storageToMarkdown callout round-trip describe block:

The pre-existing should convert Confluence macros to admonitions test in confluence-client.test.js is updated to assert the new output form.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Note on relationship to #132 / #134

This is the third bug in the same callout code path:

The author of #135 explicitly noted they discovered the bug while verifying #134.

… round-trip

The bare `[!info]` shorthand `storageToMarkdown` emitted has no explicit
body terminator. A blank line inside a multi-paragraph body is therefore
indistinguishable from the body ending, so re-uploading downloaded
markdown drops every paragraph after the first — content silently
escapes the macro on round-trip.

Switch the storage→markdown output to the `> **MARKER**` blockquote form
that README already documents as the canonical authored shape. The `>`
prefix gives the body an explicit boundary, so `markdownToStorage`
(which already handles this form correctly via the blockquote handler)
preserves every paragraph on re-upload.

`markdownToStorage` still accepts the bare `[!marker]` form on input for
backwards compatibility with already-downloaded files.

Closes #135.
@pchuri pchuri self-assigned this Apr 29, 2026
Without separator newlines, an adjacent following `<p>after</p>` lazy-
continues into the emitted blockquote and lands inside the macro on
re-upload. Markdown blockquotes have no closing delimiter — only a blank
line ends them — so a callout adjacent to surrounding prose fused with
the next paragraph after round-trip.

Wrap the emitted form in `\n…\n` to combine with the single `\n` that
htmlToMarkdown emits around adjacent `<p>` tags, producing blank-line
separation. Same convention `code` and `mermaid` macros already use.

Adds two regression guards: full round-trip with surrounding paragraphs
preserves macro boundaries, and the downloaded markdown has blank lines
on both sides of the callout.

Also updates README to document the new output form (the bare `[!info]`
shorthand is still accepted on input for backwards compatibility).
@pchuri
Copy link
Copy Markdown
Owner Author

pchuri commented Apr 29, 2026

Self-review follow-up: adjacency fix added

On re-review, the initial commit had a latent bug: the emitted > **MARKER** blockquote was not wrapped in separator newlines, so an adjacent following <p>after</p> would lazy-continue into the blockquote body on re-upload (markdown blockquotes have no closing delimiter — only a blank line ends them).

Reproduction (pre-followup):

storage:  <p>before</p><macro>foo,bar</macro><p>after</p>
                  ↓ download
markdown: before
          > **INFO**
          > foo
          >
          > bar
          after            ← lazy-continues
                  ↓ re-upload
storage:  <macro>foo, bar+after fused</macro>   ← `after` lands inside!

Follow-up commit (c34b386) wraps the output with leading + trailing \n — same convention code and mermaid macros already use (confluence-client.test.js:893-918). The single \n combines with the single \n that htmlToMarkdown emits around adjacent <p> tags to produce blank-line separation.

Adds two regression guards in the new callout round-trip describe block:

  • full round-trip with surrounding paragraphs preserves macro boundaries
  • downloaded markdown has blank lines on both sides of the callout

Also updates the README's reverse-direction documentation to match the new output form. The bare [!info] shorthand is still accepted on input for backwards compatibility with already-downloaded files.

Test count: 391 → 393 passing.

@pchuri pchuri merged commit d5a2d9f into main Apr 29, 2026
6 checks passed
@pchuri pchuri deleted the fix/admonition-roundtrip-multi-paragraph branch April 29, 2026 05:27
github-actions Bot pushed a commit that referenced this pull request Apr 29, 2026
## [2.1.3](v2.1.2...v2.1.3) (2026-04-29)

### Bug Fixes

* **converter:** emit `> **MARKER**` form to close admonition round-trip ([#136](#136)) ([d5a2d9f](d5a2d9f)), closes [#135](#135)
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 2.1.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

devlimits added a commit to devlimits/confluence-cli that referenced this pull request Apr 29, 2026
…rser2 walker

Markdown→storage and html→storage conversion was driven by a 100+ line
regex pipeline over the rendered HTML. Nested blockquotes hit the lazy
`<blockquote>(.*?)</blockquote>` matcher's wrong closing tag, code-block
CDATA literals containing `<blockquote>` confused the same matcher, and
most recent fix commits in this area (pchuri#132, pchuri#134, pchuri#136) were workarounds
for that structural fragility. Mirrors PR pchuri#137's storage-walker approach
for the opposite direction.

Introduce `lib/tree-storage-walker.js`: parses with htmlparser2
(`decodeEntities: false` so text and attribute entities round-trip
byte-identical), then walks the DOM dispatching by node type. Handlers
for headings, paragraphs, inline formatting, lists (with V1's `<p>`-wrap
quirk on tight items), code blocks (CDATA + language + `]]>` split) and
inline code, links (smart/plain/wiki + anchor `#id` branch), tables,
blockquote with INFO/WARNING/NOTE marker detection, TOC/ANCHOR/EXPAND
paragraph markers, and void tags.

`htmlToConfluenceStorage` shrinks from 173 lines to 2, delegating to
`convertTree`. `markdownToStorage` and `markdownToNativeStorage` route
through it, so all three conversion methods share the walker backend.
The depth-tracking `convertBlockquotes`/`convertBlockquoteContent`
helpers become unnecessary and are removed; nesting is the tree, not
regex ordering.

The markdown-it preprocessor that rewrites `[!info]` shorthand into
the canonical `> **INFO**` blockquote form stays in place — it operates
on markdown source where line-break information is needed to support
the single-line `[!info] body` form, which tree-level handling would
silently drop.

- Add htmlparser2 ^8.0.2 as an explicit dependency (was transitive via
  html-to-text).
- 463 tests pass (398 pre-existing + 19 new golden fixtures + 46 walker
  unit tests); lint clean.
- New `tests/fixtures/` (16 markdown + 3 html cases with expected
  `.storage` output, sidecar `.config.json` for link-style variants)
  + `scripts/fixture-ab-diff.js` (`npm run fixtures:diff`) acts as the
  byte-level regression net for any future walker change.
- Adds `.editorconfig` (2-space indent, LF, trim trailing whitespace)
  so editors that respect it pick up the project's existing
  ESLint-enforced style automatically.
devlimits added a commit to devlimits/confluence-cli that referenced this pull request Apr 29, 2026
…rser2 walker

Markdown→storage and html→storage conversion was driven by a 100+ line
regex pipeline over the rendered HTML. Nested blockquotes hit the lazy
`<blockquote>(.*?)</blockquote>` matcher's wrong closing tag, code-block
CDATA literals containing `<blockquote>` confused the same matcher, and
most recent fix commits in this area (pchuri#132, pchuri#134, pchuri#136) were workarounds
for that structural fragility. Mirrors PR pchuri#137's storage-walker approach
for the opposite direction.

Introduce `lib/tree-storage-walker.js`: parses with htmlparser2
(`decodeEntities: false` so text and attribute entities round-trip
byte-identical), then walks the DOM dispatching by node type. Handlers
for headings, paragraphs, inline formatting, lists (with V1's `<p>`-wrap
quirk on tight items), code blocks (CDATA + language + `]]>` split) and
inline code, links (smart/plain/wiki + anchor `#id` branch), tables,
blockquote with INFO/WARNING/NOTE marker detection, TOC/ANCHOR/EXPAND
paragraph markers, and void tags.

`htmlToConfluenceStorage` shrinks from 173 lines to 2, delegating to
`convertTree`. `markdownToStorage` and `markdownToNativeStorage` route
through it, so all three conversion methods share the walker backend.
The depth-tracking `convertBlockquotes`/`convertBlockquoteContent`
helpers become unnecessary and are removed; nesting is the tree, not
regex ordering.

The markdown-it preprocessor that rewrites `[!info]` shorthand into
the canonical `> **INFO**` blockquote form stays in place — it operates
on markdown source where line-break information is needed to support
the single-line `[!info] body` form, which tree-level handling would
silently drop.

- Add htmlparser2 ^8.0.2 as an explicit dependency (was transitive via
  html-to-text).
- 448 tests pass; lint clean. The existing macro-converter tests act as
  the regression net (mirrors PR pchuri#137's testing approach), augmented by
  five nested-blockquote balance tests covering the original `> > **INFO**`
  unbalance bug and four integration smoke tests covering the
  CDATA-adjacent-marker case, the public `htmlToConfluenceStorage` path,
  task-list literal-text fall-through, and table-cell `<p>` wrap.
- New `tests/tree-storage-walker.test.js` (46 tests) covers each handler
  in isolation with synthetic HTML inputs, independent of the markdown-it
  serialization shape.
- Adds `.editorconfig` (2-space indent, LF, trim trailing whitespace)
  so editors that respect it pick up the project's existing
  ESLint-enforced style automatically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG/DISCUSS] Admonition body multi-paragraph round-trip drops content outside the macro

1 participant