BUG/ENH: Common Sphinx directives have no style applied

[bashtage]

Sphinx admonitions such as

.. deprecated:: 3.0
   This is deprecated since 3.0

have no style applied.

If you search for deprecated in the specimen page you will see unstyled markup of

.. versionadded:: 1.0
   This was added in 1.0
.. versionchanged:: 2.0
   This was changed in 2.0
.. deprecated:: 3.0
   This is deprecated since 3.0

[2bndy5]

These aren't really admonitions. Rather, they are just directives that generate simple div elements:

<div class="versionadded">
<p><span class="versionmodified added">New in version 1.0: </span>This was added in 1.0</p>
</div>
<div class="versionchanged">
<p><span class="versionmodified changed">Changed in version 2.0: </span>This was changed in 2.0</p>
</div>
<div class="deprecated">
<p><span class="versionmodified deprecated">Deprecated since version 3.0: </span>This is deprecated since 3.0</p>
</div>

This is actually not unlike what the RTD theme does for these directives.

The reason these stand apart from admonitions is because the first argument (delimited by a whitespace) is concatenated with the "title". So, while you can use the theme's custom admonition feature to override the sphinx directives, the argument will appear as a custom title (not concatenated with the default title and only delimited by a blank line).

~I would only consider adding theme-equivalent overridden directives if you can show that they are used like admonitions in the mkdocs-material theme without any user-defined customization (mkdocs plugins included as "customization").~

[2bndy5]

Comparing to the RTD theme, I think they just added (or inherited) CSS to italicize the "title":

[jbms]

Yes it appears that other themes just italicize the title. Is that what you would like to happen @bashtage ?

[bashtage]

Something more along the lines of the bottom one in the link below is what I was hoping for. I can fix these using custom CSS of course. These directives are fairly common in project documentation, and so it would be great if they could be auto styled to be similar to propper admonitions.

https://pydata-sphinx-theme.readthedocs.io/en/stable/examples/kitchen-sink/admonitions.html

[2bndy5]

I would consider that theme's specfic style controversial because they aren't actual admonitions.

Consider

.. versionadded:: 1.0 Some content

   - a list element

.. note::
   Some content.

which renders like so:

I'm more inclined to add the CSS that seems inherited by other themes (from sphinx' basic theme CSS) to italicize the title. As you noted, the custom CSS to treat the generated divs as an admonition can be added by docs author(s).

Reminder, we explicitly don't inherit CSS from Sphinx' basic theme for various reasons.

[2bndy5]

There's also nothing stopping users from using our custom admonitions feature:

sphinx_immaterial_custom_admonitions = [
    {
        "name": "versionadded",
        "color": (72, 138, 87),
        "classes": ["info"],
        "override": True,
    },
]

This abstracts the custom CSS needed, but you'd have to repeat the default title to get it to show:

.. versionadded:: New in version 1.0

    - a list element.

renders as

[2bndy5]

Remember that admonitions require content. So you'd have to separate the description of the version changes in a separate paragraph nested in the directive content. There's currently no way to create an admonition that only shows a title section as the pydata theme does for these directives.

[jbms]

To me it does seem that the pydata styling fits well with this theme, better than just italicizing, and is probably similar to what mkdocs-material would do if mkdocs had similar directives.

It seems that we could style it as an admonition even without it actually being an admonition, but I suppose it would be better if it can be done in a more generic way, similar to our other custom admonition support.

As for admonitions needing a body (may be independent of this issue) that seems like an unnecessary restriction given that we allow custom admonition titles. Not clear how easily we can relax that restriction though.

[2bndy5]

As for pydata theme reference: the styling of these directives was requested in this issue

As for admonitions needing a body (may be independent of this issue) that seems like an unnecessary restriction given that we allow custom admonition titles. Not clear how easily we can relax that restriction though.

I had a thought that might help with this. We could add a custom admonition option to not require content when the directives are created. I only had has_content = True because that's how every other admonition in Sphinx/docutils is configured.

sphinx_immaterial_custom_admonitions = [
    {
        "name": "versionadded",
        "color": (72, 138, 87),
        "classes": ["info"],
        "override": True,
        "has_content": False,
    },
]

From there we can auto-override the version directives to behave like admonitions with only a title section. Users would still be able to override the auto-overridden directives if they so choose. I'd probably also add a conf.py option to disable auto-overriding the version directives as well (just like we did for all other overridden admonitions).

[2bndy5]

The concatenation of version spec with default title is still an obstacle though. I'm not sure how we'd do that without the code getting hairy (may need a few new if conditions to check for these specific directives).

[2bndy5]

The :no-title: option would have to be ignored if has_content == False.

[2bndy5]

I toyed around with this...

WIP patch

```diff diff --git a/sphinx_immaterial/custom_admonitions.py b/sphinx_immaterial/custom_admonitions.py index 1cd74fe..ee821c9 100644 --- a/sphinx_immaterial/custom_admonitions.py +++ b/sphinx_immaterial/custom_admonitions.py @@ -78,6 +78,10 @@ class CustomAdmonitionConfig(pydantic.BaseModel): admonition :ref:`defined by rST and Sphinx ` or an admonition :ref:`inherited from the mkdocs-material theme `. """ + has_content: bool = True + """Use this option to exclude showing the body of the admonition as a separate + section from the admonition title. Default is :python:`True` as with all other + admonitions inherent in Sphinx and docutils.""" # pylint: disable=no-self-argument @@ -189,7 +193,8 @@ class CustomAdmonitionDirective(Directive, ABC): if not title_text: # title_text must be an explicit string for renderers like MyST title_text = str(self.default_title) - self.assert_has_content() + if self.has_content: + self.assert_has_content() admonition_node = self.node_class("\n".join(self.content), **self.options) # type: ignore[call-arg] ( admonition_node.source, # type: ignore[attr-defined] @@ -203,12 +208,19 @@ class CustomAdmonitionDirective(Directive, ABC): self.add_name(admonition_node) if "collapsible" in self.options: admonition_node["collapsible"] = self.options["collapsible"] - if "no-title" in self.options and "collapsible" in self.options: + if "no-title" in self.options and ( + "collapsible" in self.options or not self.has_content + ): logger.error( - "title is needed for collapsible admonitions", + "title is needed for collapsible admonitions or admonitions with no " + "required content", location=admonition_node, ) del self.options["no-title"] # force-disable option + if not self.has_content and self.arguments: + # concatenate the first arg with the default title + args: List[str] = self.arguments[0].split() + title_text = self.default_title + args[0] + ": " + " ".join(args[1:]) textnodes, messages = self.state.inline_text(title_text, self.lineno) if "no-title" not in self.options and title_text: title = nodes.title(title_text, "", *textnodes) @@ -222,7 +234,9 @@ class CustomAdmonitionDirective(Directive, ABC): return [admonition_node] -def get_directive_class(name, title, classes=None) -> Type[CustomAdmonitionDirective]: +def get_directive_class( + name, title, classes=None, requires_content: bool = True +) -> Type[CustomAdmonitionDirective]: """A helper function to produce a admonition directive's class.""" # alias upstream-deprecated CSS classes for pre-defined admonitions in sphinx class_list = [nodes.make_id(name)] @@ -243,6 +257,7 @@ def get_directive_class(name, title, classes=None) -> Type[CustomAdmonitionDirec default_title = title classes = class_list optional_arguments = int(name not in admonitionlabels) + has_content = requires_content node_class = cast( Type[nodes.admonition], nodes.admonition if name != "todo" else sphinx.ext.todo.todo_node, @@ -261,7 +276,10 @@ def on_builder_inited(app: Sphinx): app.add_directive( name=admonition.name, cls=get_directive_class( - admonition.name, admonition.title, admonition.classes + admonition.name, + admonition.title, + admonition.classes, + admonition.has_content, ), override=admonition.override, ) @@ -301,7 +319,6 @@ def on_config_inited(app: Sphinx, config: Config): user_defined_admonitions = pydantic.parse_obj_as( List[CustomAdmonitionConfig], getattr(config, confval_name) ) - setattr(config, confval_name, user_defined_admonitions) user_defined_dir_names = [directive.name for directive in user_defined_admonitions] # override the specific admonitions defined in sphinx and docutils @@ -311,6 +328,39 @@ def on_config_inited(app: Sphinx, config: Config): if admonition in user_defined_dir_names: continue app.add_directive(admonition, get_directive_class(admonition, title), True) + # override sphinx directives versionadded, versionchanged, and deprecated + if getattr(config, "sphinx_immaterial_override_version_directives"): + if "versionadded" not in user_defined_dir_names: + version_added = CustomAdmonitionConfig( + name="versionadded", + title="New in version ", + icon="material/alert-circle", + color=(72, 138, 87), + override=True, + has_content=False, + ) + user_defined_admonitions.append(version_added) + if "versionchanged" not in user_defined_dir_names: + version_changed = CustomAdmonitionConfig( + name="versionchanged", + title="Changed in version ", + icon="material/alert-circle", + color=(238, 144, 64), + override=True, + has_content=False, + ) + user_defined_admonitions.append(version_changed) + if "deprecated" not in user_defined_dir_names: + version_deprecated = CustomAdmonitionConfig( + name="deprecated", + title="Deprecated since version ", + icon="material/delete", + color=(203, 70, 83), + override=True, + has_content=False, + ) + user_defined_admonitions.append(version_deprecated) + setattr(config, confval_name, user_defined_admonitions) def add_admonition_and_icon_css(app: Sphinx, env: BuildEnvironment): @@ -359,6 +409,12 @@ def setup(app: Sphinx): rebuild="html", types=bool, ) + app.add_config_value( + name="sphinx_immaterial_override_version_directives", + default=True, + rebuild="html", + types=bool, + ) app.connect("builder-inited", on_builder_inited) app.connect("env-check-consistency", add_admonition_and_icon_css) ```

Which will now produce Notice, I haven't added the CSS to italicize the prefixed title + version spec.

One caveat that might be a deal-breaker (as for using the custom admonitions feature) is the generated directive class has no way of knowing what the corresponding directive name is. So, concatenating the first argument with the default title isn't exclusive to the version directives (see WIP patch above).

There might be other unintended behavior that could be triggered but I haven't fully vetted the patch yet.

[bashtage]

Those look great.

[2bndy5]

I think I can work around the unintuitive directive class by using a special CSS class for the version directives...

I'll keep toiling and submit a PR for review.

[jbms]

Thanks for your work on this!

The way the combined title is generated seems rather hacky --- it might be cleaner to specify either a jinja2 template or str.format template in the custom admonition config.

Also your current patch does not seem to actually check the config's has_content option --- which should probably be renamed to require_content and can probably default to False.

[2bndy5]

I thought the pydantic class would type check the added user option. I'll look into that.

probably be renamed to require_content and can probably default to False.

I'm open to renaming the option as I just found out that setting the directive class' has_content=False will prevent any content from being added. I'm still playing around with this problem... I also explored the required_arguments class attr for directive classes, but it doesn't seem to fit well within the generalized custom admonitions.

[2bndy5]

The way the combined title is generated seems rather hacky --- it might be cleaner to specify either a jinja2 template or str.format template in the custom admonition config.

I'll probably go with str.format since adding a jinja template seem like overkill for this.

[jbms]

The way the combined title is generated seems rather hacky --- it might be cleaner to specify either a jinja2 template or str.format template in the custom admonition config.

I'll probably go with str.format since adding a jinja template seem like overkill for this.

Either way seems fine --- to be clear I was not proposing a jinja template in a separate file, rather just a jinja template specified as a string literal, e.g.: "Deprecated since version {{ arg }}"

[2bndy5]

Clearly its been a while since I did anything with docutils. I'll look into the jinja template as well.

[2bndy5]

I'm resorting to the same approach used to generate the titles in Sphinx. Labels (aka titles) are translateable. And italics rely solely on CSS applied to the rendered docutils node (not inline italic rST syntax).

Please note that the admonition approach will break the Sphinx builder named ChangesBuilder as we will not be using the original directive class VersionChange. I'm ok with this because the builder seems kinda useless (last time I tried it), and users can always set the new conf.py option sphinx_immaterial_override_version_directives=False to revert back to original Sphinx implementation.

[2bndy5]

I just made a fresh attempt to style these directives using original code in sphinx but with the visit_versionmodified() monkey-patched and custom admonition templates (no directives overridden). This resulted in: This was surprisingly easier than overriding the directives themselves! Now I'm going to do my thing to see if I can extend the icon & color customization to user-defined conf.py options. Stay tuned for a new PR proposal.

---

Note, that the directives take 1 required argument and 1 optional argument. Meaning that any body content for the pseudo admonitions has to be given after the optional argument (see versionadded example in screenshot).

[2bndy5]

@bashtage Looks like these directives use a node that inherits from admonitions but isn't implemented like admonitions (more like a todo admonition without the admonition style). So, this adds to the confusion. 😞

[2bndy5]

Solution available in the nightly at https://test.pypi.org/project/sphinx-immaterial/0.11.3.post1.dev17/

Its been slow, but we're getting close to a new release (just a few more simple issues to squash).

[bashtage]

They work really well. The main style looks good and is consistent with the rest of the theme.

In the end I applied a tiny bit of personal preference to them:

1. Used the color for the directive/admonition text
2. Make the actual message font-weight:normal

[2bndy5]

In a glimpse of foresight, I anticipated that users might want to make the admonitions :collapsible: or use custom CSS :class: and/or :name: (element id). Styling from other builtin admonition styles can also be inherited with sphinx_immaterial_custom_admonitions's classes option.

solution updated in nightly https://test.pypi.org/project/sphinx-immaterial/0.11.3.post1.dev21

[2bndy5]

Solution released in v0.11.4