executablebooks / jupyter-book

Create beautiful, publication-quality books and documents from computational content.
http://jupyterbook.org
BSD 3-Clause "New" or "Revised" License
3.76k stars 652 forks source link

FIX: Remove sphinx-book-theme from extensions #2111

Closed ghisvail closed 4 months ago

ghisvail commented 5 months ago

See https://github.com/executablebooks/sphinx-book-theme/pull/770

welcome[bot] commented 5 months ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

welcome[bot] commented 4 months ago

Congrats on your first merged pull request in this project! :tada: congrats
Thank you for contributing, we are very proud of you! :heart:

agoose77 commented 4 months ago

Great, thanks! This is a much-needed change.

ghisvail commented 4 months ago

Indeed, it would be worth publishing a patch release with this change.

People using readthedocs to host JB-based docs, need to run jupyter-book config before the regular build step using Sphinx. The broken config can be amended with a trivial sed call, but it's better to avoid it than documenting it, IMO.

agoose77 commented 4 months ago

This needs a bit of a rethink, actually.

There are a few separate problems interplaying, and this PR introduces a new one; sphinx-book-theme exposes directives that aren't available during LaTeX builds if we only inject the extension via the HTML theme.

I'm going to revisit the whole stack and see if we can support both usages.

agoose77 commented 4 months ago

OK, that took some digging.

The registration order is as follows:

This tells use that:

  1. The setup() level configuration registration ensures that the theme search path (in create_template_bridge is correct. We can't use any events for this because the theme is set-up immediately before the path is required (before any events are dispatched).
  2. Extensions are registered first, meaning that any theme variables set here get clobbered. These variables are not set again later because the theme's extension has already been loaded. This requires that we use e.g. config-inited to set this value before the theme is required.
agoose77 commented 4 months ago

I think I have the solution in mind, now.