Skip to content

fix: preserve the SNIPSTART marker's indentation when splicing - #97

Merged
lennessyy merged 2 commits into
mainfrom
fix/preserve-marker-indentation
Jul 29, 2026
Merged

fix: preserve the SNIPSTART marker's indentation when splicing#97
lennessyy merged 2 commits into
mainfrom
fix/preserve-marker-indentation

Conversation

@lennessyy

Copy link
Copy Markdown
Contributor

What was changed

spliceFile wrote snippet.fmt() output verbatim, always starting at column 0. A SNIPSTART marker does not have to be at column 0 — in Markdown it can be nested inside a list item, and in MDX inside a JSX element. Writing at column 0 under an indented marker ends the enclosing block.

This re-applies the marker's own leading whitespace to every non-blank line written between the markers.

Why

Two failure modes, one silent and one fatal.

Silent: in a plain Markdown list, a fenced code block at column 0 closes the list item. One <ol> becomes two. Numbering still looks right because the source keeps its literal ordinals (2. renders as <ol start=2>), so nobody noticed.

Fatal: in MDX, if a JSX element spans the marker, that element is never closed and compilation fails. On temporalio/documentation this surfaces as:

Error: MDX compilation failed for .../external-storage.mdx
Cause: Expected a closing tag for `<TabItem>` (70:6-70:52) before the end of `listItem`

Any page with <Tabs> inside a numbered procedure step hits this, which currently blocks wiring those pages to snipsync at all.

Details

  • Blank lines are left alone, so no trailing whitespace is introduced.
  • Relative indentation inside the snippet is untouched; the existing dedent behavior is unchanged.
  • Markers already at column 0 get an empty prefix, so existing targets are byte-for-byte unchanged. This is why the whole existing suite passes without fixture updates.

How this was tested

Three new tests in test/sync.test.js: indented marker inherits indentation (fence, source link, code, and relative indent all checked, plus no trailing whitespace), column-zero marker is unaffected, and re-running is idempotent.

npx jest — 38/38 pass. npx eslint . --ext .js — 0 errors, same 12 pre-existing warnings as main.

Also verified end to end against the temporalio/documentation tree with real origins:

  • the previously failing page builds, and its procedure renders as a single <ol> with tabs and both code blocks intact
  • the Go and Python external storage pages go from ['<ol>', '<ol start=2>'] back to ['<ol>']
  • no markers leak into the rendered HTML

Docs updates needed?

No. Behavior only changes for markers that are not at column 0, which previously could not be used reliably.

A SNIPSTART marker does not have to sit at column 0. In Markdown it can be
nested inside a list item, and in MDX inside a JSX element. spliceFile wrote
snippet.fmt() output verbatim, always starting at column 0, which ends the
enclosing block:

- In a plain Markdown list, the fenced block closes the list item, splitting
  one <ol> into two. Numbering still renders because the source keeps its
  literal ordinals, so this went unnoticed.
- In MDX, if a JSX element spans the marker, the element is never closed and
  compilation fails. In the Temporal docs this shows up as
  "Expected a closing tag for <TabItem> before the end of listItem" on any
  page with tabs inside a numbered step.

Re-apply the marker's own leading whitespace to every non-blank line written
between the markers. Blank lines are left alone so no trailing whitespace is
introduced, and relative indentation inside the snippet is untouched.

Markers already at column 0 get an empty prefix, so existing targets are
byte-for-byte unchanged.

Verified against the temporalio/documentation tree: the previously failing
page now builds, its procedure list renders as a single <ol>, and the Go and
Python external storage pages go from two <ol> elements back to one.
@lennessyy
lennessyy requested a review from Duncanma July 29, 2026 22:26
@lennessyy
lennessyy requested a review from a team as a code owner July 29, 2026 22:44
@lennessyy
lennessyy merged commit 4e1cd2d into main Jul 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants