Code

Opened 6 years ago

Closed 3 months ago

#6834 closed New feature (wontfix)

Add support for templates to be loaded from dynamically-selected directories

Reported by: durin42 <at> gmail.com Owned by: berkerpeksag
Component: Template system Version: master
Severity: Normal Keywords: tplrf-patched
Cc: berker.peksag@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

For an app we're building with Django, it's become a requirement to be able to vary the template load directories based on the request information we get from the user, including the directories read by the extends and includes TemplateTags. The attached patch, along with the one on #4278 gives us all the functionality we need.

Attachments (1)

loader_tags.diff (5.2 KB) - added by durin42 6 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by durin42

comment:1 Changed 6 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 6 years ago by anonymous

  • Needs tests set

comment:3 Changed 6 years ago by emulbreh

  • Keywords tplrf-patched added

This would be fixed by #7815.

comment:4 Changed 4 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

Without evaluating the patch, the concept is sound. So we'll take a good implementation of this (which might be this patch, refreshed to Django 1.2 -- I haven't looked at it).

comment:5 Changed 3 years ago by julien

  • Type set to New feature

comment:6 Changed 3 years ago by julien

  • Severity set to Normal

comment:7 Changed 2 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 2 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 7 months ago by berkerpeksag

  • Cc berker.peksag@… added
  • Patch needs improvement set

comment:10 Changed 7 months ago by berkerpeksag

  • Needs tests unset
  • Owner changed from nobody to berkerpeksag
  • Patch needs improvement unset
  • Status changed from new to assigned

I've opened a pull request on GitHub: https://github.com/django/django/pull/1702

comment:11 Changed 5 months ago by timo

  • Patch needs improvement set

There are some comments on the PR for improvement. I also have a couple more thoughts:

  1. I'm not super enthusiastic about this feature, perhaps partly because the use case is a bit vague -- I'd like to hear more about apps that allow choosing template directories based on user inputs.
  1. Do you know if Jinja2 offers a feature like this? I ask because there has been some talk of switching Django to use Jinja2 rather than using our own template language.
  1. I'd also like to see a test to ensure that directory traverse isn't possible.
  1. The current implementation doesn't seem ideal as it forces the context variable to be called dirs (am I reading that correctly?). Thus, if I have two {% include %} tags and want different directories for each of them, I'd have to jump through some hoops to make that work (like {% with dirs1 as dirs %}{% include "foo.html" dirs %}{% endwith %}) ...

comment:12 Changed 5 months ago by aaugustin

This patch breaks the TEMPLATE_LOADERS abstraction: it's adding in the template language a feature that alters the template loading behavior, but only works with Django's filesystem template loader — not the app directories template loader, for instance.

This ticket may predate the introduction of TEMPLATE_LOADERS. In my opinion it should be closed as wontfix, and the reporter should write a custom template loader. It's going to be a bit difficult to pass the request to the template loader; it may require a global (thread-local) variable or hooking on signal.

Version 0, edited 5 months ago by aaugustin (next)

comment:13 Changed 3 months ago by berkerpeksag

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

Sorry for my *very* late response. I agree with both of you.

Do you know if Jinja2 offers a feature like this? I ask because there has been some talk of switching Django to use Jinja2 rather than using our own template language.

No, Jinja2 has no feature like that, but it is pretty much unmaintained these days.

Closing the ticket as won't fix.

Thanks!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.