pradyunsg / sphinx-theme-builder

Streamline the Sphinx theme development workflow (maintained, but extremely stable as of Jan 2023)
https://sphinx-theme-builder.rtfd.io/
MIT License
29 stars 17 forks source link

Add support for non-static compiled assets #18

Open choldgraf opened 2 years ago

choldgraf commented 2 years ago

Context

Currently, the extra-compiled-assets configuration assumes that compiled assets are placed somewhere in the static folder of the theme. These are then bundled with any documentation that uses this theme.

However, there are some cases where you compile assets that are not meant to be bundled with the theme.

An example of this is HTML partials that are meant for use in Jinja templates, that define hashes for assets. Currently, these snippets of non-valid HTML (they contain pathto( calls and such) are still bundled with the theme in the _static folder. This isn't a huge problem, but it does mean that things like link auto-checkers will complain about incorrect links (because it discovers the items in the _static folder).

Another example of this is translation machinery (e.g. the .mo and .po files that you may want to generate programmatically, so that the human-editable translation files are in a more readable location (but not bundled with the theme)

What I tried

I tried adding a relative path to extra-compiled-assets (e.g. ../partials/mypartial.html). But at least for stb serve this doesn't work because the regex doesn't match relative paths. As a result, the function loops endlessly because it keeps detecting a change that is outside of the static folder.

The project.compiled_assets looks like this on the book theme for example:

$ project.compiled_assets

('src/sphinx_book_theme/theme/sphinx_book_theme/static/scripts/sphinx-book-theme.js',
 'src/sphinx_book_theme/theme/sphinx_book_theme/static/styles/sphinx-book-theme.css',
 'src/sphinx_book_theme/theme/sphinx_book_theme/static/../partials/sbt-webpack-macros.html',
 'src/sphinx_book_theme/theme/sphinx_book_theme/static/scripts/sphinx-book-theme.js.map',
 'src/sphinx_book_theme/theme/sphinx_book_theme/static/styles/sphinx-book-theme.css.map',
 'src/sphinx_book_theme/theme/sphinx_book_theme/static/../partials/sbt-webpack-macros.html.map')

Potential fix

I noticed that the compiled_assets property is documented as relative to Project root, but that the code itself keeps these assets relative to the static directory:

https://github.com/pradyunsg/sphinx-theme-builder/blob/df7f089add6726064f0966741268e803c2d2d8b2/src/sphinx_theme_builder/_internal/project.py#L374-L386

If those files were stored relative to the project root (which I guess would be src/<themename>?, then we wouldn't run into problems if compiled files did not exist in the static directory.

choldgraf commented 2 years ago

I think I figured out where in the codebase we're making assumptions that compiled files are inside of the static directory - updated the top comment with that extra context, but I want to make sure that @pradyunsg thinks the proposal is reasonable before trying a fix

pradyunsg commented 2 years ago

Hmm... compiled_assets being relative to project root while the user-specified additional_compiled_static_assets being relative to static file paths, is intentional. I didn't want/expect themes to compile anything other than CSS and JS.

Soo... there's two options:

I don't particularly like the idea of having non-static files being compiled and, IIUC, this is only for translations -- so I don't want to make a backwards incompatible change for this. Let's go with normpath in the compiled_assets property, and figure out a better translations story in a follow up.

pradyunsg commented 2 years ago

x-ref: https://github.com/pradyunsg/sphinx-theme-builder/issues/24

pradyunsg commented 2 years ago

The key that is being used to specify the additional CSS/JS files is: additional-compiled-static-assets.

Let's add another key (compiled-templates) that serves the need that sphinx-book-theme has -- which is generating a template file as part of the JS build process.