Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#500 closed enhancement (fixed)

[patch] Templates files extensions other than '.html' should be possible

Reported by: janc@… Owned by: adrian
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: UI/UX:

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)

template_file_extensions_diff (2.0 KB) - added by janc@… 10 years ago.
a small diff to implement changes that seems to allow for this, but might need more testing :)
template_file_extensions.diff (4.3 KB) - added by JanC 10 years ago.
New patch based on revision 917. Not tested with all the new 'loaders' yet (e.g. I have no 'egg' to test).
optional_template_file_extensions.diff (3.3 KB) - added by SmileyChris 9 years ago.
Optional template file extensions (based on Jan Rademaker's idea)
optional_template_file_extensions.2.diff (5.3 KB) - added by SmileyChris 9 years ago.
Updated patch, now it won't break admin if TEMPLATE_FILE_EXTENSION is changed
optional_template_file_extensions.3.diff (5.3 KB) - added by SmileyChris 9 years ago.
Bug fixes to the previous patch.

Download all attachments as: .zip

Change History (27)

Changed 10 years ago by janc@…

a small diff to implement changes that seems to allow for this, but might need more testing :)

comment:1 Changed 10 years ago by adrian

  • Status changed from new to assigned

This is an easy fix. I'll get on it as soon as I can.

comment:2 Changed 10 years ago by adrian

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in [640].

comment:3 Changed 10 years ago by Boffbowsh

  • Resolution fixed deleted
  • Status changed from closed to 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 Changed 10 years ago by JanC

  • Cc janc@… 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 Changed 10 years ago by Boffbowsh

  • Cc janc@… added; janc@… removed

Awesome! I think using JanC's patch instead of Adrian's would be a better idea. Bit of extra flexibility never hurt anyone

Changed 10 years ago by JanC

New patch based on revision 917. Not tested with all the new 'loaders' yet (e.g. I have no 'egg' to test).

comment:6 Changed 10 years ago by anonymous

  • Cc janc@… paul.bowsher@… added; janc@… removed

comment:7 Changed 9 years ago by Infininight

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 Changed 9 years ago by adrian

  • Resolution set to wontfix
  • Status changed from reopened to closed

I'm uncomfortable with this solution, because it leads to way too many filesystem reads (looking for templates).

comment:9 Changed 9 years ago by JanC

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 Changed 9 years ago by Jan Rademaker <j.rademaker@…>

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 Changed 9 years ago by Jan Rademaker <j.rademaker@…>

Err, that should read template_name = template_name + TEMPLATE_FILE_EXTENSION, of course.

comment:12 Changed 9 years ago by GomoX <gomo@…>

  • Cc paul.bowsher@… added; paul.bowsher@… 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 Changed 9 years ago by JanC

  • Resolution wontfix deleted
  • Status changed from closed to 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 Changed 9 years ago by SmileyChris

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'

Changed 9 years ago by SmileyChris

Optional template file extensions (based on Jan Rademaker's idea)

comment:15 Changed 9 years ago by JanC

SmileyChris, your patch still breaks the admin app if someone changes the default extension from ".html" to something else...

Changed 9 years ago by SmileyChris

Updated patch, now it won't break admin if TEMPLATE_FILE_EXTENSION is changed

Changed 9 years ago by SmileyChris

Bug fixes to the previous patch.

comment:16 Changed 9 years ago by SmileyChris

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 Changed 9 years ago by anonymous

  • Summary changed from Templates files extensions other than '.html' should be possible to [patch] Templates files extensions other than '.html' should be possible

comment:18 Changed 9 years ago by lukeplant

  • milestone set to 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 templates
  • render, 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 Changed 9 years ago by SmileyChris

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 Changed 9 years ago by lukeplant

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 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [2700]) magic-removal: Fixed #500 -- Removed TEMPLATE_FILE_EXTENSION setting so that template loading requires you to give the full filename, including extension. This applies everywhere, even in the 'extends' and 'include' template tags.

comment:22 Changed 9 years ago by adrian

  • milestone Version 0.92 deleted

Milestone Version 0.92 deleted

Note: See TracTickets for help on using tickets.
Back to Top