Added a small improvement for the code generation of newstyle gettext

calls.

--HG--
branch : trunk

jinja2/ext.py

@@ -139,9 +139,9 @@ def gettext(__context, __string, **variables):
 
 def _make_new_ngettext(func):
     @contextfunction
-    def ngettext(__context, __singular, __plural, num, **variables):
-        variables.setdefault('num', num)
-        rv = __context.call(func, __singular, __plural, num)
+    def ngettext(__context, __singular, __plural, __num, **variables):
+        variables.setdefault('num', __num)
+        rv = __context.call(func, __singular, __plural, __num)
         if __context.eval_ctx.autoescape:
             rv = Markup(rv)
         return rv % variables
@@ -210,6 +210,7 @@ def _extract(self, source, gettext_functions=GETTEXT_FUNCTIONS):
     def parse(self, parser):
         """Parse a translatable tag."""
         lineno = next(parser.stream).lineno
+        num_called_num = False
 
         # find all the variables referenced.  Additionally a variable can be
         # defined in the body of the trans block too, but this is checked at
@@ -236,8 +237,10 @@ def parse(self, parser):
                 variables[name.value] = var = parser.parse_expression()
             else:
                 variables[name.value] = var = nodes.Name(name.value, 'load')
+
             if plural_expr is None:
                 plural_expr = var
+                num_called_num = name.value == 'num'
 
         parser.stream.expect('block_end')
 
@@ -251,6 +254,7 @@ def parse(self, parser):
             referenced.update(singular_names)
             if plural_expr is None:
                 plural_expr = nodes.Name(singular_names[0], 'load')
+                num_called_num = singular_names[0] == 'num'
 
         # if we have a pluralize block, we parse that too
         if parser.stream.current.test('name:pluralize'):
@@ -263,6 +267,7 @@ def parse(self, parser):
                                 name.value, name.lineno,
                                 exc=TemplateAssertionError)
                 plural_expr = variables[name.value]
+                num_called_num = name.value == 'num'
             parser.stream.expect('block_end')
             plural_names, plural = self._parse_block(parser, False)
             next(parser.stream)
@@ -281,7 +286,7 @@ def parse(self, parser):
             parser.fail('pluralize without variables', lineno)
 
         node = self._make_node(singular, plural, variables, plural_expr,
-                               bool(referenced))
+                               bool(referenced), num_called_num)
         node.set_lineno(lineno)
         return node
 
@@ -318,7 +323,7 @@ def _parse_block(self, parser, allow_pluralize):
         return referenced, concat(buf)
 
     def _make_node(self, singular, plural, variables, plural_expr,
-                   vars_referenced):
+                   vars_referenced, num_called_num):
         """Generates a useful node from the data provided."""
         # no variables referenced?  no need to escape for old style
         # gettext invocations
@@ -347,6 +352,10 @@ def _make_node(self, singular, plural, variables, plural_expr,
         # handling itself
         if self.environment.newstyle_gettext:
             for key, value in variables.iteritems():
+                # the function adds that later anyways in case num was
+                # called num, so just skip it.
+                if num_called_num and key == 'num':
+                    continue
                 node.kwargs.append(nodes.Keyword(key, value))
 
         # otherwise do that here

jinja2/testsuite/ext.py

@@ -38,7 +38,7 @@
                   '{% trans %}watch out{% endtrans %}{% endblock %}',
     'plural.html': '{% trans user_count %}One user online{% pluralize %}'
                    '{{ user_count }} users online{% endtrans %}',
-    'stringformat.html': '{{ _("User: %(num)d")|format(num=user_count) }}'
+    'stringformat.html': '{{ _("User: %(num)s")|format(num=user_count) }}'
 }
 
 newstyle_i18n_templates = {
@@ -48,8 +48,10 @@
                   '{% trans %}watch out{% endtrans %}{% endblock %}',
     'plural.html': '{% trans user_count %}One user online{% pluralize %}'
                    '{{ user_count }} users online{% endtrans %}',
-    'stringformat.html': '{{ _("User: %(num)d", num=user_count) }}',
-    'ngettext.html': '{{ ngettext("%(num)d apple", "%(num)d apples", apples) }}'
+    'stringformat.html': '{{ _("User: %(num)s", num=user_count) }}',
+    'ngettext.html': '{{ ngettext("%(num)s apple", "%(num)s apples", apples) }}',
+    'ngettext_long.html': '{% trans num=apples %}{{ num }} apple{% pluralize %}'
+                          '{{ num }} apples{% endtrans %}'
 }
 
 
@@ -59,9 +61,9 @@
         'watch out':                    u'pass auf',
         'One user online':              u'Ein Benutzer online',
         '%(user_count)s users online':  u'%(user_count)s Benutzer online',
-        'User: %(num)d':                u'Benutzer: %(num)d',
-        '%(num)d apple':                u'%(num)d Apfel',
-        '%(num)d apples':               u'%(num)d Äpfel'
+        'User: %(num)s':                u'Benutzer: %(num)s',
+        '%(num)s apple':                u'%(num)s Apfel',
+        '%(num)s apples':               u'%(num)s Äpfel'
     }
 }
 
@@ -327,6 +329,22 @@ def test_autoescape_support(self):
         assert t.render(ae=True) == '<strong>Wert: &lt;test&gt;</strong>'
         assert t.render(ae=False) == '<strong>Wert: <test></strong>'
 
+    def test_num_used_twice(self):
+        tmpl = newstyle_i18n_env.get_template('ngettext_long.html')
+        assert tmpl.render(apples=5, LANGUAGE='de') == u'5 Äpfel'
+
+    def test_num_called_num(self):
+        source = newstyle_i18n_env.compile('''
+            {% trans num=3 %}{{ num }} apple{% pluralize
+            %}{{ num }} apples{% endtrans %}
+        ''', raw=True)
+        # quite hacky, but the only way to properly test that.  The idea is
+        # that the generated code does not pass num twice (although that
+        # would work) for better performance.  This only works on the
+        # newstyle gettext of course
+        assert re.search(r"l_ngettext, u?'\%\(num\)s apple', u?'\%\(num\)s "
+                         r"apples', 3", source) is not None
+
 
 class AutoEscapeTestCase(JinjaTestCase):