Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
55 changes: 46 additions & 9 deletions sphinx_design/cards.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"]),
Expand Down Expand Up @@ -171,35 +172,71 @@ 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
container.append(link_container)

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."""
Expand Down
4 changes: 3 additions & 1 deletion sphinx_design/grids.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]),
Expand Down
275 changes: 275 additions & 0 deletions tests/test_card_link.py
Original file line number Diff line number Diff line change
@@ -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'<a [^>]*\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) == []
Loading