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
Support and prefer .jinja
to _t
for static templates
#11165
Changes from 32 commits
3e735ee
5ce9483
168cb23
4a8a339
eb9938a
43f8b2b
b2889de
a720933
6f99cef
d2af9b5
526fdc7
cd0d61f
5ab2226
b8570ab
59534ea
e38e78e
6305f96
dc850b7
04aa946
8572661
0eb993d
0444616
29fb6f0
1630cc8
d85f016
8475023
bc43d2e
06aa004
7b061cf
abd8af5
d8e3fa7
30c7318
d0aa68a
299dbed
c3b5cb9
6600615
9327357
cccdfaf
cb9ea44
9a4410a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
import shutil | ||
import subprocess | ||
import tempfile | ||
import warnings | ||
from os import path | ||
from subprocess import CalledProcessError | ||
from typing import Any | ||
|
@@ -95,16 +96,25 @@ def generate_latex_macro(image_format: str, | |
} | ||
|
||
if config.imgmath_use_preview: | ||
template_name = 'preview.tex_t' | ||
template_name = 'preview.tex' | ||
else: | ||
template_name = 'template.tex_t' | ||
template_name = 'template.tex' | ||
|
||
for template_dir in config.templates_path: | ||
template = path.join(confdir, template_dir, template_name) | ||
if path.exists(template): | ||
return LaTeXRenderer().render(template, variables) | ||
|
||
return LaTeXRenderer(templates_path).render(template_name, variables) | ||
# TODO: remove "_t" template suffix support after 2025-04-06 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think support should not be removed at that time, but the deprecation period should start at that time! At the given date, a |
||
for template_suffix in ['_t', '.jinja']: | ||
jayaddison marked this conversation as resolved.
Show resolved
Hide resolved
|
||
template = path.join(confdir, template_dir, template_name + template_suffix) | ||
if path.exists(template): | ||
if template_suffix == '_t': | ||
warnings.warn( | ||
f"{template!r}: filename suffix '_t' for templates is deprecated. " | ||
"If the file is a Jinja2 template, use the suffix '.jinja' instead.", | ||
PendingDeprecationWarning, | ||
) | ||
return LaTeXRenderer().render(template, variables) | ||
|
||
# Default: fallback to a pathless in-library jinja template | ||
return LaTeXRenderer(templates_path).render(f"{template_name}.jinja", variables) | ||
|
||
|
||
def ensure_tempdir(builder: Builder) -> str: | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ | |||||
|
||||||
import os | ||||||
import posixpath | ||||||
import warnings | ||||||
from typing import TYPE_CHECKING, Callable | ||||||
|
||||||
from docutils.utils import relative_path | ||||||
|
@@ -15,6 +16,20 @@ | |||||
from sphinx.util.template import BaseRenderer | ||||||
|
||||||
|
||||||
def _template_basename(filename: str) -> str | None: | ||||||
# TODO: remove "_t" template suffix support after 2025-04-06 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above, the deprecation period should start at that date. |
||||||
if filename.lower().endswith('_t'): | ||||||
warnings.warn( | ||||||
f"{filename!r}: filename suffix '_t' for templates is deprecated. " | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
"If the file is a Jinja2 template, use the suffix '.jinja' instead.", | ||||||
PendingDeprecationWarning, | ||||||
) | ||||||
return filename[:-2] | ||||||
elif filename.lower().endswith(".jinja"): | ||||||
return filename[:-6] | ||||||
return None | ||||||
jayaddison marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
|
||||||
def copy_asset_file(source: str, destination: str, | ||||||
context: dict | None = None, | ||||||
renderer: BaseRenderer | None = None) -> None: | ||||||
|
@@ -35,14 +50,13 @@ def copy_asset_file(source: str, destination: str, | |||||
# Use source filename if destination points a directory | ||||||
destination = os.path.join(destination, os.path.basename(source)) | ||||||
|
||||||
if source.lower().endswith('_t') and context is not None: | ||||||
if _template_basename(source) and context is not None: | ||||||
jayaddison marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if renderer is None: | ||||||
from sphinx.util.template import SphinxRenderer | ||||||
renderer = SphinxRenderer() | ||||||
|
||||||
with open(source, encoding='utf-8') as fsrc: | ||||||
if destination.lower().endswith('_t'): | ||||||
destination = destination[:-2] | ||||||
destination = _template_basename(destination) or destination | ||||||
jayaddison marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
with open(destination, 'w', encoding='utf-8') as fdst: | ||||||
fdst.write(renderer.render_string(fsrc.read(), context)) | ||||||
else: | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -435,7 +435,8 @@ def astext(self) -> str: | |
'body': ''.join(self.body), | ||
'indices': self.generate_indices(), | ||
}) | ||
return self.render('latex.tex_t', self.elements) | ||
template = self._find_template('latex.tex') | ||
return self.render(template, self.elements) | ||
|
||
def hypertarget(self, id: str, withdoc: bool = True, anchor: bool = True) -> str: | ||
if withdoc: | ||
|
@@ -512,14 +513,27 @@ def generate(content: list[tuple[str, list[IndexEntry]]], collapsed: bool) -> No | |
|
||
return ''.join(ret) | ||
|
||
def render(self, template_name: str, variables: dict[str, Any]) -> str: | ||
renderer = LaTeXRenderer(latex_engine=self.config.latex_engine) | ||
def _find_template(self, template_name: str) -> str: | ||
for template_dir in self.config.templates_path: | ||
template = path.join(self.builder.confdir, template_dir, | ||
template_name) | ||
if path.exists(template): | ||
return renderer.render(template, variables) | ||
# TODO: remove "_t" template suffix support after 2025-04-06 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, this should be the start of the deprecation period. |
||
for template_suffix in ['_t', '.jinja']: | ||
jayaddison marked this conversation as resolved.
Show resolved
Hide resolved
|
||
template = path.join(self.builder.confdir, template_dir, | ||
template_name + template_suffix) | ||
if path.exists(template): | ||
if template_suffix == '_t': | ||
warnings.warn( | ||
f"{template!r}: filename suffix '_t' for templates is deprecated. " | ||
"If the file is a Jinja2 template, use the suffix '.jinja' " | ||
"instead.", | ||
PendingDeprecationWarning, | ||
) | ||
return template | ||
|
||
# Default: fallback to a pathless in-library jinja template | ||
return f"{template_name}.jinja" | ||
|
||
def render(self, template_name: str, variables: dict[str, Any]) -> str: | ||
renderer = LaTeXRenderer(latex_engine=self.config.latex_engine) | ||
return renderer.render(template_name, variables) | ||
|
||
@property | ||
|
@@ -902,8 +916,8 @@ def visit_table(self, node: Element) -> None: | |
def depart_table(self, node: Element) -> None: | ||
labels = self.hypertarget_to(node) | ||
table_type = self.table.get_table_type() | ||
table = self.render(table_type + '.tex_t', | ||
{'table': self.table, 'labels': labels}) | ||
template = self._find_template(f"{table_type}.tex") | ||
table = self.render(template, {'table': self.table, 'labels': labels}) | ||
self.body.append(BLANKLINE) | ||
self.body.append(table) | ||
self.body.append(CR) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
<# TODO: remove "_t" template suffix support after 2025-04-06 #> | ||
AU REVOIR, KANIGGETS |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1379,6 +1379,10 @@ def test_latex_table_custom_template_caseA(app, status, warning): | |
result = (app.outdir / 'python.tex').read_text(encoding='utf8') | ||
assert 'SALUT LES COPAINS' in result | ||
|
||
# # TODO: remove "_t" template suffix support after 2025-04-06 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above |
||
assert 'TODO' not in result | ||
assert 'AU REVOIR, KANIGGETS' in result | ||
|
||
|
||
@pytest.mark.sphinx('latex', testroot='latex-table', | ||
confoverrides={'templates_path': ['_mytemplates']}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if they would not yet be called "deprecated".
I think consensus was to mark them as "pending deprecation"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yep: I'll update the
CHANGES
entry (and this pull request title) to say "pending deprecation" instead.Sorta-related: reading back through some of the previous commentary, it looks like I misread one of your previous messages:
Somehow I didn't read/understand the
never deprecate them
part of that.To me, the goal here is to make the purpose and content of the template files less ambiguous. Currently
_t
indicates that they're templates, but doesn't say much about what kind of templating that is..jinja
indicates both template, and a templating language that's in use.(so, correcting myself: supporting both sounds good, but I would like to suggest deprecating
_t
at some point)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we supported both suffixes
_t
and.jinja
equally: would there be any situations where we'd recommend using_t
instead of.jinja
?