id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
7799	django.template refactoring	Johannes Dollinger	nobody	"== 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)[[BR]]
      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).
      {{{
#!python
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)[[BR]]
      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)[[BR]]
      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)[[BR]]
      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)[[BR]]
      Contains `InvalidTemplateLibrary`, `Library`, `get_library()`, and `add_to_builtins()`.
      Mostly untouched.
      
    * `loader`[[BR]]
      Untouched. Moved `TemplateDoesNotExist` here.
    
    * `utils`[[BR]]
      `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`[[BR]]
      Provides backwards compatibility for `Variable`, `VariableNode`, `TokenParser`,
      and `resolve_variable()`
    
    * `defaulttags`, `loader_tags`[[BR]]
      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[[BR]]
      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[[BR]]
      fixed. test included.
    
    * #4278 - get_template should accept a dirs argument[[BR]]
      fixed through `Context(loader=...)`.
    
    * #4746 - allow whitespace before and after filter separator[[BR]]
      fixed. tests are there (filter-syntax03, filter-syntax04) but expect TemplateSyntaxError.
    
    * #5270 - empty string literals[[BR]]
      fixed. tests included.
    
    * #5756 - accept filters (almost) everywhere[[BR]]
      fixed. a few tests included.
    
    * #5862 - dup of #5756[[BR]]
      fixed.
    
    * #5971 - token parser bug[[BR]]
      `TokenParser` will be deprecated.
    
    * #6271 - filter arguments with spaces[[BR]]
      fixed. test included. (seems to be fixed in trunk already)
    
    * #6296 - expressions for ifequal[[BR]]
      fixed. tests included (dup of #5756 ?)
    
    * #6510 - `get_nodes_by_type()` problem
      probably fixed.
    
    * #6535 - negative numeric literals[[BR]]
      fixed. tests included.
      
    * #6834 - support for templates to be loaded from dynamically-selected directories[[BR]]
      fixed through `Context(loader=...)`
    
    * #7295 - quotes, escaping and translation of string literals handled inconsistently in templates[[BR]]
      fixed.
    
    * #7438 keyword support for simple_tag and inclusion_tag[[BR]]
      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[[BR]]
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[[BR]]
This could be useful. But would be easy to abuse.

"		closed	Template system	dev		wontfix	tplrf		Unreviewed	1	1	1	1	0	0