From 69e12dbb951fad8bfb7e92b8b3060aff288b5b98 Mon Sep 17 00:00:00 2001
From: Armin Ronacher <armin.ronacher@active-4.com>
Date: Mon, 12 May 2008 09:00:03 +0200
Subject: [PATCH] improved variable exporting, added TODO file

--HG--
branch : trunk
---
 TODO               | 57 ++++++++++++++++++++++++++++++++++++++++++++++
 docs/jinjaext.py   |  9 +++++---
 docs/tricks.rst    |  4 ++--
 jinja2/compiler.py | 22 +++++++++++++++---
 jinja2/ext.py      | 10 ++++----
 jinja2/nodes.py    | 34 +++++++++++++++++++++++++++
 6 files changed, 123 insertions(+), 13 deletions(-)
 create mode 100644 TODO

diff --git a/TODO b/TODO
new file mode 100644
index 0000000..6881b76
--- /dev/null
+++ b/TODO
@@ -0,0 +1,57 @@
+Todo Before Release
+===================
+
+This has to be implemented before the release:
+
+Drop special casing template globals
+------------------------------------
+
+The idea some time ago was to partially evaluate templates at compile time.
+For that we decided to not allow globals being overridden by locals as the
+static compiler could have changed semantics.  Turns out that the static
+compiler blows up the complexity to something nobody wants to support which
+means that this restriction is kinda pointless.
+
+That should go for good.
+
+Pull Attributes Onces
+---------------------
+
+Imagine the following template::
+
+    {% if foo.bar %}
+        {{ baz(foo.bar) }}
+    {% endif %}
+
+Problem with that is that it compiles to this::
+
+    if environment.subscribe(l_foo, 'bar'):
+        if 0: yield None
+        yield u'\n    %s\n' % (
+            l_baz(environment.subscribe(l_foo, 'bar')),
+        )
+
+As `environment.subscribe` is more expensive then regular attribute lookups
+(it tries getitem/getattr and in sandbox mode also permissions) multiple
+lookups with the same parameters in the same scope should get local aliases.
+The result we have is that one::
+
+    t1 = environment.subscribe(l_foo, 'bar')
+    if t1:
+        if 0: yield None
+        yield u'\n    %s\n' % (
+            l_baz(t1),
+        )
+
+However that should only happen if the attribute is accessed multiple times
+unlike locals and filters/tests which are always pulled.  We're not doing that
+for filters/tests/locals as nested scopes may access it and testing is too
+complicated for the tiny performance improvement but easy for attribute
+lookups, keeping the complexity of the whole thing in mind.
+
+Use `set` for Assignments
+-------------------------
+
+The keyword-less way to assign variabeles is a left-over from the days when
+it was possible to use Jinja2 like a regular programming language without
+necessary template end/start delimiters to group statements.
diff --git a/docs/jinjaext.py b/docs/jinjaext.py
index 78a45ec..1ed6d35 100644
--- a/docs/jinjaext.py
+++ b/docs/jinjaext.py
@@ -168,14 +168,17 @@ def jinja_nodes(dirname, arguments, options, content, lineno,
         if node.abstract:
             members = []
             for key, name in node.__dict__.iteritems():
-                if not key.startswith('_') and callable(name):
+                if not key.startswith('_') and \
+                   not hasattr(node.__base__, key) and callable(name):
                     members.append(key)
             if members:
                 members.sort()
                 doc.append('%s :members: %s' % (p, ', '.join(members)), '')
-        else:
+        if node.__base__ != object:
             doc.append('', '')
-            doc.append(p + ' :Node type: :class:`%s`' % node.__base__.__name__, '')
+            doc.append('%s :Node type: :class:`%s`' %
+                       (p, node.__base__.__name__), '')
+            # XXX: sphinx bug?  Expr gives a rst warning
         doc.append('', '')
         children = node.__subclasses__()
         children.sort(key=lambda x: x.__name__.lower())
diff --git a/docs/tricks.rst b/docs/tricks.rst
index 6029ec5..1cfdaa8 100644
--- a/docs/tricks.rst
+++ b/docs/tricks.rst
@@ -65,12 +65,12 @@ child template::
 The layout template can then access `active_page`.  Additionally it makes
 sense to defined a default for that variable::
 
-    {% navigation_bar = [
+    {% set navigation_bar = [
         ('/', 'index', 'Index'),
         ('/downloads/', 'downloads', 'Downloads'),
         ('/about/', 'about', 'About')
     ] -%}
-    {% active_page = active_page|default('index') -%}
+    {% set active_page = active_page|default('index') -%}
     ...
     <ul id="navigation">
     {% for href, id, caption in navigation_bar %}
diff --git a/jinja2/compiler.py b/jinja2/compiler.py
index cf0f4a5..cb0da5a 100644
--- a/jinja2/compiler.py
+++ b/jinja2/compiler.py
@@ -43,6 +43,7 @@ def generate(node, environment, name, filename, stream=None):
     """Generate the python source for a node tree."""
     if not isinstance(node, nodes.Template):
         raise TypeError('Can\'t compile non template nodes')
+    node.freeze()
     generator = CodeGenerator(environment, name, filename, stream)
     generator.visit(node)
     if stream is None:
@@ -1138,10 +1139,25 @@ class CodeGenerator(NodeVisitor):
 
         # make sure toplevel assignments are added to the context.
         if frame.toplevel:
-            for name in assignment_frame.assigned_names:
+            public_names = [x for x in assignment_frame.assigned_names
+                            if not x.startswith('__')]
+            if len(assignment_frame.assigned_names) == 1:
+                name = iter(assignment_frame.assigned_names).next()
                 self.writeline('context.vars[%r] = l_%s' % (name, name))
-                if not name.startswith('__'):
-                    self.writeline('context.exported_vars.add(%r)' % name)
+            else:
+                self.writeline('context.vars.update({')
+                for idx, name in enumerate(assignment_frame.assigned_names):
+                    if idx:
+                        self.write(', ')
+                    self.write('%r: l_%s' % (name, name))
+                self.write('})')
+            if public_names:
+                if len(public_names) == 1:
+                    self.writeline('context.exported_vars.add(%r)' %
+                                   public_names[0])
+                else:
+                    self.writeline('context.exported_vars.update((%s))' %
+                                   ', '.join(map(repr, public_names)))
 
     def visit_Name(self, node, frame):
         if node.ctx == 'store' and frame.toplevel:
diff --git a/jinja2/ext.py b/jinja2/ext.py
index aac7528..117c2f3 100644
--- a/jinja2/ext.py
+++ b/jinja2/ext.py
@@ -77,17 +77,17 @@ class Extension(object):
 
     def attr(self, name, lineno=None):
         """Return an attribute node for the current extension.  This is useful
-        to pass callbacks to template code::
+        to pass constants on extensions to generated template code::
 
-            nodes.Call(self.attr('_my_callback'), args, kwargs, None, None)
-
-        That would call `self._my_callback` when the template is evaluated.
+            self.attr('_my_attribute', lineno=lineno)
         """
         return nodes.ExtensionAttribute(self.identifier, name, lineno=lineno)
 
     def call_method(self, name, args=None, kwargs=None, dyn_args=None,
                     dyn_kwargs=None, lineno=None):
-        """Call a method of the extension."""
+        """Call a method of the extension.  This is a shortcut for
+        :meth:`attr` + :class:`jinja2.nodes.Call`.
+        """
         if args is None:
             args = []
         if kwargs is None:
diff --git a/jinja2/nodes.py b/jinja2/nodes.py
index 67f7559..0db061a 100644
--- a/jinja2/nodes.py
+++ b/jinja2/nodes.py
@@ -90,13 +90,20 @@ class Node(object):
     two attributes: `lineno` (the line number of the node) and `environment`.
     The `environment` attribute is set at the end of the parsing process for
     all nodes automatically.
+
+    Nodes can be frozen which makes them hashable.  The compiler freezes the
+    nodes automatically.  Modifications on frozen nodes are possible but not
+    allowed.
     """
     __metaclass__ = NodeType
     fields = ()
     attributes = ('lineno', 'environment')
     abstract = True
+    frozen = False
 
     def __init__(self, *fields, **attributes):
+        if self.abstract:
+            raise TypeError('abstract nodes are not instanciable')
         if fields:
             if len(fields) != len(self.fields):
                 if not self.fields:
@@ -213,6 +220,30 @@ class Node(object):
             todo.extend(node.iter_child_nodes())
         return self
 
+    def freeze(self):
+        """Freeze the complete node tree which makes them hashable.
+        This happens automatically on compilation.  Frozen nodes must not be
+        modified any further.  Extensions may not freeze nodes that appear
+        in the final node tree (ie: nodes that are returned from the extension
+        parse method).
+        """
+        todo = deque([self])
+        while todo:
+            node = todo.popleft()
+            node.frozen = True
+            todo.extend(node.iter_child_nodes())
+
+    def __eq__(self, other):
+        return type(self) is type(other) and self.__dict__ == other.__dict__
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __hash__(self):
+        if not self.frozen:
+            raise TypeError('unfrozen nodes are unhashable')
+        return hash(tuple(self.__dict__.items()))
+
     def __repr__(self):
         return '%s(%s)' % (
             self.__class__.__name__,
@@ -374,6 +405,7 @@ class BinExpr(Expr):
     """Baseclass for all binary expressions."""
     fields = ('left', 'right')
     operator = None
+    abstract = True
 
     def as_const(self):
         f = _binop_to_func[self.operator]
@@ -387,6 +419,7 @@ class UnaryExpr(Expr):
     """Baseclass for all unary expressions."""
     fields = ('node',)
     operator = None
+    abstract = True
 
     def as_const(self):
         f = _uaop_to_func[self.operator]
@@ -412,6 +445,7 @@ class Name(Expr):
 
 class Literal(Expr):
     """Baseclass for literals."""
+    abstract = True
 
 
 class Const(Literal):
-- 
2.26.2