diff --git a/CHANGELOG.md b/CHANGELOG.md index 63181cc..a0c2769 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +- 🐛 FIX: `button-ref` no longer strips whitespace from reference targets, so + multi-word labels (e.g. from `autosectionlabel`) resolve correctly + ({pr}`281`, {issue}`110`) +- 🐛 FIX: `button-ref` now renders rich/nested inline content (emphasis, icons, + etc.) identically to `button-link`, instead of flattening it to plain text + during cross-reference resolution ({pr}`281`, {issue}`228`) - 🐛 FIX: Inline icon roles no longer leak SVG markup into toctree labels and the search index ({pr}`279`, {issue}`99`) - 🐛 FIX: `article-info` octicons regain their `sd-pr-2` spacing class (previously silently dropped by the HTML writer) ({pr}`279`) - 🐛 FIX: Paragraphs inside dropdowns no longer have user classes overwritten by `sd-card-text`, and card/dropdown body styling is applied only to direct child paragraphs, not nested content ({pr}`278`, {issue}`40`) diff --git a/docs/badges_buttons.md b/docs/badges_buttons.md index c688587..d3cbf22 100644 --- a/docs/badges_buttons.md +++ b/docs/badges_buttons.md @@ -105,8 +105,8 @@ Reference Button text ```` ````` -Note that by default sphinx converts the content of references to raw text. -For example `**Bold text**` with `ref-type` set to `ref` will be rendered without bold: +The content of a `button-ref` supports rich inline formatting (such as emphasis +and icons), which is rendered in the button, just as it is for `button-link`: ```{button-ref} buttons :ref-type: ref @@ -115,14 +115,11 @@ For example `**Bold text**` with `ref-type` set to `ref` will be rendered withou **Bold text** ``` -However, if using [myst-parser](https://myst-parser.readthedocs.io/), you can set the `ref-type` to `myst`, and the content will be properly rendered: +Reference targets may also contain spaces, for example the labels that +`sphinx.ext.autosectionlabel` generates from section titles. -```{button-ref} buttons -:ref-type: myst -:color: primary - -**Bold text** -``` +When using [myst-parser](https://myst-parser.readthedocs.io/), you can also set +`ref-type` to `myst` to resolve Markdown-style references. Use the `click-parent` option to make the button's parent container also clickable. diff --git a/sphinx_design/badges_buttons.py b/sphinx_design/badges_buttons.py index 1dac332..1d228fa 100644 --- a/sphinx_design/badges_buttons.py +++ b/sphinx_design/badges_buttons.py @@ -1,7 +1,11 @@ +from typing import Any + from docutils import nodes from docutils.parsers.rst import directives from sphinx import addnodes from sphinx.application import Sphinx +from sphinx.transforms.post_transforms import SphinxPostTransform +from sphinx.util import ws_re from sphinx.util.docutils import ReferenceRole, SphinxRole from sphinx_design.shared import SEMANTIC_COLORS, SdDirective, make_choice, text_align @@ -41,6 +45,8 @@ def setup_badges_and_buttons(app: Sphinx) -> None: app.add_directive(DIRECTIVE_NAME_BUTTON_LINK, ButtonLinkDirective) app.add_directive(DIRECTIVE_NAME_BUTTON_REF, ButtonRefDirective) + app.add_post_transform(ButtonRefContentStash) + app.add_post_transform(ButtonRefContentGraft) def create_bdg_classes(color: str | None, outline: bool) -> list[str]: @@ -147,6 +153,11 @@ class _ButtonDirective(SdDirective): "class": directives.class_option, } + @staticmethod + def get_target(argument: str) -> str: + """Normalise the directive argument into a reference target.""" + raise NotImplementedError + def create_ref_node( self, rawtext: str, target: str, explicit_title: bool, classes: list[str] ) -> nodes.Node: @@ -155,7 +166,7 @@ def create_ref_node( def run_with_defaults(self) -> list[nodes.Node]: rawtext = self.arguments[0] - target = directives.uri(rawtext) + target = self.get_target(rawtext) classes = ["sd-sphinx-override", "sd-btn", "sd-text-wrap"] if "color" in self.options: if "outline" in self.options: @@ -211,6 +222,11 @@ def run_with_defaults(self) -> list[nodes.Node]: class ButtonLinkDirective(_ButtonDirective): """A button directive with an external link.""" + @staticmethod + def get_target(argument: str) -> str: + """Return the URI, with all whitespace removed (as for a normal link).""" + return directives.uri(argument) + def create_ref_node( self, rawtext: str, target: str, explicit_title: bool, classes: list[str] ) -> nodes.Node: @@ -225,13 +241,29 @@ def create_ref_node( class ButtonRefDirective(_ButtonDirective): """A button directive with an internal link.""" + @staticmethod + def get_target(argument: str) -> str: + """Return the reference target, collapsing internal whitespace. + + This mirrors :class:`~sphinx.roles.XRefRole`'s whitespace + normalisation, so that multi-word labels (e.g. those generated by + ``sphinx.ext.autosectionlabel`` from section titles) are preserved and + resolve correctly, rather than being stripped of all whitespace. + """ + return ws_re.sub(" ", argument).strip() + def create_ref_node( self, rawtext: str, target: str, explicit_title: bool, classes: list[str] ) -> nodes.Node: """Create the reference node.""" ref_type = self.options.get("ref-type", "any") + if ref_type == "ref": + # match sphinx's :ref: role, which is ``XRefRole(lowercase=True)``: + # std-domain labels are stored lowercased, so lowercase the target + # too. ``doc``/``myst`` targets are case-sensitive, and the ``any`` + # resolver lowercases internally where needed + target = target.lower() options = { - # TODO the presence of classes raises an error if the link cannot be found "classes": classes, "reftarget": target, "refdoc": self.env.docname, @@ -241,3 +273,99 @@ def create_ref_node( "refwarn": True, } return addnodes.pending_xref(rawtext, **options) + + +_BUTTON_REF_STASH_ATTR = "sd_button_ref_content" +"""Name of the (transient) python attribute on the ``document`` object, +mapping marker class names to stashed ``button-ref`` content nodes.""" + +_BUTTON_REF_MARKER_PREFIX = "sd-button-ref-content-" +"""Prefix of the transient marker classes used to correlate a ``button-ref`` +``pending_xref`` with its resolved node; never present in final output.""" + + +class ButtonRefContentStash(SphinxPostTransform): + """Stash the rich content of each ``button-ref`` cross-reference. + + Sphinx's standard-domain resolvers rebuild an explicit-title ``ref``/``doc`` + cross-reference from ``node.astext()``, which flattens any nested inline + markup (emphasis, icons, ...) down to plain text (see issue #228). Since + ``button-ref`` allows arbitrary parsed content, we stash a copy of that + content before any resolver runs, tagging the ``pending_xref`` with a + unique marker class. Whatever node the xref is replaced with inherits the + marker (docutils ``replace_self`` copies ids/classes onto the + replacement), letting :class:`ButtonRefContentGraft` restore the content + afterwards, without relying on any non-public resolver API. + """ + + # must run before every cross-reference resolver: the built-in + # ``ReferencesResolver`` runs at priority 10, and myst-parser's resolver + # (which handles ``ref-type: myst`` buttons) at priority 9 + default_priority = 8 + + def run(self, **kwargs: Any) -> None: + """Tag and stash every ``button-ref`` pending cross-reference.""" + stash: dict[str, nodes.Element] = {} + for index, node in enumerate(self.document.findall(addnodes.pending_xref)): + if "sd-btn" not in node.get("classes", ()): + continue + if not node.get("refexplicit"): + # the directive had no content: the resolver-generated text + # (e.g. the target's section title) is the desired button text + continue + if next(iter(node[0].findall(addnodes.pending_xref)), None) is not None: + # the button content itself contains a cross-reference: a + # stashed copy would re-insert an unresolved ``pending_xref`` + # after every resolver has run, crashing the writer. Fall back + # to the pre-existing behaviour (the std domain flattens the + # content to plain text) - a link nested inside a button link + # would be invalid HTML anyway + continue + marker = f"{_BUTTON_REF_MARKER_PREFIX}{index}" + node["classes"].append(marker) + stash[marker] = node[0].deepcopy() + # a plain (transient) python attribute: unlike a node attribute it can + # never leak into pickled doctrees or (XML) serialisations + setattr(self.document, _BUTTON_REF_STASH_ATTR, stash) + + +class ButtonRefContentGraft(SphinxPostTransform): + """Restore stashed ``button-ref`` content onto resolved references. + + Counterpart to :class:`ButtonRefContentStash`: locate the node that + inherited each marker class from its ``pending_xref``, strip the marker, + and, for resolved references, replace the (possibly text-flattened) + resolver-built content with the stashed rich content, so that + ``button-ref`` renders its content identically to ``button-link``. + """ + + # run just after the built-in ``ReferencesResolver`` (priority 10) + default_priority = 11 + + def run(self, **kwargs: Any) -> None: + """Graft stashed content onto each marked node.""" + stash: dict[str, nodes.Element] = getattr( + self.document, _BUTTON_REF_STASH_ATTR, {} + ) + if not stash: + return + for element in list(self.document.findall(nodes.Element)): + classes = element.get("classes", []) + for marker in [cls for cls in classes if cls in stash]: + classes.remove(marker) + if isinstance(element, nodes.reference): + # a resolver replaced the xref with a reference, possibly + # rebuilding its content from plain text (the std domain + # does, for explicit-title ref/doc targets): restore the + # stashed rich content (a no-op in effect for resolvers + # that already preserved the original content node) + element.children = [] + element.append(stash[marker].deepcopy()) + # otherwise the xref was unresolved, and the built-in resolver + # substituted the original (still rich) content node itself, + # which inherited the marker: stripping the marker is enough. + # If an (external) resolver produced a node that did not + # inherit the marker at all, nothing is found here and the + # graft is a no-op: such resolvers build on the passed + # contnode, which was never flattened by the std domain. + setattr(self.document, _BUTTON_REF_STASH_ATTR, {}) diff --git a/tests/test_buttons.py b/tests/test_buttons.py new file mode 100644 index 0000000..24318c3 --- /dev/null +++ b/tests/test_buttons.py @@ -0,0 +1,342 @@ +"""Tests for the ``button-link`` and ``button-ref`` directives. + +Covers: + +* #110 - ``button-ref`` targets containing whitespace (e.g. the labels that + ``sphinx.ext.autosectionlabel`` generates from section titles) must be + preserved, not stripped, so that multi-word labels resolve. +* #228 - rich/nested inline content (emphasis, icons, ...) inside a + ``button-ref`` must render identically to the same content inside a + ``button-link``, rather than being flattened to plain text. +* An unresolved ``button-ref`` target must emit a normal missing-reference + warning and still build (no traceback). +""" + +from docutils import nodes +import pytest + +from sphinx_design.badges_buttons import ( + _BUTTON_REF_MARKER_PREFIX, + ButtonLinkDirective, + ButtonRefDirective, +) +from sphinx_design.icons import sd_icon + +try: + import myst_parser # noqa: F401 + + MYST_INSTALLED = True +except ImportError: + MYST_INSTALLED = False + +# guard MyST variants so the ``py311-no-myst`` environment still passes +MYST_PARAM = pytest.param( + "myst", + marks=pytest.mark.skipif(not MYST_INSTALLED, reason="myst-parser not installed"), +) + + +def _build(sphinx_builder, fmt, rst, myst, *, assert_pass=True): + """Build a single-document project written in ``rst`` or ``myst``.""" + if fmt == "rst": + builder = sphinx_builder( + conf_kwargs={"extensions": ["sphinx_design", "sphinx.ext.autosectionlabel"]} + ) + builder.src_path.joinpath("index.rst").write_text(rst, encoding="utf8") + else: + builder = sphinx_builder( + conf_kwargs={ + "extensions": [ + "myst_parser", + "sphinx_design", + "sphinx.ext.autosectionlabel", + ], + "myst_enable_extensions": ["colon_fence"], + } + ) + builder.src_path.joinpath("index.md").write_text(myst, encoding="utf8") + builder.build(assert_pass=assert_pass) + return builder + + +def test_get_target_normalisation(): + """The two button directives normalise their argument differently. + + ``button-link`` strips *all* whitespace (URLs cannot contain spaces), while + ``button-ref`` only collapses internal whitespace runs to single spaces, + matching Sphinx's own ``XRefRole`` so multi-word labels survive (#110). + """ + assert ButtonLinkDirective.get_target("my article section") == "myarticlesection" + assert ( + ButtonRefDirective.get_target("my article\n section") == "my article section" + ) + assert ButtonRefDirective.get_target(" spaced-label ") == "spaced-label" + + +MULTIWORD = { + "rst": """ +Heading +======= + +My Article Section +------------------ + +Some content. + +.. button-ref:: my article section + :ref-type: ref + + Go to the section +""", + "myst": """ +# Heading + +## My Article Section + +Some content. + +```{button-ref} my article section +:ref-type: ref + +Go to the section +``` +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_button_ref_multiword_target(fmt, sphinx_builder): + """A ``button-ref`` to a multi-word (autosectionlabel) label resolves (#110).""" + builder = _build(sphinx_builder, fmt, MULTIWORD["rst"], MULTIWORD["myst"]) + html = (builder.out_path / "index.html").read_text(encoding="utf8") + # the whitespace-containing target resolves to the section anchor + assert 'href="#my-article-section"' in html + assert "Go to the section" in html + + +NESTED = { + "rst": """ +Heading +======= + +.. _target-section: + +Target Section +-------------- + +.. button-ref:: target-section + :ref-type: ref + + Go *emphasis* :octicon:`heart` end + +.. button-link:: https://example.com + + Go *emphasis* :octicon:`heart` end +""", + "myst": """ +# Heading + +(target-section)= + +## Target Section + +```{button-ref} target-section +:ref-type: ref + +Go *emphasis* {octicon}`heart` end +``` + +```{button-link} https://example.com +Go *emphasis* {octicon}`heart` end +``` +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_button_nested_content(fmt, sphinx_builder): + """Nested inline markup renders the same in ``button-ref`` and + ``button-link`` - it must not be flattened to (escaped) text (#228).""" + builder = _build(sphinx_builder, fmt, NESTED["rst"], NESTED["myst"]) + html = (builder.out_path / "index.html").read_text(encoding="utf8") + # both buttons keep the emphasis and the (real, unescaped) octicon svg + assert html.count("emphasis") == 2 + assert html.count("sd-octicon-heart") == 2 + # the octicon must not leak through as escaped text (the #228 symptom) + assert "<svg" not in html + + # structurally, both resolved references carry the same rich child nodes + doctree = builder.get_doctree("index", post_transforms=True) + refs = [ + ref + for ref in doctree.findall(nodes.reference) + if "sd-btn" in ref.get("classes", []) + ] + assert len(refs) == 2 + for ref in refs: + assert any(isinstance(child, nodes.emphasis) for child in ref.findall()) + # the octicon is an sd_icon inline node (since #279) + assert any(isinstance(child, sd_icon) for child in ref.findall()) + + # the transient stash/graft marker classes must never leak into output + assert _BUTTON_REF_MARKER_PREFIX not in html + assert _BUTTON_REF_MARKER_PREFIX not in doctree.pformat() + + +UNRESOLVED_RST = """ +Heading +======= + +.. button-ref:: nonexistent-label + :ref-type: ref + + Broken *ref* content +""" + + +def test_button_ref_unresolved_warns(sphinx_builder): + """An unresolved ``button-ref`` warns and still builds (no crash), keeping + its (now unlinked) rich content.""" + builder = sphinx_builder(conf_kwargs={"extensions": ["sphinx_design"]}) + builder.src_path.joinpath("index.rst").write_text(UNRESOLVED_RST, encoding="utf8") + # a missing-reference warning is emitted, but the build completes + builder.build(assert_pass=False) + assert "undefined label: 'nonexistent-label'" in builder.warnings + + html = (builder.out_path / "index.html").read_text(encoding="utf8") + # the content survives (unlinked) rather than crashing the build + assert "ref" in html + assert 'href="#nonexistent-label"' not in html + + # the transient stash/graft marker classes must never leak into output, + # even on the unresolved path (where the content node inherits the + # pending_xref's classes) + doctree = builder.get_doctree("index", post_transforms=True) + assert _BUTTON_REF_MARKER_PREFIX not in html + assert _BUTTON_REF_MARKER_PREFIX not in doctree.pformat() + + +PLAIN_RST = """ +Heading +======= + +.. _target-section: + +Target Section +-------------- + +.. button-ref:: target-section + :ref-type: ref + + Single word ref + +.. button-link:: https://example.com + + A link button +""" + + +def test_button_plain_targets_unchanged(sphinx_builder): + """Single-word ref targets and ``button-link`` URLs behave as before (#110 + must not regress the simple cases).""" + builder = sphinx_builder(conf_kwargs={"extensions": ["sphinx_design"]}) + builder.src_path.joinpath("index.rst").write_text(PLAIN_RST, encoding="utf8") + builder.build() # asserts no warnings + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert 'href="#target-section"' in html + assert "Single word ref" in html + assert 'href="https://example.com"' in html + assert "A link button" in html + + +NO_CONTENT_RST = """ +Heading +======= + +.. _target-section: + +My Fancy Section +---------------- + +.. button-ref:: target-section + :ref-type: ref +""" + + +def test_button_ref_without_content_shows_title(sphinx_builder): + """A ``button-ref`` without content displays the resolved target's title + (built by the resolver), which must not be overwritten by the + content-preservation machinery for #228.""" + builder = sphinx_builder(conf_kwargs={"extensions": ["sphinx_design"]}) + builder.src_path.joinpath("index.rst").write_text(NO_CONTENT_RST, encoding="utf8") + builder.build() # asserts no warnings + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert 'href="#target-section"' in html + # the button text is the resolved section title, not the raw target + assert "My Fancy Section" in html + assert _BUTTON_REF_MARKER_PREFIX not in html + + +NESTED_XREF = { + "rst": """ +Heading +======= + +.. _jump-target: + +Target Section +-------------- + +Some content. + +.. button-ref:: jump-target + + Jump to :ref:`jump-target` now +""", + "myst": """ +# Heading + +(jump-target)= + +## Target Section + +Some content. + +```{button-ref} jump-target + +Jump to {ref}`jump-target` now +``` +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_button_ref_nested_xref_no_crash(fmt, sphinx_builder): + """A cross-reference role inside ``button-ref`` content must not crash. + + Such content is deliberately not stash/grafted (re-inserting it after + resolution would leave an unresolved ``pending_xref`` for the writer); + it falls back to the pre-existing std-domain text flattening. + """ + builder = _build(sphinx_builder, fmt, NESTED_XREF["rst"], NESTED_XREF["myst"]) + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert 'href="#jump-target"' in html + assert "Jump to" in html + assert _BUTTON_REF_MARKER_PREFIX not in html + + +def test_button_ref_titlecase_ref_type(sphinx_builder): + """A Title-Case target with ``ref-type: ref`` resolves like ``:ref:`` does. + + Sphinx's ``:ref:`` role lowercases its target (std labels are stored + lowercased), so ``button-ref`` matches it - the visible heading text can + be pasted verbatim as the target. + """ + src = MULTIWORD["rst"].replace( + ".. button-ref:: my article section", + ".. button-ref:: My Article Section", + ) + assert src != MULTIWORD["rst"] + builder = _build(sphinx_builder, "rst", src, MULTIWORD["myst"]) + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert 'href="#my-article-section"' in html