#17199 closed Bug (invalid)
Base Template Loader should return the origin
Reported by: | German M. Bravo | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | German M. Bravo, bmihelac@… | Triage Stage: | Design decision needed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've been trying to get the origin from the loaders, but the origin was lost when BaseLoader didn't return it. I believe it should return origin... any thoughts? I'm attaching a patch.
Attachments (1)
Change History (11)
comment:1 by , 13 years ago
Cc: | added |
---|---|
Component: | Uncategorized → Template system |
Has patch: | set |
Type: | Uncategorized → Bug |
by , 13 years ago
Attachment: | #17199-template_loader_origin.diff added |
---|
comment:3 by , 13 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
Version: | → SVN |
Custom
Loader
classes should inherit fromdjango.template.loader.BaseLoader
and override theload_template_source()
method, which takes atemplate_name
argument, loads the template from disk (or elsewhere), and returns a tuple:(template_string, template_origin)
.
The
load_template()
method of theLoader
class retrieves the template string by callingload_template_source()
, instantiates aTemplate
from the template source, and returns a tuple:(template, template_origin)
I hope this clarifies the expected behavior. With this new information, could you explain what Django does wrong exactly, and provide some steps to reproduce the issue -- or upload a test case? Thanks!
comment:4 by , 13 years ago
Using TEMPLATE_DEBUG = True
(so that origin is returned, otherwise origin is never actually returned, see make_origin()
in django.template.loader
), if I do: template, origin = loader.find_template('mytemplate.html')
, it does not return origin (this is because load_template()
in BaseLoader
explicitly returns None instead of the origin). Same thing with the load_template()
in the "cached" loader. It returns the tuple (template, None)
.
After reading that bit of documentation, I can see load_template()
doesn't even remotely return (as it should) a template_origin, but a string with the display_name
(and only if the template is not found). I could also make a new patch that would make django consistent with said behavior in the documentation, however, BaseLoader.load_template()
currently already returns display_name
instead of the origin
, and simply changing that could break some things.
In my opinion, as I said first, load_template()
should always return origin (as stated in the documentation) and not display_name
... and only when the template is not found. Whether it should always return origin (even if TEMPLATE_DEBUG
is not enabled) is yet another dilemma, as the documentation never states load_template()
returns template_origin
*only* if TEMPLATE_DEBUG
is enabled.
comment:5 by , 13 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → reopened |
I'm reopening because I truly see this being wrong in some way... or I might be. After further digging, it truly seems there are some inconsistencies, and I need the be able to get origin of the template while using django.template.loader.find_template()
, which I currently can't, the way django code is now.
comment:6 by , 13 years ago
Cc: | added |
---|
comment:7 by , 13 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:8 by , 12 years ago
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
#16096 (filed by a core dev) suggests that origin
is purely for debug puposes. That would explain why it's set only when the template isn't found and when TEMPLATE_DEBUG = True
.
As far as I can tell, you're trying to use the origin
returned by BaseLoader.find_template()
for much more than its purpose.
Wouldn't #16096 be a much more simple solution for the problem you're trying to solve?
comment:9 by , 11 years ago
It is true that origin
is currently used only for debugging purposes, but it could be used for much more. For example, in django-overextends it would allow to better detect cycles between templates. If a tag could know what is the filename of the current template it would know which templates it has to find when extending templates with the same name (should it hide the current one to prevent a cycle or not - it should hide only if current template filename is the same as the filename in extends
).
It is true that only origin name would be needed, not whole origin (with template content).
comment:10 by , 11 years ago
See an example implementation of a extends
tag which allows arbitrary nesting of extends and prevents import cycles. But for it to work, origin information is needed.
I'm now not sure if template loaders should return the tempalte and the origin, or the template and the display_name. I guess they should return the origin instead, as it has more information... My doubt rises from the fact BaseLoader returns display_name, but seemingly, cached template loader returns origin when load_template() fails to find the template... My patch returns display_name (from origin.name) as it seems to be consistent with all the other load_template() methods.