remove django.templates __path__ hacking

[oyvind]

As mentioned in VersionOneFeatures

I made a attempt at fixing it, posting git patch

[oyvind]

new template library loader

[oyvind]

better error messages, no conflicting library names, better naming of variables

[oyvind]

Do not walk subdirs in os.walk

[oyvind]

get_library no longer uses django.templatetags.module

[oyvind]

all tests in runtests.py templates now pass

[Malcolm Tredinnick]

A good start. Needs a little more tweaking, though.

Line 19 in the new version of templatetags/__init__.py can't be right. It looks like you're trying to replicate Python's import behaviour, but that's not the way to do it. I would be cautious of anything trying to do with this work without using at least some functions from Python's imp module. See below (the "Finally,..." paragraph) for more here: I feel a bit suspicious about the whole approach in this part of the code.

Lines 8 and 9 in the same file cry out for a better solution. One possible, probably for another ticket, is to add a "tags to load always" list in settings. This would help people who are poking into the builtin tags as well. We could add django.templatetags.* to that list.

Even simpler, and probably sufficient for the purposes of this ticket is to iterate over ['django'] + list(settings.INSTALLED_APPS). That avoids having some special list we have to remember to update if we add more tags in there.

Finally, the approach in get_library_names() looks complex. You're doing a lot of computation there in the hope of getting one hit on the returned dictionary when you try to load a tag. Is it possible to let Python do more of the work here. Loop over the list of directories (<app_name>+'.templatetags') and try appending what is being loaded to the end of that and importing. As soon as you don't get an import error, stop with success. That will also mean if you try to load the same template tags in multiple templates in the the same process, Python will just reuse the one already in memory, instead of walking all those directories again. Push less of the work down into get_library_names() and try to use Python's builtin import facilities (esp. __import__() a bit more.

[oyvind]

More use of import, tests pass

[oyvind]

populate app_label in templatetags/init..py, import_library is own function

[oyvind]

better naming of variables, some code fixes, added some docstrings

[oyvind]

All tests pass

[Marc Fargas]

#3349 talks about ImportError in template loading.

[oyvind]

This patch shows the tried modules, should the message be less specific, and ask about apps in INSTALLED_APPS as in #3349?

[oyvind]

removed os import

[Marc Fargas]

I prefer to know the tried modules, just pointed out #3349 in case it gets invalidated whe this one gets in.
But I doubt it now as I changed #3349 summary (as the patch fixed other places with obscure errors).

[Ramiro Morales]

See also #6579 regarding the use of [''] as the 4th. parameter to ___init__() (with some notes from the Python developers advising against its use.)

[oyvind]

fixed import to get the correct module, moved defaulttags and defaultfilters to avoid circular imports, fixes #6579

[oyvind]

forgot to remove some files in the commit, again

[oyvind]

new loader using imp, may need some cleaup, and decide if add_to_builtins should change, and if a import should use a list instead of the getattr solution

[oyvind]

removed import using getattr

[oyvind]

Final patch, i hope :)

[Alex Gaynor]

1.0 beta has shipped and this was not fixed, will be handled post-1.0.

[Malcolm Tredinnick]

I realise people are only trying to help, but can we have a little restraint with triaging tickets off the feature list? Particularly when a maintainer is currently the assigned person. This is still on the 1.0 track. I can do it in a more-or-less backwards-compatible way, with luck.

[Ramiro Morales]

#6579 was related and marked as duplicate.

[Antti Kaihola]

#9413 is a symptom of this and has been marked as duplicate.

[matehat]

Just a suggestion: if an app (say called 'chunkies') needs only one templatetag module, it would be better to put its content in the 'templatetags/_ _init_ _.py' module instead of forcing it into a new file, say 'templatetags/chunkiestags.py'. This way, we would be able to load it with {% load chunkies %} instead of {% load chunkiestags %}. I looked at the patch and it would only require a short change in 'django/template/_ _init_ _.py'

    if module.endswith("%s.%s" % (library_name, 'templatetags')):
        taglib_module = module
    else:
        taglib_module = str('%s.%s' % (module, library_name))

[(none)]

Milestone post-1.0 deleted

[Malcolm Tredinnick]

No actual progress on this since 1.0 (yeah, we suck). We'll get to it one day, but not this time around.

[mrts]

Re-targeting for 1.2.

[Andrew Badr]

can take a look for 1.2

[Andrew Badr]

Oyvind, or anyone: how do I use the latest patch? What's up with the /dev/null stuff?

[Ramiro Morales]

Replying to andrewbadr:

What's up with the /dev/null stuff?

Download the patch using the 'Original Format' link at the bottom of the page and examine it with your preferred text editor. The fact you see /dev/null files is because of an interaction between a) Behavior of some tools (like Git, Mercurial, ...?) that generate diffs containing a /dev/null path for the original file in the header when a new file has been added and b) Trac patch HTML renderer that picks and shows that specific header piece.

Patch should apply cleanly using GNU patch though.

[Andrew Badr]

Here's a version of new_template_lib_loader_14.diff updated to work against trunk. This is my first time using git for patches so hopefully I didn't mess anything up.

Two important notes:

• The defaulttags module isn't just moved; it is patched and moved at the same time. There is a one-line difference in the "load" function.
• The templatetag admindocs break. I'll be getting to that later tonight. It was previously pretty tightly coupled to the internals, so I'll assume that's ok for the new version if I can't find a better solution.

It would be great if a committer could confirm that this patch's general approach is correct.

[Andrew Badr]

If you are looking at this patch...

• Should get_templatetags_modules be using import_module or something besides a manual import? I don't know too much about that stuff.
• The division of code feels wrong. I want to at least move get_templatetags_modules from django.templatetags to django.template (the only place it's used) and maybe even rename it to "get_templatelibrary_modules" (matching the terminology used in django.template). At that point, the django/templatetags folder would only contain libraries, so I'd want to move the directory to django/template/templatetags. (The first change has a stronger case.) Thoughts?

[Russell Keith-Magee]

@andrewbadr - Yes, you should be making better use of importlib.import_module. Essentially, any code where you are using imp directly and splitting on '.' - replace it with a call to import_module('full.qualified.module.name')

You need to be careful about moving big blocks of code around, though. The template tests currently fail, for example, because "from django.templatetags.defaultfilters import stringfilter" no longer works. I have plenty of code that imports and uses templates filters as functions. These tests may still have been passing for you if you had a stale .pyc file. django.template may not be the best place for this code, but its a location with historical significance; it can't be changed lightly.

I'll upload an updated patch shortly, which I'll probably commit tomorrow when my head is a little clearer. Other than the import_module and code relocation issues, I'm happy with the approach that has been taken.

[Russell Keith-Magee]

RC1 of template load refactoring

[Andrew Badr]

Before committing, note the admindoc breakage I mentioned above and that Øyvind should get credit for writing the patch (I just translated it to trunk). I can work on the admindoc issue tomorrow.

[Russell Keith-Magee]

(In [12295]) Fixed #6587 -- Removed nasty path hacking in templatetag loading. Thanks to Øyvind Satvik and Andrew Badr for their work on this patch.

[Jacob]

Milestone 1.2 deleted