Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theme fix - use style sources from theme instead from built-in #91

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
53 changes: 2 additions & 51 deletions sphinx_simplepdf/builders/simplepdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import subprocess
import weasyprint

import sass

from bs4 import BeautifulSoup

from sphinx import __version__
Expand Down Expand Up @@ -44,61 +42,14 @@ def __init__(self, *args, **kwargs):

debug_sphinx = {
"version": __version__,
"confidr": self.app.confdir,
"confdir": self.app.confdir,
"srcdir": self.app.srcdir,
"outdir": self.app.outdir,
"extensions": self.app.config.extensions,
"simple_config": {x.name: x.value for x in self.app.config if x.name.startswith("simplepdf")},
}
self.app.config.html_context["spd"] = debug_sphinx

# Generate main.css
logger.info("Generating css files from scss-templates")
css_folder = os.path.join(self.app.outdir, f"_static")
scss_folder = os.path.join(
os.path.dirname(__file__), "..", "themes", "simplepdf_theme", "static", "styles", "sources"
)
sass.compile(
dirname=(scss_folder, css_folder),
output_style="nested",
custom_functions={
sass.SassFunction("config", ("$a", "$b"), self.get_config_var),
sass.SassFunction("theme_option", ("$a", "$b"), self.get_theme_option_var),
},
)

def get_config_var(self, name, default):
"""
Gets a config variables for scss out of the Sphinx configuration.
If name is not found in config, the specified default var is returned.

Args:
name: Name of the config var to use
default: Default value, if name can not be found in config

Returns: Value
"""
simplepdf_vars = self.app.config.simplepdf_vars
if name not in simplepdf_vars:
return default
return simplepdf_vars[name]

def get_theme_option_var(self, name, default):
"""
Gets a option variables for scss out of the Sphinx theme options.
If name is not found in theme options, the specified default var is returned.

Args:
name: Name of the option var to use
default: Default value, if name can not be found in config

Returns: Value
"""
simplepdf_theme_options = self.app.config.simplepdf_theme_options
if name not in simplepdf_theme_options:
return default
return simplepdf_theme_options[name]

def finish(self) -> None:
super().finish()

Expand Down Expand Up @@ -156,7 +107,7 @@ def finish(self) -> None:
success = True
break
except subprocess.TimeoutExpired:
logger.warning(f"TimeoutExpired in weasyprint, retrying")
logger.warning("TimeoutExpired in weasyprint, retrying")
except subprocess.CalledProcessError as e:
logger.warning(f"CalledProcessError in weasyprint, retrying\n{str(e)}")
finally:
Expand Down
29 changes: 27 additions & 2 deletions sphinx_simplepdf/themes/simplepdf_theme/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
"""
from os import path

import sass

__version__ = '0.1.0'

__version__ = '0.3.0'
Copy link
Member

@danwos danwos Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also: I would like to stay with 0.1.0 and raise it with an additional PR.

__version_full__ = __version__


Expand All @@ -14,10 +16,33 @@ def get_html_theme_path():
return cur_dir


def create_custom_css(app):

def get_config_var(name, default):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the doc_string of the original function here?
I know the code is understandable without it, but it also explains the use case and therefore why it is needed.

return app.config.simplepdf_vars.get(name, default)

def get_theme_var(name, default):
return app.config.simplepdf_theme_options.get(name, default)

here = path.abspath(path.dirname(__file__))
scss_folder = path.join(here, "static", "styles", "sources")

staticdir = path.join(app.builder.outdir, "_static")

sass.compile(
dirname=(scss_folder, staticdir),
output_style="nested",
custom_functions={
sass.SassFunction("config", ("$a", "$b"), get_config_var),
sass.SassFunction("theme_option", ("$a", "$b"), get_theme_var),
},
)


# See http://www.sphinx-doc.org/en/stable/theming.html#distribute-your-theme-as-a-python-package
def setup(app):
app.add_html_theme('simplepdf_theme', path.abspath(path.dirname(__file__)))
# app.add_css_file('styles/main.css')
app.connect('builder-inited', create_custom_css)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with this line we call create_custom_css every time, so for each builder including html.

We should add a check inside create_custom_css if the current builder is simplepdf.


return {
"parallel_read_safe": True,
Expand Down