django.template refactoring

[Johannes Dollinger]

Why should django.template be refactored?

Filter and Variable parsing is inconsistent. Many ad hoc parsers in defaulttags are fragile.
Whitespace handling is ungraceful.

The patch provided splits __init__.py into separate modules and introduces a TokenStream
class that allows parsing of literals, vars (called lookups) and filter expressions. No docs yet,
have a look at the source (it all happens in ~150 loc).

Additionally some features crept in, listed below.
The patch only touches django.template and django.templatetags.
Tests are separate so you can run them with the old code.

Modules in django.template

• context (no dependencies)
  Mostly untouched. There's a Context level template cache included, accessible via Context.get_template() and Context.select_template(). This is used in IncludeNode, ExtendsNode, and inclusion_tag. Slightly backwards incompatible if you rely on parse time sideeffects (ie {% load %}) in included templates.

A loader kwarg is added to Context to force use of a specific templateloader (something with a get_template() method).

class PrefixLoader:
    def __init__(self, prefix_list):
        self.prefix_list = prefix_list
    def get_template(name):
        from django.template.loader import select_template
        return select_template(['%s/%s' % (prefix, name) for prefix in self.prefix_list])

tpl.render(Context({}, loader=PrefixLoader(['a', 'b', 'c'])))

• expressions (no dependencies)
  Contains Lookup, Literal, and FilterExpression. All are subclasses of a marker class Expression. These replace Variable and the old FilterExpression class. A Variable equivalent is provided in compat.Variable.

• nodes (depends on expressions)
  Contains Node, NodeList, TextNode, and ExpressionNode. The latter is the old VariableNode renamed. A VariableNode alias is provided in compat.

• compiler (depends on context, expressions, nodes)
  Contains Parser, Lexer, Token, TemplateSyntaxError, Origin, StringOrgin, and compile_string(). Those are mostly untouched. Additionally there are TokenStream, and TokenSyntaxError. Those provide a safe way to parse Tokens containing expressions.

• library (depends on compiler, nodes, context)
  Contains InvalidTemplateLibrary, Library, get_library(), and add_to_builtins(). Mostly untouched.

• loader
  Untouched. Moved TemplateDoesNotExist here.

• utils
ConditionalNode, base class for IfNode, IfEqualNode. EmptyNode. Helper functions: parse_conditional_nodelists(parser, name), parse_as(bits) parse_context_map(bits), render_with_context_map(renderable, context_map, context). parse_args_and_kwargs(bits), resolve_args_and_kwargs(args, kwargs)

• compat
  Provides backwards compatibility for Variable, VariableNode, TokenParser, and resolve_variable()

• defaulttags, loader_tags
  Refactored to use TokenStream where appropriate. Extended with-tag syntax to allow

  {% with foo as x, bar as y, ... %}
  

  Extended include-tag syntax to allow

  {% include "template" with foo as x, bar as y %}
  

Tickets

• #1105 - takes_context and takes_nodelist for simple_tag
  fixed, based on julien's patch. tests not included to keep the tplrf-tests.diff functional for current trunk.

• #2949 - templating engine too tightly tied to TEMPLATE_DIRS in the settings fixed through Context(loader=...).

• #3544 - recursive includes
  fixed. test included.

• #4278 - get_template should accept a dirs argument
  fixed through Context(loader=...).

• #4746 - allow whitespace before and after filter separator
  fixed. tests are there (filter-syntax03, filter-syntax04) but expect TemplateSyntaxError.

• #5270 - empty string literals
  fixed. tests included.

• #5756 - accept filters (almost) everywhere
  fixed. a few tests included.

• #5862 - dup of #5756
  fixed.

• #5971 - token parser bug
  TokenParser will be deprecated.

• #6271 - filter arguments with spaces
  fixed. test included. (seems to be fixed in trunk already)

• #6296 - expressions for ifequal
  fixed. tests included (dup of #5756 ?)

• #6510 - get_nodes_by_type() problem probably fixed.

• #6535 - negative numeric literals
  fixed. tests included.

• #6834 - support for templates to be loaded from dynamically-selected directories
  fixed through Context(loader=...)

• #7295 - quotes, escaping and translation of string literals handled inconsistently in templates
  fixed.

• #7438 keyword support for simple_tag and inclusion_tag
  not fixed yet. but utils.parse_args_and_kwargs() would make this easy. it's there for {% url %} anyway.

Performance

Comparing runtests.py templates measures, it appears the refactored version is faster.
But I'm not sure why and don't really trust these numbers. Are failing tests expensive?

Todo

{% if not %} is currently valid. The refactored implementation is greedy and reports a syntax error.
Could be fixed, not sure it's worth it. Broken tests are: if-tag-not02, if-tag-not03. DDN.

Numeric literals ending with "." are now considered a syntax error.
Broken test: ifequal-numeric07. DDN.

{% url %} currently accepts an arbitrary unquoted unicode string for the view name.
The refactored implementation only accepts bare python identifiers or quoted string literals.
Broken test: url05. DDN.

TemplateSyntaxError messages need some work. Eventually shortcut methods on TokenStream.

Find better names for TokenSyntaxError and TokenStream.

Provide a decorator that handles the boilerplate bits=parser.token_stream(token)
and bits.assert_consumed() calls.

Docs and more tests.

#1199 - multiple filter arguments
I'd like to implement this. But the proposed syntax foo|bar:x,y,z will not work
for {% url %} and the new {% with %}.
Proposals:

• New syntax: foo|bar(x,y,z), keeping foo|bar:x for bc.
• Tupel primitives: foo|bar:(x,y,z), would allow {% for x in (a,b,c) %}
• Lisp style: foo|bar:(x|cons:y), this would basically allows bracketed expressions as arguments to filters. Arguably the ugliest.

#7261 - __html__() support
This could be useful. But would be easy to abuse.

[Johannes Dollinger]

Note: #1199 would allow to deprecate _() magic and replace it with regular filters: {% foo|bar:(myvar|gettext) %}. Of course you could also introduce the concept of template functions, but if I wanted jinja, I'd use jinja.

[Malcolm Tredinnick]

This looks like a very large ticket and it's probably a bit risky to have just created it, rather than as a result of a django-dev discussion (since people might be tempted to put design thinking into the ticket comments). That being said, you should always feel free to work on whatever you like.

However (and this is very important), if you're refactoring the template stuff, then that is all you should be doing in this patch. No new features! Combining large changes that are only meant to reorganise the code (and not cause regressions) with things that change features isn't fair to anybody and will lead to the ticket being closed as wontfix because you aren't doing things in a way that allows us to work properly. A number of the changes you are proposing are reasonably controversial and bear in mind that Tom Tobin (korpios) is also working in this are on some refactoring (which he announced on django-dev), so you should probably check in with him, too.

So, yes, there's probably some stuff to do here, but keep things manageable. Make it just a refactoring. Those other tickets exist to solve their individual problems already and one issue per ticket is the preferred way to work unless it's absolutely unavoidable (we never mind putting certain fixes on hold until a reasonably certain refactor lands, so that isn't an issue if the refactoring is sensible). Separate out things that are actually problematic from things where you would just prefer a different name -- which usually are just rearranging the furniture rather than improvements, since everybody likes different names, although there are exceptions of course -- and from the things that add new functionality.

[Julien Phalip]

How about a template-refactor branch? Considering the large number of tickets involved, having a separate branch of Django may be easier to test and work with...

[Malcolm Tredinnick]

No justification for it at the moment. All the refactoring I've seen suggested here and in django-dev threads are small self-contained changes. And all the tickets are self-contained, too. Branches are complex to manage.

[Johannes Dollinger]

django-dev seems like a bad place to discuss anything unrelated to 1.0.
That's why I did not even try this time. I actually wanted people to put
design thinking into the ticket comments, that could be summarized and
posted to django-dev in a month .. or five.

I agree new features should be separate tickets. I was just being lazy. So I'll
close this now and open a new ticket with a stripped down patch, since I cannot
change the ticket description (can I?).

And avoid new branches at all costs, please.

[Adrian Holovaty]

At first glance, I like this a lot. Yes, please open a new ticket with a slimmed-down patch.

[Johannes Dollinger]

New ticket here: #7806.