From ba3f24494c590bc5a20b61ad32397333119a3d54 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 14 Jul 2026 15:58:58 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20card=20:link:=20whitespace?= =?UTF-8?q?=20stripping=20for=20ref=20targets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- CHANGELOG.md | 1 + sphinx_design/cards.py | 55 ++++++-- sphinx_design/grids.py | 4 +- tests/test_card_link.py | 275 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 325 insertions(+), 10 deletions(-) create mode 100644 tests/test_card_link.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a0c2769..f6381d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- 🐛 FIX: `card`/`grid-item-card` `:link:` no longer strips whitespace from reference targets (and lowercases them for `link-type: ref`, matching the `:ref:` role), completing the fix for {issue}`110` ({pr}`282`) - 🐛 FIX: `button-ref` no longer strips whitespace from reference targets, so multi-word labels (e.g. from `autosectionlabel`) resolve correctly ({pr}`281`, {issue}`110`) diff --git a/sphinx_design/cards.py b/sphinx_design/cards.py index 3e46e55..ec51cfb 100644 --- a/sphinx_design/cards.py +++ b/sphinx_design/cards.py @@ -6,6 +6,7 @@ from docutils.statemachine import StringList from sphinx import addnodes from sphinx.application import Sphinx +from sphinx.util import ws_re from sphinx.util.docutils import SphinxDirective from sphinx.util.logging import getLogger @@ -61,7 +62,7 @@ class CardDirective(SdDirective): "img-bottom": directives.uri, "img-background": directives.uri, "img-alt": directives.unchanged, - "link": directives.uri, + "link": directives.unchanged_required, "link-type": make_choice(["url", "any", "ref", "doc"]), "link-alt": directives.unchanged, "shadow": make_choice(["none", "sm", "md", "lg"]), @@ -171,28 +172,32 @@ def create_card( # noqa: PLR0915 if "link" in options: link_container = PassthroughTextElement() _classes = ["sd-stretched-link", "sd-hide-link-text"] - _rawtext = options.get("link-alt") or options["link"] - if options.get("link-type", "url") == "url": + link_type = options.get("link-type", "url") + # the raw (un-normalised) target, kept for user-visible fallback text + link_raw = options["link"] + link_target = cls.get_link_target(link_raw, link_type) + _rawtext = options.get("link-alt") or link_raw + if link_type == "url": link = nodes.reference( _rawtext, "", nodes.inline(_rawtext, _rawtext), - refuri=options["link"], + refuri=link_target, classes=_classes, ) else: - options = { + ref_options = { # TODO the presence of classes raises an error if the link cannot be found "classes": _classes, - "reftarget": options["link"], + "reftarget": link_target, "refdoc": inst.env.docname, - "refdomain": "" if options["link-type"] == "any" else "std", - "reftype": options["link-type"], + "refdomain": "" if link_type == "any" else "std", + "reftype": link_type, "refexplicit": "link-alt" in options, "refwarn": True, } link = addnodes.pending_xref( - _rawtext, nodes.inline(_rawtext, _rawtext), **options + _rawtext, nodes.inline(_rawtext, _rawtext), **ref_options ) inst.set_source_info(link) link_container += link @@ -200,6 +205,38 @@ def create_card( # noqa: PLR0915 return card + @staticmethod + def get_link_target(target: str, link_type: str) -> str: + """Normalise a ``link`` option value into a reference target. + + The ``link`` option is captured verbatim (``unchanged_required``) so + that whitespace in reference targets is preserved; how it is normalised + then depends on ``link-type``: + + - ``url`` (the default): all whitespace is removed, exactly as + :func:`docutils.parsers.rst.directives.uri` did before -- URLs cannot + contain unescaped whitespace. + - ``ref``: internal whitespace runs are collapsed to single spaces and + the target is lowercased, mirroring Sphinx's ``:ref:`` role + (``XRefRole(lowercase=True)``); std-domain labels are stored + lowercased, so a multi-word, Title-Case heading (e.g. one generated + by ``sphinx.ext.autosectionlabel``) can be pasted verbatim. + - ``doc`` / ``any``: internal whitespace runs are collapsed, but case is + preserved (docnames are case-sensitive; the ``any`` resolver + lowercases internally where needed). + + :param target: The raw ``link`` option value. + :param link_type: The ``link-type`` option + (``url``/``ref``/``doc``/``any``). + :return: The normalised reference target. + """ + if link_type == "url": + return directives.uri(target) + target = ws_re.sub(" ", target).strip() + if link_type == "ref": + return target.lower() + return target + @staticmethod def split_content(content: StringList, offset: int) -> CardContent: """Split the content into header, body and footer.""" diff --git a/sphinx_design/grids.py b/sphinx_design/grids.py index 743f231..c0325c0 100644 --- a/sphinx_design/grids.py +++ b/sphinx_design/grids.py @@ -223,7 +223,9 @@ class GridItemCardDirective(SdDirective): "img-top": directives.uri, "img-bottom": directives.uri, "img-alt": directives.unchanged, - "link": directives.uri, + # captured verbatim; ``CardDirective.create_card`` normalises it + # per ``link-type`` (whitespace is significant for reference targets) + "link": directives.unchanged_required, "link-type": make_choice(["url", "any", "ref", "doc"]), "link-alt": directives.unchanged, "shadow": make_choice(["none", "sm", "md", "lg"]), diff --git a/tests/test_card_link.py b/tests/test_card_link.py new file mode 100644 index 0000000..b4cce89 --- /dev/null +++ b/tests/test_card_link.py @@ -0,0 +1,275 @@ +"""Tests for the ``card`` / ``grid-item-card`` ``:link:`` option. + +Covers #110 for the card path (the follow-up to the ``button-ref`` fix in +#281): a ``:link:`` reference target that contains whitespace -- e.g. the +labels ``sphinx.ext.autosectionlabel`` generates from section titles -- must be +preserved (and, for ``link-type: ref``, lowercased to match the ``:ref:`` +role), rather than having all its whitespace stripped by ``directives.uri``. + +The bug affected both the ``card`` directive (``cards.py``) and the +``grid-item-card`` directive (``grids.py``), which has its own ``option_spec`` +that previously also ran ``:link:`` through ``directives.uri`` before delegating +to ``CardDirective.create_card``. +""" + +import re + +from docutils.parsers.rst import directives +import pytest + +from sphinx_design.cards import CardDirective + +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``. + + ``sphinx.ext.autosectionlabel`` is enabled so that section titles become + (multi-word) reference labels -- exactly the #110 reproduction. + """ + 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 _stretched_link_hrefs(html): + """Return the ``href`` of every card ``sd-stretched-link`` anchor. + + This isolates the card's own link from unrelated anchors (e.g. the section + heading's permalink), which also point at ``#my-section-name``. + """ + return re.findall(r']*\bsd-stretched-link\b[^>]*href="([^"]*)"', html) + + +def test_get_link_target_normalisation(): + """``get_link_target`` normalises a raw ``:link:`` value per ``link-type``. + + ``url`` strips *all* whitespace (byte-identical to the previous + ``directives.uri`` behaviour); ``ref`` collapses internal whitespace *and* + lowercases (matching Sphinx's ``XRefRole(lowercase=True)`` ``:ref:`` role); + ``doc``/``any`` collapse whitespace but preserve case. + """ + # url: identical to directives.uri (all whitespace removed) + assert CardDirective.get_link_target( + "https://example.com/a b", "url" + ) == directives.uri("https://example.com/a b") + assert ( + CardDirective.get_link_target("my article section", "url") == "myarticlesection" + ) + # ref: whitespace collapsed to single spaces, then lowercased + assert CardDirective.get_link_target("My Article\n Section", "ref") == ( + "my article section" + ) + assert CardDirective.get_link_target(" Spaced-Label ", "ref") == "spaced-label" + # doc / any: whitespace collapsed, case preserved + assert CardDirective.get_link_target("My Doc\n Name", "doc") == "My Doc Name" + assert CardDirective.get_link_target("My Any\n Target", "any") == "My Any Target" + + +CARD_MULTIWORD = { + "rst": """ +Heading +======= + +My Section Name +--------------- + +Some content. + +.. card:: A card title + :link: my section name + :link-type: ref + + Card body +""", + "myst": """ +# Heading + +## My Section Name + +Some content. + +:::{card} A card title +:link: my section name +:link-type: ref + +Card body +::: +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_card_ref_multiword_target(fmt, sphinx_builder): + """A ``card`` ``:link:`` to a multi-word ref label resolves (#110).""" + builder = _build(sphinx_builder, fmt, CARD_MULTIWORD["rst"], CARD_MULTIWORD["myst"]) + # a clean build (no ``undefined label`` warning) already proves it resolved + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert "#my-section-name" in _stretched_link_hrefs(html) + + +GRID_MULTIWORD = { + "rst": """ +Heading +======= + +My Section Name +--------------- + +Some content. + +.. grid:: 1 + + .. grid-item-card:: A grid card + :link: my section name + :link-type: ref + + Grid card body +""", + "myst": """ +# Heading + +## My Section Name + +Some content. + +::::{grid} 1 + +:::{grid-item-card} A grid card +:link: my section name +:link-type: ref + +Grid card body +::: + +:::: +""", +} + + +@pytest.mark.parametrize("fmt", ["rst", MYST_PARAM]) +def test_grid_item_card_ref_multiword_target(fmt, sphinx_builder): + """A ``grid-item-card`` ``:link:`` to a multi-word ref label resolves. + + This is the exact #110 reproduction; ``grid-item-card`` has its own + ``option_spec``, so it needs the same fix as ``card``. + """ + builder = _build(sphinx_builder, fmt, GRID_MULTIWORD["rst"], GRID_MULTIWORD["myst"]) + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert "#my-section-name" in _stretched_link_hrefs(html) + + +TITLECASE_RST = """ +Heading +======= + +My Section Name +--------------- + +Some content. + +.. card:: A card title + :link: My Section Name + :link-type: ref + + Card body +""" + + +def test_card_ref_titlecase_target(sphinx_builder): + """A Title-Case ``ref`` target resolves like ``:ref:`` (lowercase parity). + + Sphinx stores std-domain labels lowercased and ``:ref:`` lowercases its + target, so the verbatim heading text can be pasted as ``:link:``. + """ + builder = _build(sphinx_builder, "rst", TITLECASE_RST, "") + html = (builder.out_path / "index.html").read_text(encoding="utf8") + assert "#my-section-name" in _stretched_link_hrefs(html) + + +URL_RST = """ +Heading +======= + +.. card:: A card title + :link: https://example.com/a b + :link-type: url + + Card body + +.. card:: Another card + :link: https://example.com/plain + + Body +""" + + +def test_card_link_url_whitespace_unchanged(sphinx_builder): + """``link-type: url`` behaviour is byte-identical to before the fix. + + ``directives.uri`` removed all whitespace from URLs; the default + (``link-type: url``) path must keep doing exactly that. + """ + builder = _build(sphinx_builder, "rst", URL_RST, "") + html = (builder.out_path / "index.html").read_text(encoding="utf8") + hrefs = _stretched_link_hrefs(html) + assert hrefs == [ + directives.uri("https://example.com/a b"), + "https://example.com/plain", + ] + assert hrefs[0] == "https://example.com/ab" + + +UNRESOLVED_RST = """ +Heading +======= + +.. card:: A card title + :link: No Such Label + :link-type: ref + + Card body +""" + + +def test_card_ref_unresolved_warns(sphinx_builder): + """An unresolved ``ref`` ``:link:`` warns and still builds (no crash). + + The (normalised, lowercased) target appears in a standard missing-reference + warning; the card body still renders, unlinked. + """ + builder = _build(sphinx_builder, "rst", UNRESOLVED_RST, "", assert_pass=False) + # the collapsed + lowercased target is what is looked up (and reported) + assert "undefined label: 'no such label'" in builder.warnings + html = (builder.out_path / "index.html").read_text(encoding="utf8") + # build completed and the content survives, just without a resolved link + assert "Card body" in html + assert _stretched_link_hrefs(html) == []