Hi all,

I've been working on #15053, which enables Django template loaders to extend
templates of the same name recursively. The most common use-case for this is
extending the admin templates without needing to copy the application 
templates
into a project.

To illustrate the new algorithm, here are some examples:

(-> indicates the next file that is extended)
(fs indicates filesystem template folders)

1. Extend admin template from filesystem directory

fs/admin/base.html # extends admin/base.html
-> django/admin/base.html

2. Extend in multiple filesystem directories

fs/base.html # extends base.html
-> fs2/base.html # extends base.html
-> fs3/base.html

3. Extend using both a filesystem and app loader with multiple directories

fs/base.html # extends base.html
-> fs2/base.html # extends base.html
-> app1/base.html # extends base.html
-> app2/base.html # extends base2.html
-> fs/base2.html # extends base2.html
-> fs2/base2.html

I found it difficult to support this algorithm with the existing template
loaders, and as you can see from example 3, the extends algorithm isn't
necessarily linear with respect to template loaders anymore.

Therefore, the patch I submitted changes three main areas. The first two are
mostly separate from Aymeric's recent changes, but the third is not.

1. Added a new loader api and made it consistent across all loaders.
2. Modified the extends node to track extended templates and passes
    them as a skip argument to the loaders.
3. Updated the debug view template postmortem api to work with the new
    algorithm, as well as added support for the egg and cached loader.

Below is an explanation of what I changed in 1 and 2 and a proposal for
3 that hopefully works with multiple template engines.

## 1. Add new template loader apis

I tried to solve this patch without changing the template loader apis, but I
eventually decided this was inevitable for two reasons:

1. The loaders don't implement a consistent api. For instance, the
filesystem and app loaders define a get_template_source method that is
used elsewhere for debug information, whereas the egg and cached loaders do
not. The cached loader implements load_template but not 
load_template_source.

2. The loaders need to be able to skip templates that have already been
extended, but adding a new argument to load_template is difficult to do in a
backwards-compatible way.

This led me to the following loader api:

### Loader API

Loader.get_template_sources

A method that yields all paths the loader will look at for a given template
name. For example, if the filesystem loader receives "index.html" as an
argument, this yields the full path of "index.html" for each directory on 
the
search path. The cached loader yields tuples of (loader_class, 
template_paths)
for each configured loader.
This method already exists for the filesystem and app loaders. This patch
implements it for all loaders.

Loader.get_internal

Returns the contents for a template given a ``source`` as returned by
``get_template_sources``.

This is where a filesystem loader would read contents from the filesystem,
or a database loader would read from the database. If a matching template
doesn't exist this should raise a ``TemplateDoesNotExist`` error.

BaseLoader.get_template

Returns a ``Template`` object for a given ``template_name`` by looping
through results from ``get_template_sources`` and calling ``get_internal``.
This returns the first matching template. If no template is found,
``TemplateSyntaxError`` is raised.

This method takes an optional ``skip`` argument. ``skip`` is a set 
containing template sources. This is used when extending templates to
allow enable extending other templates of the same name. It's also used
to avoid recursion errors.

In general, it will be enough to define ``get_template_sources`` and
``get_internal`` for custom template loaders. ``get_template`` will
usually not need to be overridden.

This loader api enables the following changes:

## 2. Update the extends tag

The new extends tag keeps track of which sources were tried in the local
context. These sources are passed to the loader ``get_template`` method as
the ``skip`` argument. In doing so, the extends tag never extends the same 
source file twice. This enables recursive extending, and also avoids 
filesystem recursion errors when extending is truly circular.

The main caveat here is that I changed Template.origin to always be 
available
on the template--not just when TEMPLATE_DEBUG is true. Without this, the
extends tag has no way to know the source path of the current template.

I think this change is okay, but I don't know the original reasons for
excluding this in the first place. It could be simply because there was no
use-case presented outside of debug.

## 3. Debug api

Django displays a template postmortem message when a matching template isn't
found. This is a linear list grouped by template loader. This api is 
currently
supported only by the filesystem and app directories loaders.

This api works by looping through template loaders that define a
``get_template_sources`` method, calling that method, and using os.path
functions on each source to see if it exists as a file or is readable. This
information is passed in as ``loader_debug_info`` to the debug template.

There are some limitations to this approach:

* It only works with Django loaders that define a ``get_template_sources`` 
  method -- the filesystem and app loaders
* It only works if ``get_template_sources`` are filesystem paths
* It assumes each loader will only be called at most once and in order. This
  isn't always the case if template recursion is enabled.
* The postmortem doesn't account for multiple template engines.

This patch proposes a new debug api that works with all loaders. It 
simplifies
the logic in the debug views quite a bit, and I think it can work for 
multiple
template engines as well, as explained below.

### The approach

Rather than trying to rerun template engines or loaders, this patch passes 
in
debug information as part of ``TemplateDoesNotExist``. This is done by 
updating
the multiple places where Django catches ``TemplateDoesNotExist`` exceptions
and silently throws them away. Instead of simply silencing them, this patch
accumulates which templates failed and includes them in subsequent
``TemplateDoesNotExist`` exceptions that are raised.

This information is kept as a list of tuples saved to a ``tried`` attribute 
of
the exception.

Here's a somewhat complex example of this attribute when using multiple
loaders. It extends through multiple loaders, but fails when no more 
matching
templates exist:

[
    # Note: (loader, path, status)
    (filesystem.Loader, "/path/to/fs/base.html", "Found")
    (filesystem.Loader, "/path/to/fs/base.html", "Skipped")
    (app_directories.Loader, "/path/to/app/base.html", "Found")
    (filesystem.Loader, "/path/to/fs/base.html", "Skipped")
    (filesystem.Loader, "/path/to/app/base.html", "Skipped")
    (app_directories.Loader, "/path/to/app2/base.html", "Source does not 
exist")
]

Here's a before and after of what the postmortem looks like:

Before:

<https://lh5.googleusercontent.com/-6QNQEJ2KRgk/VJ-PD146t3I/AAAAAAAAAL4/7I9_bfG0_CA/s1600/ee0e758e-7e63-11e4-9a72-7571d591de87.png>
After:

<https://lh5.googleusercontent.com/-dV-rsPD1Mng/VJ-PWou7LiI/AAAAAAAAAMA/1jMpl_i8u1M/s1600/17f55944-7e64-11e4-8ff5-8ab12d135f48.png>
After (complex example):

<https://lh6.googleusercontent.com/-01W4-h6L8jY/VJ-Pj8bPeEI/AAAAAAAAAMI/cT9qEU8jWj8/s1600/3010746e-7e64-11e4-919b-d222b66a28d7.png>

### Multiple engine support

I'll publish an updated pull request once Aymeric's multiple template 
engines
branch lands. At that time, I think the ``tried`` attribute will change to
this:

[
    # (loader, path, status)
    (engine, filesystem.Loader, "/path/to/fs/base.html", "Found")
    (engine, filesystem.Loader, "/path/to/fs/base.html", "Skipped")
    (engine, app_directories.Loader, "/path/to/app/base.html", "Found")
    (engine, filesystem.Loader, "/path/to/fs/base.html", "Skipped")
    (engine, filesystem.Loader, "/path/to/app/base.html", "Skipped")
    (engine, app_directories.Loader, "/path/to/app2/base.html", "Source 
does not exist")
]

With this api, other engines can display a postmortem by simply passing a
``tried`` value into ``TemplateDoesNotExist``. I'll qualify simply, though,
because other template engines don't necessarily provide an easy way to get 
the
list of templates that were tried in their internal exceptions. This seems 
to
be the case with Jinja2.

Jinja2 loaders have a list_templates method, but it's not that helpful here,
escecially since the results are sorted alphabetically. With that said, I 
don't
think it would be too bad to support the postmortem for the main loaders:
Filesystem, PackageLoader, ChoiceLoader, etc. It does require reimplementing
some code, though, to loop through the loader search paths.

With multiple engine support, I see the template postmortem info changing
to the format below. This information is optional in case engine is unable 
to
provide it. Here's a simple example with some engines, 1, 2, and 3, a 
template
that doesn't exist:

Django tried loading templates with <engine1>:
  * path/to/template1 (Source does not exist)
  * otherpath/to/template1 (Source does not exist)

Django tried loading templates with <engine2>:
  * path/to/template1 (Source does not exist)
  * otherpath/to/template1 (Source does not exist)

Django tried loading templates with <engine3>:
  * template1 (Source does not exist)
  (this engine doesn't provide the list of templates that were tried)

## Conclusion

Overall, I'm pretty happy with this new api. I have a few concerns though,
which are as follows:

1. This patch deprecates the old loader apis: Loader.load_template,
    Loader.load_template_sources, etc. I think this is good, but there's 
already
    a lot of deprecations happening with the multiple engine branch.

2. The origin api needs a design decision. I made that available regardless 
of
    the debug setting. Also, it sounds like Aymeric may be doing some work 
to
    refactor this api.

3. The debug api is also something Aymeric is scheduled to work on. Aymeric,
    if you've got something better in mind I've no problem waiting for that 
to be
    in place.

Feedback is welcome.

Thanks.

Preston

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9389e64f-a0cb-475b-9b42-3964e03046be%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to