Docs: install theme as Sphinx extension may not be required

[astrojuanlu]

As @evildmp noted, the installation docs say:

sphinx_rtd_theme/docs/installing.rst

Lines 11 to 22 in b07560b

	In your ``conf.py`` file:
	.. code:: python
	import sphinx_rtd_theme
	extensions = [
	...
	'sphinx_rtd_theme',
	]
	html_theme = "sphinx_rtd_theme"

However, just doing html_theme = 'sphinx_rtd_theme' works for most cases. There's a note below:

sphinx_rtd_theme/docs/installing.rst

Lines 25 to 29 in b07560b

	.. note::
	Adding this theme as an extension is what enables localization of theme
	strings in your translated output. If these strings are not translated in
	your output, either we lack the localized strings for your locale, or you
	are using an old version of the theme.

but I don't understand what it means. Perhaps projects that want to opt-out from localization can use the simplified procedure? What am I missing here?

[webknjaz]

Perhaps projects that want to opt-out from localization can use the simplified procedure? What am I missing here?

It's not just localisation, it's also a few other compatibility shims: https://github.com/readthedocs/sphinx_rtd_theme/blob/73d1707/sphinx_rtd_theme/__init__.py#L51-L61.

But yes, strictly speaking, most of the theme will work w/o loading the extension because it's also declared as an entry point: https://github.com/readthedocs/sphinx_rtd_theme/blob/73d1707/setup.py#L116.

[humitos]

But yes, strictly speaking, most of the theme will work w/o loading the extension because it's also declared as an entry point: 73d1707/setup.py#L116.

I'm not 100% sure what it means the theme is declared as an entry point. Can you expand on that? What's the difference between the entry point and add it as an installed extension?

[humitos]

furo theme says to only define html_theme on its instructions and defines the entry point in pyproject.toml. So, I suppose we should be able to follow the same simplified approach and remove it from extensions and that particular note as well.

[agjohnson]

I will note that specifying the theme in extensions is explicit version of what Sphinx already does implicitly:

• https://github.com/sphinx-doc/sphinx/blob/master/sphinx/theming.py#L195

Themes are loaded as extensions to execute everything in the theme's setup() function, but it's still functionally an extension.

Also also, not all projects have or need a pyproject.toml or setup.{cfg,py}, especially if the project is just documentation.

So for that, I think the current directions are probably still best. The note on translations is very confusing and probably unnecessary though.

[humitos]

Also also, not all projects have or need a pyproject.toml or setup.{cfg,py}, especially if the project is just documentation.

I don't think projects need these files. We mentioned them because the theme declares itself as an entry point as the official Sphinx documentation mentions.

If adding the theme as extension is not required, I would remove it from the documentation since it just adds an extra step for no reason.

[humitos]

I don't 100% understand the Sphinx internals, but it seems we cannot remove the installation as an extension: #1434 (comment). We would need more research and test if we want to move forward with this.

[webknjaz]

@humitos there are two ways for a theme to get registered in Sphinx.
If a theme declares the respective entry point in its packaging metadata, then Sphinx will make it possible to just use the theme name in conf.py, if it's installed.
If a theme does not, then the end-users would have to import and call something returning the theme path and paste that into conf.py. Never do both as it may result in weird behavior.

A project containing a theme may contain an extension entry-point function (setup()). Sphinx will call it whenever the respective importable reference is listed in extensions of conf.py. Sometimes, that hook does some theme registration (and maybe adds some build-related event handlers).

I saw themes different things. In my opinion, the most straightforward way is to use entry points for making the theme discoverable and optionally the extension entry point for handling the assets. So the end-users would just have to add one or two lines to their config and that's it.

[humitos]

If a theme declares the respective entry point in its packaging metadata, then Sphinx will make it possible to just use the theme name in conf.py, if it's installed.

This is how we are registering ours and it's what Sphinx says in their theme development documentation. That's why I think that added the theme as an extension is not required and I wanted to remove that extra step.

However, based on the user comment that I linked in my previous comment, it seems that our jquery setup is not executed when the theme is not defined as a Sphinx extension.

I'd be really good if we can test these difference scenarios and arrive at a conclusion that works for all these scenarios.

[webknjaz]

This makes the importable discoverable: https://github.com/readthedocs/sphinx_rtd_theme/blob/2c19bb1/setup.cfg#L62-L64. And this https://github.com/readthedocs/sphinx_rtd_theme/blob/2c19bb1/sphinx_rtd_theme/__init__.py#L73 adds the template path.
I believe it's already loaded as an extension automatically and probably early in the flow.

This is what activates sphinxcontrib.jquery and additionally registers its JS files: https://github.com/readthedocs/sphinx_rtd_theme/blob/2c19bb1/sphinx_rtd_theme/__init__.py#L63-L70. I'd expect it to run before loading conf.py, though. Perhaps, it needs to run after some of the conf.py settings are declared.. Maybe, if this chunk was plugged via app.connect(), it'd be called at a more appropriate time.

[humitos]

Maybe, if this chunk was plugged via app.connect(), it'd be called at a more appropriate time.

Good point. This a good thing to test 👍🏼 -- if that does work, I think we can move forward with this issue.

[humitos]

I tested this by myself using the documentation of the theme and I was able to reproduce the issue mentioned by the user: search doesn't work when removing the theme from extensions. I've tested the current code, but also moving the jquery chunk's code to an app.connect('config-inited', ...).

So, adding the theme in extensions seems to be required for the jQuery extension the theme depends on to setup correctly.

---

BTW, the message about localization can be removed. I followed the same tests than for jQuery and in both cases (added/removed from extensions) the strings are shown properly translated.

I think, for now, we should update that note to mention it's required to add as an extension because of jQuery.