-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Revert "Support and prefer .jinja
to _t
for static templates (#11165)"
#11329
Revert "Support and prefer .jinja
to _t
for static templates (#11165)"
#11329
Conversation
60972ca
to
7279428
Compare
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.
Thanks for your concern. One worry is that third-party extensions may be enticed by deprecation warning to adopt .jinja
suffix thus breaking usage with older Sphinx for end-users. Let's way for other maintainers opinion.
I agree with this, at the very least we should likely not issue the deprecation warning initially (perhaps control via an opt-in environment variable?), and improve the wording to be more directive.
I don't think this is a realistic concern (happy to be proven wrong!) -- this suggests that there would be a second subsequent application of Jinja by some non-Sphinx tool? I also don't know what the useful work-around would be for those wanting to preserve current plain copying behaviour. A |
I think we should revert iff we have evidence of the A |
Ok. I've re-read the deprecation policy, and would prefer to re-use existing Sphinx and Python mechanisms (that is: I'm not sure I'm keen on a custom environment variable). If we update the warning message to emphasize the pending aspect of the deprecation: providing guidance on how to filter the warning, instead of recommending switching the suffix immediately: would that be acceptable? (the idea being: the
I'm not sure how realistic it is either, but it was an additional doubt. I haven't seen it in practice anywhere; my worry was webservers configured to render templates dynamically. As a workaround, I think that adding a duplicate |
Yes, but every single Jinja construct would need to be escaped! (Or: we add special handling for this case, I'm not sure which is less-surprising.) A |
True - that'd be both mindbending and quite an obscure way to structure part of an application. Ok: perhaps for |
@humitos -- does Read The Docs' data include what files are copied or in source trees (and therefore the proportion of files with a particular suffix)? No worries if not, thought I'd ask you to try and get some empirical data here! A |
I've decided to defer this -- I still think that we can press ahead with the work, but some changes need to be made (most notably related to when and how we warn users), and I want to release 6.2. A |
Thanks @AA-Turner - I'd still like to make this change too, but feel like I could do with taking a week or two away from the code before returning to it for a fresh attempt. It seems like it should be straightforward when planned and deployed carefully, but I got into a bit of a tangle somehow while (not) thinking it through. |
I'm almost feeling ready to take another look at this - maybe a little more mental preparation first, though. I'd like to make sure some of the linkchecker changes I've been working on are all tidied up and in good shape before re-attempting this, to make sure I can focus (relatively!) clearly on it. Given the reaction to the deprecation warning in the prerelease phase for 6.2.0, I think that @mgeier's original suggestion to roll-out the migration functionality before a deprecation warning is issued would make sense. So, I'm currently thinking:
|
I would like to (again) suggest to support both suffixes indefinitely. Phase 1 sounds good, Phase 2 and 3 can be postponed indefinitely. |
What's the reason you'd prefer to support the (I'm OK with phase 2 and 3 taking a while, but I think it's generally likely to be beneficial and clearer to users and various other ecosystem elements if the purpose of the template files is represented by an appropriate file suffix, and that we should encourage people in that direction) |
To avoid friction for casual users. I think [citation needed] that there are many users who have docs generated by Sphinx, but their docs rarely change and those people might have other things to worry about than regularly checking if a Sphinx update has broken their docs. And if they are using an unmaintained Sphinx extension that uses Note that [citation needed] many Sphinx users are not programmers, so a "simple" change from Of course some breakage is unavoidable in a project as dynamic as Sphinx, but I think all contributors and maintainers should strive to reduce the number and frequency of breakages. And this is a prime example of a 100% avoidable breakage. I think the Sphinx docs can simply recommend using And sure, at some point the |
This reverts commit 5d13215.
Feature or Bugfix
Purpose
OldJinjaSuffixWarning
foralabaster.css_t
in unreleased Sphinx 6.2 alabaster#203 and the related build logs, I don't feel confident that Support and prefer.jinja
to_t
for static templates #11165 is ready for inclusion in Sphinx 6.2.0Detail
alabaster
and other themes would require updating the minimum supported Sphinx version for each of those themes -- because they'd become incompatible with older versions of Sphinx..jinja
suffix, and expect those files to be copied statically (without any templating applied), then Support and prefer.jinja
to_t
for static templates #11165 would break those workflows.These are manageable concerns, but they'd be better addressed if the changes are communicated clearly, and I think that at least one additional release -- and then a good duration of time to allow affected extensions/sites to adjust accordingly -- prior to supporting the
.jinja
suffix could make sense to achieve that.Relates
.jinja
to_t
for static templates #11165