Sphinx extension: fix creation of nodes for options and return values; produce error nodes (#175)

* Make sure that external references (intersphinx) are formatted the same as internal ones.

* Format error messages.

* Make sure references look the same no matter whether they are resolved by intersphinx, internally, or not at all.

* Add changelog fragment.

* Reformat.

Author: felixfontein

changelogs/fragments/175-sphinx-ext-nodes.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - "Fix the way the Sphinx extension creates nodes for options and return values so they look identical for internal references, external (intersphinx) references, and unresolved references (https://github.com/ansible-community/antsibull-docs/pull/175)."
+  - "Make sure that broken ``:ansopt:`` and ``:ansretval:`` parameters result in correctly rendered error messages (https://github.com/ansible-community/antsibull-docs/pull/175)."

src/sphinx_antsibull_ext/antsibull-minimal.css

@@ -341,7 +341,7 @@ table.ansible-option-table {
 }
 
 .ansible-option, .ansible-option-value, .ansible-return-value {
-  a.reference.internal {
+  a.reference.internal, a.reference.external {
     color: unset;
 
     &:hover {

src/sphinx_antsibull_ext/css/antsibull-minimal.scss

@@ -127,14 +127,16 @@ def _create_ref_or_not(
     entrypoint: str | None,
     ref_parameter: str,
     text: str,
-) -> tuple[str, list[t.Any]]:
+) -> t.Any:
+    # When successfully resolving *internal* references, Sphinx does **NOT**
+    # use the node we provide, but simply extracts the text and creates a new
+    # node. Thus we use nodes.inline so that the result is the same no matter
+    # whether the reference was internal, not resolved, or external
+    # (intersphinx).
+    content = nodes.inline(text, text)
     ref = create_ref(plugin_fqcn, plugin_type, entrypoint, ref_parameter)
     if ref is None:
-        return text, []
-
-    # The content node will be replaced by Sphinx anyway, so it doesn't matter what kind
-    # of node we are using...
-    content = nodes.literal(text, text)
+        return content
 
     options = {
         "reftype": "ref",
@@ -144,13 +146,13 @@ def _create_ref_or_not(
     }
     refnode = addnodes.pending_xref(text, content, **options)
     refnode["reftarget"] = ref
-    return "", [refnode]
+    return refnode
 
 
 # pylint:disable-next=unused-argument
 def _create_error(rawtext: str, text: str, error: str) -> tuple[list[t.Any], list[str]]:
-    node = ...  # FIXME
-    return [node], []
+    content = nodes.strong(text, error, classes=["error"])
+    return [content], []
 
 
 # pylint:disable-next=unused-argument,dangerous-default-value
@@ -182,7 +184,7 @@ def option_role(name, rawtext, text, lineno, inliner, options={}, content=[]):
     else:
         text = f"{option}={value}"
         classes.append("ansible-option-value")
-    text, subnodes = _create_ref_or_not(
+    content = _create_ref_or_not(
         _create_option_reference,
         plugin_fqcn,
         plugin_type,
@@ -191,11 +193,8 @@ def option_role(name, rawtext, text, lineno, inliner, options={}, content=[]):
         text,
     )
     if value is None:
-        content = nodes.strong(rawtext, text, *subnodes)
-        content = nodes.literal(rawtext, "", content, classes=classes)
-    else:
-        content = nodes.literal(rawtext, text, *subnodes, classes=classes)
-    return [content], []
+        content = nodes.strong(rawtext, "", content)
+    return [nodes.literal(rawtext, "", content, classes=classes)], []
 
 
 # pylint:disable-next=unused-argument,dangerous-default-value
@@ -244,15 +243,15 @@ def return_value_role(name, rawtext, text, lineno, inliner, options={}, content=
         text = f"{rv}"
     else:
         text = f"{rv}={value}"
-    text, subnodes = _create_ref_or_not(
+    content = _create_ref_or_not(
         _create_return_value_reference,
         plugin_fqcn,
         plugin_type,
         entrypoint,
         rv_link,
         text,
     )
-    return [nodes.literal(rawtext, text, *subnodes, classes=classes)], []
+    return [nodes.literal(rawtext, "", content, classes=classes)], []
 
 
 # pylint:disable-next=unused-argument,dangerous-default-value