#500 closed enhancement (fixed)
[patch] Templates files extensions other than '.html' should be possible
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | Template system | Version: | |
Severity: | normal | Keywords: | |
Cc: | janc@…, paul.bowsher@… gomo@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently templates are "forced" to end with '.html', which confuses syntax highlighting support in most editors when editing templates that are not HTML. It would be nice if people could define the allowed template file extensions themselves.
Attachments (5)
Change History (27)
by , 19 years ago
Attachment: | template_file_extensions_diff added |
---|
comment:1 by , 19 years ago
Status: | new → assigned |
---|
This is an easy fix. I'll get on it as soon as I can.
comment:3 by , 19 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
A project wide setting for this isn't really appropriate in my opinion. My app for example will use the template system for xhtml, xml, emails, and server configuration files. I think a better solution would be to assume .html is required, unless a .something extension is specified in the template loader. Example:
template_loader.get_template('polls/index')
would use polls/index.html
template_loader.get_template('polls/pollcreated.eml')
would use polls/pollcreated.eml
Shame I missed this issue to be honest, I knew I'd be encountering it at some point and was prepared to write a patch to do what i've outlined above. I've not got to that stage yet, but when I do I'll get a patch written
comment:4 by , 19 years ago
Cc: | added |
---|
Boffbowsh, I think you can do what you want with my patch and:
TEMPLATE_FILE_EXTENSIONS = ( ".html", # try adding .html, because otherwise the admin breaks "", # an empty string keeps the filename as provided to template_loader.get_template() )
comment:5 by , 19 years ago
Cc: | added; removed |
---|
Awesome! I think using JanC's patch instead of Adrian's would be a better idea. Bit of extra flexibility never hurt anyone
by , 19 years ago
Attachment: | template_file_extensions.diff added |
---|
New patch based on revision 917. Not tested with all the new 'loaders' yet (e.g. I have no 'egg' to test).
comment:6 by , 19 years ago
Cc: | added; removed |
---|
comment:7 by , 19 years ago
Seems to me the default shouldn't really be html. Seeing that it can be used to make any text format anyhow there no real reason for it to be .html. Perhaps it would be best to make a new extension, .pytmpl maybe.
Would allow easy detection by editors and allow it to get a custom icon where possible (Say the TextMate sidebar)
It is a bit late to make a change like this... but on the other hand better to make it now than even later.
comment:8 by , 19 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
I'm uncomfortable with this solution, because it leads to way too many filesystem reads (looking for templates).
comment:9 by , 19 years ago
Unless there is something I don't see, my patch used with the default settings didn't/doesn't[1] require more file reads (it works exactly the same as it does now), and also not when used as Infininight wants to use it[2]. People who want to use several template filename extensions should know what they are doing and they can minimize the number of file reads by putting the most used extension first (and there could be a warning in the docs about this).
[1] I would have to check if it still applies, but updating it shouldn't be difficult.
[2] Infininight can already do what he wants _now_ of course...
comment:10 by , 19 years ago
How about something like this? Only append TEMPLATE_FILE_EXTENSION when necessary. This doesn't require any extra options and doesn't cause any extra filesystem activity. As Boffbowsh suggested.
from os import path # Append TEMPLATE_FILE_EXTENSION unless an extension has # been specified if (path.splitext(template_name)[1] == ''): template_name + TEMPLATE_FILE_EXTENSION # ... for template_dir in template_dirs: filepath = os.path.join(template_dir, template_name)
comment:11 by , 19 years ago
Err, that should read template_name = template_name + TEMPLATE_FILE_EXTENSION
, of course.
comment:12 by , 19 years ago
Cc: | added; removed |
---|
Ok I just came in through the window and I think the easy way to solve this is just specify the goddamn extension when you load the template. What's the problem with that? I actually felt quite surprised when i first discovered that Django will append ".html" to your template string before loading it. Removing this sort of behaviour should be part of RemovingTheMagic IMO.
comment:13 by , 19 years ago
Resolution: | wontfix |
---|---|
Status: | closed → reopened |
GomoX: some existing templates (including the admin!) presume ".html" will be attached; those break when no ".html" is added. That was the original reason why I wanted multiple extensions (I consider non-HTML templates ending in .html ugly).
So your proposal is okay to me, if the admin and other existing stuff gets patched too... :-)
comment:14 by , 19 years ago
I agree with Jan Rademaker and Boffbowsh. The solution does not lead to an increase in filesystem reads, is backwardly compatible, and gives us the flexibility to use other extensions.
I guess the only backwards compatible issue is if someone uses a template like 'my.food.html'
by , 19 years ago
Attachment: | optional_template_file_extensions.diff added |
---|
Optional template file extensions (based on Jan Rademaker's idea)
comment:15 by , 19 years ago
SmileyChris, your patch still breaks the admin app if someone changes the default extension from ".html" to something else...
by , 19 years ago
Attachment: | optional_template_file_extensions.2.diff added |
---|
Updated patch, now it won't break admin if TEMPLATE_FILE_EXTENSION is changed
by , 19 years ago
Attachment: | optional_template_file_extensions.3.diff added |
---|
Bug fixes to the previous patch.
comment:16 by , 19 years ago
Sorry for all the spam ;)
So to summarise:
There is a bug in the current implementation by Adrian: if TEMPLATE_FILE_EXTENSION is changed, the admin views break.
My patch (based on ideas from Jan Rademaker and Boffbowsh) allows the explicit use of template file extensions in views and template extend calls. If no extension is given, TEMPLATE_FILE_EXTENSION is used. If TEMPLATE_FILE_EXTENSION differs from the new ADMIN_TEMPLATE_FILE_EXTENSION setting, both are checked (admin last).
Adrian's wontfix reasons are mostly negated. Extra filesystem calls are only needed if TEMPLATE_FILE_EXTENSION has changed (and even then, only if extensions are not explicitly given). These extra calls for the admin extension (.html) happen at the end of all other template location checks, so they are the "last resort".
This patch does break backward compatibility where files with more than one "." have been used as templates (for example "main.css.html")
comment:17 by , 19 years ago
Summary: | Templates files extensions other than '.html' should be possible → [patch] Templates files extensions other than '.html' should be possible |
---|
comment:18 by , 19 years ago
milestone: | → Version 0.92 |
---|
I would be for removing TEMPLATE_FILE_EXTENSION altogether, as GomoX suggested. I can't see what use it is -- as Chris pointed out, if you are using admin at all it is currently useless since you can't change it. Even if you are not, you are quite likely to want a number of different file extensions. Like GomoX, I was surprised that I didn't specify the .html myself, especially for a template system which is not HTML specific -- it doesn't make much sense, and it caught me out a number of times. AFAICS, the only advantage to the current behaviour is that you save 5 keystrokes here and there, but that comes at the expense of a lot of flexibility and problems with editors etc.
This would be a backwards incompatible change, but one that is probably easier to fix than most of the other changes in magic-removal. The main things you would have to grep for would be:
{% extends %}
in the templatesrender
,render_to_response
and probably some others- generic view functions
I'm happy to do the patch for the admin etc if Adrian agrees, and I can report on how much pain it was (if that is a metric that will sway things) and document everything on the BackwardsIncompatibleChanges page.
The "magic-removal" branch aims to make several sweeping changes to the Django codebase, removing warts that Django has accumulated over the years
Fits the bill I think.
comment:19 by , 19 years ago
Luke, are you sure it's better to remove TEMPLATE_FILE_EXTENSION altogether? My patch makes it work happily with admin.
Most of the time you'd want ".htm" or ".html" as your default but you should have the option to specify an alternate explicitly.
Maybe this is a bit of magic however. I guess it would be simpler to always have to supply the extension.
comment:20 by , 19 years ago
Chris,
I'm not sure -- it's an opinion, but I don't think the convenience of the having it add '.html' automatically is worth the complications it adds, even if that case covers 95% of the usage or more.
I should mention, in regards to your patch, that I have often in the past used files with more than one dot in the filename. For example, I had files which needed one extension for use with one tool, and I had my editor configured to recognise a 'compound extension' e.g. things like foo.html.php. I don't have that situation now, but I have no doubt that people will come along who do, as Django becomes more and more popular. At that point, if it is post 1.0, we will have bugs that we can't/won't fix, but right now we have the opportunity to do it right, especially with the magic-removal branch, and not make life harder in the future for the Django maintainers or users.
Put it this way -- if we were in a situation where the template engine didn't add .html to file names, I'm pretty sure we would leave it as is, knowing the complications it brings in.
comment:21 by , 19 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
a small diff to implement changes that seems to allow for this, but might need more testing :)