Opened 18 years ago

Closed 18 years ago

Last modified 17 years ago

#583 closed enhancement (fixed)

[patch] Add app-template dirs to TEMPLATE_DIRS

Reported by: sune.kirkeby@… Owned by: Adrian Holovaty
Component: Template system Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This patch automatically adds templates/ directories under app-modules to TEMPLATE_DIRS. This is nice :)

Attachments (4)

settings.patch (1.0 KB ) - added by sune.kirkeby@… 18 years ago.
settings-v2.patch (1.6 KB ) - added by sune.kirkeby@… 18 years ago.
settings-v3.patch (1.6 KB ) - added by sune.kirkeby@… 18 years ago.
Python 2.3 safe patch
apploader.py (889 bytes ) - added by hugo 18 years ago.
template loader that looks into app directories

Download all attachments as: .zip

Change History (18)

by sune.kirkeby@…, 18 years ago

Attachment: settings.patch added

comment:1 by Adrian Holovaty, 18 years ago

In your patch, what does this comment mean?

"There is a good reason i jump through these hoops, if the original TEMPLATE_DIRS setting was a list-instance, we should keep it that way."

comment:2 by Adrian Holovaty, 18 years ago

This solution could have some performance issues for installations with a lot of values in INSTALLED_APPS. For that reason, I'm a bit weary of including the patch.

That said, maybe it could be optional, according to a GET_TEMPLATES_IN_APPS setting, which would be set to False by default.

comment:3 by sune.kirkeby@…, 18 years ago

"There is a good reason i jump through these hoops, if the original TEMPLATE_DIRS setting was a list-instance, we should keep it that way."

In Django Developer Gadgets I introduced the template_dirs_hacker middleware, it depends on TEMPLATE_DIRS being a list, so it can change it with slice-assignment. That was what I was referring to, sorry if that was a bit obscure for anyone else :)

comment:4 by sune.kirkeby@…, 18 years ago

This solution could have some performance issues for installations with a lot of values in INSTALLED_APPS. For that reason, I'm a bit weary of including the patch.

I don't see how; the patch only includes app-templates directories if they actually exist, and is only executed when the settings module is imported initially. But, if you still want it, I can add a GET_TEMPLATES_IN_APPS setting.

comment:5 by Adrian Holovaty, 18 years ago

Yeah, I'm super-concerned about keeping startup costs low, so a GET_TEMPLATES_IN_APPS setting would be best.

comment:6 by sune.kirkeby@…, 18 years ago

I'm super-concerned about keeping startup costs low,

Hmm... Right. I can't say that I can see why; but it's your call. Here is a new patch, which adds a ADD_APP_TEMPLATE_DIRS setting; I changed the name, to better describe what it does. Also, I would argue that you should change the default value to True, and let the convenience of the common user come before the efficience of initilization for the one-in-thousand user...

by sune.kirkeby@…, 18 years ago

Attachment: settings-v2.patch added

comment:7 by eugene@…, 18 years ago

I can't say that I can see why

I can say why: FastCGI. Speed of the process creation becomes crucial, especially under heavy load.

by sune.kirkeby@…, 18 years ago

Attachment: settings-v3.patch added

Python 2.3 safe patch

comment:8 by Adrian Holovaty, 18 years ago

I just chatted about this on IRC with Hugo, and he suggested we use the new TEMPLATE_LOADERS setting (from [870]) to accomplish this -- say, by adding something like django.core.template.loaders.autoapp to that setting. This solution would mean we wouldn't be creating another setting. The problem with this, though, is that the autoapp loader would have to know how to load from the filesystem *and* from eggs, which is redundant. Thoughts?

comment:9 by Adrian Holovaty, 18 years ago

Status: newassigned

comment:10 by hugo, 18 years ago

Hmm. When I played with the template loaders, I just added two template loaders to the list (one the dbtemplate loader, the other the standard template loader). Templates where first searched in the first one and next in the second one if they didn't exist in the first - so just add the egg loader _and_ the autoapp loader would give you exactly what extending the template directories would. I thought that's why the TEMPLATE_LOADERS thingy is a list - to have multiple of them installed. Or am I missing something?

The autoapp thingy would just be a "decentralized file storage" - for example it could have some kind of caching of directory content so that it only searches in the directories when the content is actually changed, that would remove penalties from having many apps (and therefore many template directories to check).

by hugo, 18 years ago

Attachment: apploader.py added

template loader that looks into app directories

comment:11 by hugo, 18 years ago

I attached a sample implementation of such a app-path-loader. It doesn't do any caching of directory contents, that's left as an exercise for the reader ;-)

comment:12 by Adrian Holovaty, 18 years ago

Ah, but your apploader.py example doesn't handle loading templates from eggs, which was my point. I'm considering moving the admin into a standalone app in contrib (see #627), and that would mean the default admin templates would live within the admin app, rather than in django/conf/admin_templates.

This arrangement would need to work with eggs, too (see #596), so that means the auto-app-path loader would need to work with both filesystem- and egg-based templates. So that's why I'm leaning toward a GET_TEMPLATES_IN_APPS setting.

comment:13 by Adrian Holovaty, 18 years ago

On second thought, it'd probably be cleaner if it were a template loader rather than a separate setting, because the auto-app thing would have to look for installed apps in both filesystem and eggs either way.

comment:14 by Adrian Holovaty, 18 years ago

Resolution: fixed
Status: assignedclosed

(In [892]) Fixed #583 -- Added app_directories template loader, which searches for templates in 'templates' directory in each INSTALLED_APPS package. It's turned off by default.

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