From 547d0b6cd9ad4e21b817bb80f27950bb9977ed33 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 4 Jul 2008 16:35:10 +0200 Subject: [PATCH] Fixed a bug with the loop context of a for loop if the iterator passed has a volatile `__len__` like the listreverseiterator. `else` in inline if-expressions is optional now. --HG-- branch : trunk --- CHANGES | 7 ++++++- jinja2/compiler.py | 34 ++++++++++++++++++++++++++-------- jinja2/nodes.py | 5 +++++ jinja2/parser.py | 6 ++++-- jinja2/runtime.py | 40 ++++++++++++++++++++-------------------- tests/test_forloop.py | 6 ++++++ tests/test_syntax.py | 10 +++++++++- 7 files changed, 76 insertions(+), 32 deletions(-) diff --git a/CHANGES b/CHANGES index e70bc97..03a125c 100644 --- a/CHANGES +++ b/CHANGES @@ -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) diff --git a/jinja2/compiler.py b/jinja2/compiler.py index 3edc5f8..a90526c 100644 --- a/jinja2/compiler.py +++ b/jinja2/compiler.py @@ -622,6 +622,13 @@ class CodeGenerator(NodeVisitor): 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 @@ class CodeGenerator(NodeVisitor): 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 @@ class CodeGenerator(NodeVisitor): 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,13 +1341,20 @@ class CodeGenerator(NodeVisitor): 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('(') @@ -1344,7 +1362,7 @@ class CodeGenerator(NodeVisitor): 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): diff --git a/jinja2/nodes.py b/jinja2/nodes.py index 56daae4..effa6d4 100644 --- a/jinja2/nodes.py +++ b/jinja2/nodes.py @@ -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() diff --git a/jinja2/parser.py b/jinja2/parser.py index 1e43ed8..d365d4c 100644 --- a/jinja2/parser.py +++ b/jinja2/parser.py @@ -307,8 +307,10 @@ class Parser(object): 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 diff --git a/jinja2/runtime.py b/jinja2/runtime.py index 0417f61..d6c5553 100644 --- a/jinja2/runtime.py +++ b/jinja2/runtime.py @@ -197,14 +197,19 @@ class TemplateReference(object): 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 @@ class LoopContext(object): 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 @@ class LoopContext(object): @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 @@ class LoopContextIterator(object): def next(self): ctx = self.context ctx.index0 += 1 - return ctx._next(), ctx + return ctx._iterator.next(), ctx class Macro(object): diff --git a/tests/test_forloop.py b/tests/test_forloop.py index b8d3ae6..f7e4d68 100644 --- a/tests/test_forloop.py +++ b/tests/test_forloop.py @@ -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) diff --git a/tests/test_syntax.py b/tests/test_syntax.py index 4d28dbc..29b3974 100644 --- a/tests/test_syntax.py +++ b/tests/test_syntax.py @@ -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' -- 2.26.2