Permalink
Browse files

Fixed a bug with the loop context of a for loop if the iterator passe…

…d has a volatile `__len__` like the listreverseiterator. `else` in inline if-expressions is optional now.

--HG--
branch : trunk
  • Loading branch information...
1 parent c670b11 commit 547d0b6cd9ad4e21b817bb80f27950bb9977ed33 @mitsuhiko mitsuhiko committed Jul 4, 2008
Showing with 76 additions and 32 deletions.
  1. +6 −1 CHANGES
  2. +26 −8 jinja2/compiler.py
  3. +5 −0 jinja2/nodes.py
  4. +4 −2 jinja2/parser.py
  5. +20 −20 jinja2/runtime.py
  6. +6 −0 tests/test_forloop.py
  7. +9 −1 tests/test_syntax.py
View
@@ -19,9 +19,14 @@ Version 2.0
- added :meth:`jinja2.environment.TemplateStream.dump`.
-- added support for implicit string literal concatenation.
+- added missing support for implicit string literal concatenation.
``{{ "foo" "bar" }}`` is equivalent to ``{{ "foobar" }}``
+- `else` is optional for conditional expressions. If not given it evaluates
+ to `false`.
+
+- improved error reporting for undefined values by providing a position.
+
Version 2.0rc1
--------------
(no codename, released on June 9th 2008)
View
@@ -622,6 +622,13 @@ def macro_def(self, node, frame):
bool(frame.accesses_caller)
))
+ def position(self, node):
+ """Return a human readable position for the node."""
+ rv = 'line %d' % node.lineno
+ if self.name is not None:
+ rv += ' in' + repr(self.name)
+ return rv
+
# -- Statement Visitors
def visit_Template(self, node, frame=None):
@@ -835,8 +842,11 @@ def visit_FromImport(self, node, frame):
self.writeline('l_%s = environment.undefined(%r %% '
'included_template.__name__, '
'name=%r)' %
- (alias, 'the template %r does not export '
- 'the requested name ' + repr(name), name))
+ (alias, 'the template %%r (imported on %s) does '
+ 'not export the requested name %s' % (
+ self.position(node),
+ repr(name)
+ ), name))
self.outdent()
if frame.toplevel:
var_names.append(alias)
@@ -908,10 +918,11 @@ def visit_For(self, node, frame):
if 'loop' not in aliases and 'loop' in find_undeclared(
node.iter_child_nodes(only=('else_', 'test')), ('loop',)):
self.writeline("l_loop = environment.undefined(%r, name='loop')" %
- "'loop' is undefined. the filter section of a loop as well " \
- "as the else block doesn't have access to the special 'loop' "
- "variable of the current loop. Because there is no parent "
- "loop it's undefined.")
+ ("'loop' is undefined. the filter section of a loop as well "
+ "as the else block doesn't have access to the special 'loop'"
+ " variable of the current loop. Because there is no parent "
+ "loop it's undefined. Happened in loop on %s" %
+ self.position(node)))
self.writeline('for ', node)
self.visit(node.target, loop_frame)
@@ -1330,21 +1341,28 @@ def visit_Test(self, node, frame):
self.write(')')
def visit_CondExpr(self, node, frame):
+ def write_expr2():
+ if node.expr2 is not None:
+ return self.visit(node.expr2, frame)
+ self.write('environment.undefined(%r)' % ('the inline if-'
+ 'expression on %s evaluated to false and '
+ 'no else section was defined.' % self.position(node)))
+
if not have_condexpr:
self.write('((')
self.visit(node.test, frame)
self.write(') and (')
self.visit(node.expr1, frame)
self.write(',) or (')
- self.visit(node.expr2, frame)
+ write_expr2()
self.write(',))[0]')
else:
self.write('(')
self.visit(node.expr1, frame)
self.write(' if ')
self.visit(node.test, frame)
self.write(' else ')
- self.visit(node.expr2, frame)
+ write_expr2()
self.write(')')
def visit_Call(self, node, frame, forward_caller=False):
View
@@ -499,6 +499,11 @@ class CondExpr(Expr):
def as_const(self):
if self.test.as_const():
return self.expr1.as_const()
+
+ # if we evaluate to an undefined object, we better do that at runtime
+ if self.expr2 is None:
+ raise Impossible()
+
return self.expr2.as_const()
View
@@ -307,8 +307,10 @@ def parse_condexpr(self):
expr1 = self.parse_or()
while self.stream.skip_if('name:if'):
expr2 = self.parse_or()
- self.stream.expect('name:else')
- expr3 = self.parse_condexpr()
+ if self.stream.skip_if('name:else'):
+ expr3 = self.parse_condexpr()
+ else:
+ expr3 = None
expr1 = nodes.CondExpr(expr2, expr1, expr3, lineno=lineno)
lineno = self.stream.current.lineno
return expr1
View
@@ -197,14 +197,19 @@ def __repr__(self):
class LoopContext(object):
"""A loop context for dynamic iteration."""
- def __init__(self, iterable, enforce_length=False, recurse=None):
- self._iterable = iterable
- self._next = iter(iterable).next
- self._length = None
+ def __init__(self, iterable, recurse=None):
+ self._iterator = iter(iterable)
self._recurse = recurse
self.index0 = -1
- if enforce_length:
- len(self)
+
+ # try to get the length of the iterable early. This must be done
+ # here because there are some broken iterators around where there
+ # __len__ is the number of iterations left (i'm looking at your
+ # listreverseiterator!).
+ try:
+ self._length = len(iterable)
+ except (TypeError, AttributeError):
+ self._length = None
def cycle(self, *args):
"""Cycles among the arguments with the current loop index."""
@@ -213,7 +218,7 @@ def cycle(self, *args):
return args[self.index0 % len(args)]
first = property(lambda x: x.index0 == 0)
- last = property(lambda x: x.revindex0 == 0)
+ last = property(lambda x: x.index0 + 1 == x.length)
index = property(lambda x: x.index0 + 1)
revindex = property(lambda x: x.length - x.index0)
revindex0 = property(lambda x: x.length - x.index)
@@ -237,18 +242,13 @@ def loop(self, iterable):
@property
def length(self):
if self._length is None:
- try:
- # first try to get the length from the iterable (if the
- # iterable is a sequence)
- length = len(self._iterable)
- except TypeError:
- # if that's not possible (ie: iterating over a generator)
- # we have to convert the iterable into a sequence and
- # use the length of that.
- self._iterable = tuple(self._iterable)
- self._next = iter(self._iterable).next
- length = len(tuple(self._iterable)) + self.index0 + 1
- self._length = length
+ # if was not possible to get the length of the iterator when
+ # the loop context was created (ie: iterating over a generator)
+ # we have to convert the iterable into a sequence and use the
+ # length of that.
+ iterable = tuple(self._iterator)
+ self._iterator = iter(iterable)
+ self._length = len(iterable) + self.index0 + 1
return self._length
def __repr__(self):
@@ -272,7 +272,7 @@ def __iter__(self):
def next(self):
ctx = self.context
ctx.index0 += 1
- return ctx._next(), ctx
+ return ctx._iterator.next(), ctx
class Macro(object):
@@ -116,6 +116,12 @@ def test_looploop(env):
assert tmpl.render(table=['ab', 'cd']) == '[1|1][1|2][2|1][2|2]'
+def test_reversed_bug(env):
+ tmpl = env.from_string('{% for i in items %}{{ i }}{% if not loop.last %}'
+ ',{% endif %}{% endfor %}')
+ assert tmpl.render(items=reversed([3, 2, 1])) == '1,2,3'
+
+
def test_loop_errors(env):
tmpl = env.from_string(LOOPERROR1)
raises(UndefinedError, tmpl.render)
View
@@ -8,7 +8,7 @@
"""
from py.test import raises
from jinja2 import Environment, DictLoader
-from jinja2.exceptions import TemplateSyntaxError
+from jinja2.exceptions import TemplateSyntaxError, UndefinedError
CALL = '''{{ foo('a', c='d', e='f', *['b'], **{'g': 'h'}) }}'''
@@ -124,6 +124,14 @@ def test_conditional_expression(env):
assert tmpl.render() == '0'
+def test_short_conditional_expression(env):
+ tmpl = env.from_string('<{{ 1 if false }}>')
+ assert tmpl.render() == '<>'
+
+ tmpl = env.from_string('<{{ (1 if false).bar }}>')
+ raises(UndefinedError, tmpl.render)
+
+
def test_filter_priority(env):
tmpl = env.from_string(FILTERPRIORITY)
assert tmpl.render() == 'FOOBAR'

0 comments on commit 547d0b6

Please sign in to comment.