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

Support and prefer .jinja to _t for static templates #11165

Merged
merged 40 commits into from Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3e735ee
Templating: use '.jinja2' suffix on Jinja2 template files instead of …
jayaddison Jan 31, 2023
5ce9483
ruff suggestion: adopt union typing syntax (with support provided by …
jayaddison Feb 3, 2023
168cb23
Merge branch 'master' into templating/jinja2-suffix
jayaddison Feb 13, 2023
4a8a339
Update file extension: the 'jinja2' project has been renamed to 'jinj…
jayaddison Feb 19, 2023
eb9938a
Merge branch 'master' into templating/jinja2-suffix
jayaddison Feb 19, 2023
43f8b2b
Fixup: ruff lint error (missing trailing comma after multi-line multi…
jayaddison Feb 19, 2023
b2889de
Fixup: adjust trim-length for template suffix (was: seven characters …
jayaddison Feb 19, 2023
a720933
Localization: undo changes to resource files
jayaddison Mar 21, 2023
6f99cef
Merge branch 'master' into templating/jinja2-suffix
jayaddison Mar 21, 2023
d2af9b5
Logic adjustment and refactor: rename '_template_basename' utility me…
jayaddison Apr 6, 2023
526fdc7
Emit a more-generic PendingDeprecationWarning instead of RemovedInSph…
jayaddison Apr 6, 2023
cd0d61f
CHANGES: document the deprecation of the '_t' file-copy template suffix
jayaddison Apr 6, 2023
5ab2226
Code commentary / feature removal timeline: provide two years from to…
jayaddison Apr 6, 2023
b8570ab
Merge branch 'master' into templating/jinja2-suffix
jayaddison Apr 6, 2023
59534ea
CHANGES: Add issue thread number reference
jayaddison Apr 6, 2023
e38e78e
Cleanup: remove unused import
jayaddison Apr 6, 2023
6305f96
Documentation: update hint related to LaTeX template filenames
jayaddison Apr 6, 2023
dc850b7
Babel resource extraction: update latex template method mappings to i…
jayaddison Apr 6, 2023
04aa946
Babel resource extraction: update javascript template method mapping …
jayaddison Apr 6, 2023
8572661
Babel resource extraction: update latex template options to include r…
jayaddison Apr 6, 2023
0eb993d
Revert changes to latex file rendering
jayaddison Apr 6, 2023
0444616
LaTeX templating: re-migrate to preferred '.tex.jinja' template suffi…
jayaddison Apr 6, 2023
29fb6f0
Fixup: migrate imgmath LaTeX template to use .jinja suffix
jayaddison Apr 6, 2023
1630cc8
Test coverage: ensure that we have test coverage for the legacy LaTeX…
jayaddison Apr 6, 2023
d85f016
Fixup: migrate built-in preview.tex LaTeX template
jayaddison Apr 6, 2023
8475023
Merge branch 'master' into templating/jinja2-suffix
AA-Turner Apr 6, 2023
bc43d2e
CHANGES: Update deprecation note to mention built-in templates
jayaddison Apr 6, 2023
06aa004
imgmath LaTeX templating: provide backwards-compatibility in the case…
jayaddison Apr 6, 2023
7b061cf
Safety in code path where a multi-item iterable is expected to be red…
jayaddison Apr 6, 2023
abd8af5
Fixup: add missing import
jayaddison Apr 6, 2023
d8e3fa7
Revert "Logic adjustment and refactor: rename '_template_basename' ut…
jayaddison Apr 6, 2023
30c7318
Fixup: imgmath: fallback to template filename with .jinja suffix
jayaddison Apr 6, 2023
d0aa68a
Template suffix iteration: use the 'tuple' datatype instead of 'list'…
jayaddison Apr 7, 2023
299dbed
Documentation: add explanatory docstring to utility '_template_basena…
jayaddison Apr 7, 2023
c3b5cb9
Adjust date, add helper function
AA-Turner Apr 7, 2023
6600615
Add tests
AA-Turner Apr 7, 2023
9327357
Update ``theming.rst``
AA-Turner Apr 7, 2023
cccdfaf
Update CHANGES
AA-Turner Apr 7, 2023
cb9ea44
left -> removed
AA-Turner Apr 7, 2023
9a4410a
credits
AA-Turner Apr 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES
Expand Up @@ -23,6 +23,9 @@ Deprecated
----------

* #11247: Deprecate the legacy ``intersphinx_mapping`` format
* #11165: The ``_t`` suffix for templating (``sphinx.util.copy_asset()``,
``sphinx.util.copy_asset_file()`` and built-in templates) is now deprecated.
Copy link
Contributor

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"?

Copy link
Contributor Author

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:

I was wondering though: since it seems to be reasonably simple to support both _t and .jinja for a deprecation period, why not support both indefinitely and never deprecate them?

That seems like a good approach to me 👍

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)

Copy link
Contributor Author

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?

It's recommended to use ``.jinja`` as a filename suffix instead.

Features added
--------------
Expand Down
8 changes: 4 additions & 4 deletions doc/latex.rst
Expand Up @@ -1811,12 +1811,12 @@ Miscellany
.. hint::

As an experimental feature, Sphinx can use user-defined template file for
LaTeX source if you have a file named ``_templates/latex.tex_t`` in your
LaTeX source if you have a file named ``_templates/latex.tex.jinja`` in your
project.

Additional files ``longtable.tex_t``, ``tabulary.tex_t`` and
``tabular.tex_t`` can be added to ``_templates/`` to configure some aspects
of table rendering (such as the caption position).
Additional files ``longtable.tex.jinja``, ``tabulary.tex.jinja`` and
``tabular.tex.jinja`` can be added to ``_templates/`` to configure some
aspects of table rendering (such as the caption position).
jayaddison marked this conversation as resolved.
Show resolved Hide resolved

.. versionadded:: 1.6
currently all template variables are unstable and undocumented.
Expand Down
5 changes: 3 additions & 2 deletions sphinx/builders/_epub_base.py
Expand Up @@ -603,7 +603,8 @@ def build_content(self) -> None:
html.escape(self.refnodes[0]['refuri'])))

# write the project file
copy_asset_file(path.join(self.template_dir, 'content.opf_t'), self.outdir, metadata)
content_t = path.join(self.template_dir, 'content.opf.jinja')
copy_asset_file(content_t, self.outdir, metadata)

def new_navpoint(self, node: dict[str, Any], level: int, incr: bool = True) -> NavPoint:
"""Create a new entry in the toc from the node at given level."""
Expand Down Expand Up @@ -686,7 +687,7 @@ def build_toc(self) -> None:
navpoints = self.build_navpoints(refnodes)
level = max(item['level'] for item in self.refnodes)
level = min(level, self.config.epub_tocdepth)
copy_asset_file(path.join(self.template_dir, 'toc.ncx_t'), self.outdir,
copy_asset_file(path.join(self.template_dir, 'toc.ncx.jinja'), self.outdir,
self.toc_metadata(level, navpoints))

def build_epub(self) -> None:
Expand Down
8 changes: 4 additions & 4 deletions sphinx/builders/changes.py
Expand Up @@ -133,10 +133,10 @@ def hl(no: int, line: str) -> str:
f.write(self.templates.render('changes/rstsource.html', ctx))
themectx = {'theme_' + key: val for (key, val) in
self.theme.get_options({}).items()}
copy_asset_file(path.join(package_dir, 'themes', 'default', 'static', 'default.css_t'),
self.outdir, context=themectx, renderer=self.templates)
copy_asset_file(path.join(package_dir, 'themes', 'basic', 'static', 'basic.css'),
self.outdir)
default_t = path.join(package_dir, 'themes', 'default', 'static', 'default.css.jinja')
copy_asset_file(default_t, self.outdir, context=themectx, renderer=self.templates)
basic = path.join(package_dir, 'themes', 'basic', 'static', 'basic.css')
copy_asset_file(basic, self.outdir)

def hl(self, text: str, version: str) -> str:
text = html.escape(text)
Expand Down
2 changes: 1 addition & 1 deletion sphinx/builders/epub3.py
Expand Up @@ -184,7 +184,7 @@ def build_navigation_doc(self) -> None:
# 'includehidden'
refnodes = self.refnodes
navlist = self.build_navlist(refnodes)
copy_asset_file(path.join(self.template_dir, 'nav.xhtml_t'), self.outdir,
copy_asset_file(path.join(self.template_dir, 'nav.xhtml.jinja'), self.outdir,
self.navigation_doc_metadata(navlist))

# Add nav.xhtml to epub file
Expand Down
2 changes: 1 addition & 1 deletion sphinx/builders/gettext.py
Expand Up @@ -283,7 +283,7 @@ def finish(self) -> None:
ensuredir(path.join(self.outdir, path.dirname(textdomain)))

context['messages'] = list(catalog)
content = GettextRenderer(outdir=self.outdir).render('message.pot_t', context)
content = GettextRenderer(outdir=self.outdir).render('message.pot.jinja', context)

pofn = path.join(self.outdir, textdomain + '.pot')
if should_write(pofn, content):
Expand Down
4 changes: 2 additions & 2 deletions sphinx/builders/latex/__init__.py
Expand Up @@ -402,7 +402,7 @@ def copy_support_files(self) -> None:
# use pre-1.6.x Makefile for make latexpdf on Windows
if os.name == 'nt':
staticdirname = path.join(package_dir, 'texinputs_win')
copy_asset_file(path.join(staticdirname, 'Makefile_t'),
copy_asset_file(path.join(staticdirname, 'Makefile.jinja'),
self.outdir, context=context)

@progress_message(__('copying additional files'))
Expand Down Expand Up @@ -441,7 +441,7 @@ def write_message_catalog(self) -> None:
if self.context['babel'] or self.context['polyglossia']:
context['addtocaptions'] = r'\addto\captions%s' % self.babel.get_language()

filename = path.join(package_dir, 'templates', 'latex', 'sphinxmessages.sty_t')
filename = path.join(package_dir, 'templates', 'latex', 'sphinxmessages.sty.jinja')
copy_asset_file(filename, self.outdir, context=context, renderer=LaTeXRenderer())


Expand Down
18 changes: 9 additions & 9 deletions sphinx/cmd/quickstart.py
Expand Up @@ -370,29 +370,29 @@ def write_file(fpath: str, content: str, newline: str | None = None) -> None:
if 'quiet' not in d:
print(__('File %s already exists, skipping.') % fpath)

conf_path = os.path.join(templatedir, 'conf.py_t') if templatedir else None
conf_path = os.path.join(templatedir, 'conf.py.jinja') if templatedir else None
if not conf_path or not path.isfile(conf_path):
conf_path = os.path.join(package_dir, 'templates', 'quickstart', 'conf.py_t')
conf_path = os.path.join(package_dir, 'templates', 'quickstart', 'conf.py.jinja')
with open(conf_path, encoding="utf-8") as f:
conf_text = f.read()

write_file(path.join(srcdir, 'conf.py'), template.render_string(conf_text, d))

masterfile = path.join(srcdir, d['master'] + d['suffix'])
if template._has_custom_template('quickstart/master_doc.rst_t'):
if template._has_custom_template('quickstart/master_doc.rst.jinja'):
msg = ('A custom template `master_doc.rst_t` found. It has been renamed to '
'`root_doc.rst_t`. Please rename it on your project too.')
print(colorize('red', msg))
write_file(masterfile, template.render('quickstart/master_doc.rst_t', d))
write_file(masterfile, template.render('quickstart/master_doc.rst.jinja', d))
else:
write_file(masterfile, template.render('quickstart/root_doc.rst_t', d))
write_file(masterfile, template.render('quickstart/root_doc.rst.jinja', d))

if d.get('make_mode') is True:
makefile_template = 'quickstart/Makefile.new_t'
batchfile_template = 'quickstart/make.bat.new_t'
makefile_template = 'quickstart/Makefile.new.jinja'
batchfile_template = 'quickstart/make.bat.new.jinja'
else:
makefile_template = 'quickstart/Makefile_t'
batchfile_template = 'quickstart/make.bat_t'
makefile_template = 'quickstart/Makefile.jinja'
batchfile_template = 'quickstart/make.bat.jinja'

if d['makefile'] is True:
d['rsrcdir'] = 'source' if d['sep'] else '.'
Expand Down
7 changes: 4 additions & 3 deletions sphinx/ext/apidoc.py
Expand Up @@ -102,7 +102,7 @@ def create_module_file(package: str, basename: str, opts: Any,
'qualname': qualname,
'automodule_options': options,
}
text = ReSTRenderer([user_template_dir, template_dir]).render('module.rst_t', context)
text = ReSTRenderer([user_template_dir, template_dir]).render('module.rst.jinja', context)
write_file(qualname, text, opts)


Expand Down Expand Up @@ -138,7 +138,8 @@ def create_package_file(root: str, master_package: str, subroot: str, py_files:
'show_headings': not opts.noheadings,
'maxdepth': opts.maxdepth,
}
text = ReSTRenderer([user_template_dir, template_dir]).render('package.rst_t', context)
engine = ReSTRenderer([user_template_dir, template_dir])
text = engine.render('package.rst.jinja', context)
write_file(pkgname, text, opts)

if submodules and opts.separatemodules:
Expand All @@ -163,7 +164,7 @@ def create_modules_toc_file(modules: list[str], opts: Any, name: str = 'modules'
'maxdepth': opts.maxdepth,
'docnames': modules,
}
text = ReSTRenderer([user_template_dir, template_dir]).render('toc.rst_t', context)
text = ReSTRenderer([user_template_dir, template_dir]).render('toc.rst.jinja', context)
write_file(name, text, opts)


Expand Down
24 changes: 17 additions & 7 deletions sphinx/ext/imgmath.py
Expand Up @@ -7,6 +7,7 @@
import shutil
import subprocess
import tempfile
import warnings
from os import path
from subprocess import CalledProcessError
from typing import Any
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 DeprecationWarning should be created, and after another year or two support for _t can be removed.

for template_suffix in ('_t', '.jinja'):
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:
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
23 changes: 20 additions & 3 deletions sphinx/util/fileutil.py
Expand Up @@ -4,6 +4,7 @@

import os
import posixpath
import warnings
from typing import TYPE_CHECKING, Callable

from docutils.utils import relative_path
Expand All @@ -15,6 +16,23 @@
from sphinx.util.template import BaseRenderer


def _template_basename(filename: str) -> str | None:
"""Given an input filename:
If the input looks like a template, then return the filename output should
be written to. Otherwise, return no result (None)."""
# TODO: remove "_t" template suffix support after 2025-04-06
Copy link
Contributor

Choose a reason for hiding this comment

The 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. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
f"{filename!r}: filename suffix '_t' for templates is deprecated. "
f"{filename!r}: filename suffix '_t' for templates will become deprecated. "

"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:
Expand All @@ -35,14 +53,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:
Expand Down
32 changes: 23 additions & 9 deletions sphinx/writers/latex.py
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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'):
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
Expand Down Expand Up @@ -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)
Expand Down
@@ -0,0 +1,2 @@
<# TODO: remove "_t" template suffix support after 2025-04-06 #>
AU REVOIR, KANIGGETS
2 changes: 1 addition & 1 deletion tests/test_build_html.py
Expand Up @@ -1153,7 +1153,7 @@ def test_html_assets(app):
# html_extra_path
assert (app.outdir / '.htaccess').exists()
assert not (app.outdir / '.htpasswd').exists()
assert (app.outdir / 'API.html_t').exists()
assert (app.outdir / 'API.html.jinja').exists()
assert (app.outdir / 'css/style.css').exists()
assert (app.outdir / 'rimg.png').exists()
assert not (app.outdir / '_build' / 'index.html').exists()
Expand Down
4 changes: 4 additions & 0 deletions tests/test_build_latex.py
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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']})
Expand Down