Skip to content

🐛 Fix button-ref target whitespace and nested content#281

Merged
chrisjsewell merged 1 commit into
mainfrom
brief/15-button-ref-targets
Jul 14, 2026
Merged

🐛 Fix button-ref target whitespace and nested content#281
chrisjsewell merged 1 commit into
mainfrom
brief/15-button-ref-targets

Conversation

@chrisjsewell

@chrisjsewell chrisjsewell commented Jul 14, 2026

Copy link
Copy Markdown
Member

Two button-ref bugs, plus a parity improvement:

Changes

  • :ref: does not resolve autosectionlabel #110 — targets no longer whitespace-stripped: both button directives ran their argument through directives.uri, which deletes all whitespace — correct for button-link URLs, but it turned multi-word ref targets (exactly what autosectionlabel generates, e.g. my article section) into myarticlesectionundefined label. Each directive now normalizes its own argument: button-link keeps directives.uri; button-ref collapses whitespace runs like Sphinx's XRefRole does. Additionally, ref-type: ref targets are lowercased to match the :ref: role (XRefRole(lowercase=True) — std labels are stored lowercased), so the visible Title-Case heading text can be pasted verbatim; doc/myst targets stay case-sensitive.
  • Nested parse broken for button-ref but not button-link #228 — rich content preserved through resolution: the std-domain resolvers rebuild explicit-title refs from astext(), flattening nested inline markup (emphasis, icons rendered as escaped text). Fixed with a stash/graft pair of post-transforms using only stable APIs: content is stashed and the pending_xref tagged with a transient marker class before any resolver runs (priority 8 — below myst-parser's resolver at 9 and the built-in at 10), then grafted back onto the resolved reference (priority 11). Buttons whose content itself contains a cross-reference role are deliberately left on the old flatten path — regrafting would re-insert an unresolved pending_xref and crash the writer (and a link inside a button link is invalid HTML anyway).
  • Content-less button-refs keep the resolver-generated section title (guarded by refexplicit).
  • Unresolved button-refs degrade to a standard undefined label warning with unlinked content — no traceback (locking in the hardening from 🐛 Keep buttons intact under gettext translation #270). Docs updated: the "bold text will be rendered without bold" caveat is gone (it now renders), and targets may contain spaces.

Verification

  • New tests/test_buttons.py (11 tests, rst + myst): multi-word target resolution (verified red on the old code: undefined label: 'myarticlesection'), Title-Case + ref-type: ref, nested-content parity with button-link, nested-xref-in-content build safety (verified red — reproduced the writer crash pre-fix), content-less title rendering, unresolved warning path, and marker-leak assertions (HTML + doctree) on every path.
  • Independent adversarial review: marker-leak sweep across HTML/latex/text/xml/gettext outputs and pickled doctrees/env (clean, including incremental rebuilds), resolver-path matrix (std ref/doc, any, myst resolving + failing, missing-ref), sd-btn class output byte-identical to main outside the intended fixes, nitpicky-mode parity.
  • Full suite green on pinned Sphinx 7.2.6 / 7.4.7 / 8.2.3 (145 passed each, versions verified in-venv) and the standard matrix on 9.0.4. An earlier draft used a private resolver API that doesn't exist before Sphinx 8.2 — the final design deliberately uses only stable public mechanics.

Closes #110
Closes #228

@chrisjsewell chrisjsewell force-pushed the brief/15-button-ref-targets branch from 7f2dc96 to eb913e7 Compare July 14, 2026 10:23
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.72%. Comparing base (220407d) to head (926cc8b).

Files with missing lines Patch % Lines
sphinx_design/badges_buttons.py 98.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #281      +/-   ##
==========================================
+ Coverage   90.41%   90.72%   +0.30%     
==========================================
  Files          13       13              
  Lines        1158     1207      +49     
==========================================
+ Hits         1047     1095      +48     
- Misses        111      112       +1     
Flag Coverage Δ
pytests 90.72% <98.00%> (+0.30%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chrisjsewell chrisjsewell force-pushed the brief/15-button-ref-targets branch from eb913e7 to c6137fa Compare July 14, 2026 15:28
button-ref now normalises its target like Sphinx's XRefRole (collapsing
whitespace) instead of stripping it via directives.uri, so multi-word
autosectionlabel labels resolve (#110). A stash/graft post-transform pair
(around the resolvers, using only stable Sphinx/docutils APIs) preserves
rich inline content (emphasis, icons) on resolved button-ref nodes,
matching button-link output rather than flattening to text (#228).
@chrisjsewell chrisjsewell force-pushed the brief/15-button-ref-targets branch from c6137fa to 926cc8b Compare July 14, 2026 15:30
@chrisjsewell chrisjsewell merged commit abbf459 into main Jul 14, 2026
25 checks passed
@chrisjsewell chrisjsewell deleted the brief/15-button-ref-targets branch July 14, 2026 15:33
chrisjsewell added a commit that referenced this pull request Jul 14, 2026
Both `card` (cards.py) and `grid-item-card` (grids.py, via its own
option_spec) ran the `:link:` option through `directives.uri`, which
deletes all whitespace. For `link-type: ref`, a multi-word target such
as an `autosectionlabel` label (e.g. `my section name`) became
`mysectionname`, producing an `undefined label` warning.

The `link` option is now captured verbatim (`unchanged_required`) and
normalised inside `CardDirective.create_card`, where `link-type` is
known: `url` keeps the `directives.uri` behaviour (byte-identical for
URLs), `ref` collapses whitespace runs and lowercases (matching the
`:ref:` role, `XRefRole(lowercase=True)`), and `doc`/`any` collapse
whitespace but preserve case. Completes the fix for #110 (the
`button-ref` path was fixed in #281).
chrisjsewell added a commit that referenced this pull request Jul 14, 2026
Badge roles now accept a trailing `; tooltip` suffix, rendered as a
native HTML `title` attribute (closes #81). The suffix is split on the
last unescaped `;` (a literal `;` is written `\;`), mirroring the icon
roles' string grammar via the new `split_tooltip` helper.

Since `;` is a legal character in URLs and reference targets, the
`bdg-link-*`/`bdg-ref-*` families only accept the suffix after a
complete explicit `title <target>` form (the `;` must follow the
closing `>`); a bare target is never split, so existing targets
containing `;` render byte-identically to before. The `\;` escape is
resolved uniformly for both parsers (docutils NUL-encodes escapes,
MyST forwards backslashes verbatim).

- plain badges use a new `sd_badge` node (a `nodes.inline` whose HTML
  visitor emits `title` when set, with explicit passthrough visitors for
  latex/text/man/texinfo); output without a tooltip is byte-identical to
  before (only the doctree node name changes, hence the badge-basic
  fixture regeneration)
- `bdg-link-*` set `reftitle` on the reference (native `title`)
- `bdg-ref-*` carry the tooltip across cross-reference resolution using
  the #281 marker-class + document-stash pattern (an arbitrary attribute
  does not survive resolution), applied as `reftitle` by a post-transform
  that runs after the resolver; no marker/attribute leaks into output

Also removes a stale `TODO escape HTML` on the button `:tooltip:` option
(the HTML writer already escapes attribute values) and the dead
commented-out `reftitle` code in the link badge role.
chrisjsewell added a commit that referenced this pull request Jul 14, 2026
Badge roles now accept a trailing `; tooltip` suffix, rendered as a
native HTML `title` attribute (closes #81). The suffix is split on the
last unescaped `;` (a literal `;` is written `\;`), mirroring the icon
roles' string grammar via the new `split_tooltip` helper.

Since `;` is a legal character in URLs and reference targets, the
`bdg-link-*`/`bdg-ref-*` families only accept the suffix after a
complete explicit `title <target>` form (the `;` must follow the
closing `>`); a bare target is never split, so existing targets
containing `;` render byte-identically to before. The `\;` escape is
resolved uniformly for both parsers (docutils NUL-encodes escapes,
MyST forwards backslashes verbatim).

- plain badges use a new `sd_badge` node (a `nodes.inline` whose HTML
  visitor emits `title` when set, with explicit passthrough visitors for
  latex/text/man/texinfo); output without a tooltip is byte-identical to
  before (only the doctree node name changes, hence the badge-basic
  fixture regeneration)
- `bdg-link-*` set `reftitle` on the reference (native `title`)
- `bdg-ref-*` carry the tooltip across cross-reference resolution using
  the #281 marker-class + document-stash pattern (an arbitrary attribute
  does not survive resolution), applied as `reftitle` by a post-transform
  that runs after the resolver; no marker/attribute leaks into output

Also removes a stale `TODO escape HTML` on the button `:tooltip:` option
(the HTML writer already escapes attribute values) and the dead
commented-out `reftitle` code in the link badge role.
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.

Nested parse broken for button-ref but not button-link :ref: does not resolve autosectionlabel

1 participant