Repository: mozilla/puente

Issue:
pgettext for templates
Templates have `_` (alias for gettext), `gettext` and `ngettext` available. These are handled by Jinja2 contextfunctions which call the underlying installed gettext/ngettext callables, get strings back and then wrap in a `Markup`.

https://github.com/mitsuhiko/jinja2/blob/master/jinja2/ext.py#L135

Yay! That's great!

However, there's no pgettext and no way that I can see to pass in a msgctxt.

This issue covers figuring out what to do about that. At the moment, I'm leaning towards adding it as a global to our `PuenteI18nExtension` for the short-term.

Long-term, we should submit a PR for this issue: https://github.com/mitsuhiko/jinja2/issues/441

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index eab331f..8bafb63 100644
--- a/README.rst
+++ b/README.rst
@@ -12,6 +12,8 @@ using Jinja2 templates.
 * merge command that merges new strings from a ``.pot`` file into locale ``.po``
   files
 * code to collapse whitespace for Jinja2's trans block
+* add pgettext and npgettext to template environment and they correctly
+  escape things and work the same way as Jinja2's newstyle gettext
 * configured using Django settings
 * solid documentation
 * solid tests
diff --git a/docs/goals.rst b/docs/goals.rst
index b26bd8e..b57553a 100644
--- a/docs/goals.rst
+++ b/docs/goals.rst
@@ -33,7 +33,9 @@ Puente does three nice things:
 
 1. makes it easy to migrate from Tower to something you can use with Django 1.8
 2. collapses whitespace in Jinja2 trans blocks
-3. pulls bits from Django settings to configure extraction (e.g. Jinja2
+3. adds pgettext and npgettext to template environment that work like Jinja2's
+   newstyle gettext
+4. pulls bits from Django settings to configure extraction (e.g. Jinja2
    extensions)
 
 If you don't care about any of those things, go use Babel's pybabel command and
@@ -75,9 +77,11 @@ What's different between Tower and Puente?
    indentation of the HTML template. That stinks because translators have to go
    through and fix all the translations.
 
-3. Tower had a bunch of code to support msgctxt in extraction and gettext
-   calls, but Puente relies on Django's pgettext functions and Babel's
-   msgctxt support and that works super.
+3. Tower had a bunch of code to support msgctxt in extraction and gettext calls,
+   but Puente relies on Django's pgettext functions and Babel's msgctxt support
+   and that works super... except in Jinja2 templates. Puente adds pgettext and
+   npgettext to the template environment and they work just like Jinja2's
+   newstyle gettext.
 
 4. Tower had its own gettext and ngettext that marked output as safe, but Puente
    drops that because it's unneeded if you're using Jinja2's newstyle gettext
@@ -120,7 +124,11 @@ We need to do the following before we can end Puente:
 
    https://github.com/mitsuhiko/jinja2/issues/504
 
-2. Puente's extract command should work more like Babel's pybabel extract
+2. IN PROGRESS: Jinja2 needs to support pgettext/npgettext in templates.
+
+   https://github.com/mitsuhiko/jinja2/issues/441
+
+3. Puente's extract command should work more like Babel's pybabel extract
    command.
 
    The way forward is to phase Puente out for pybabel. In order to make that
@@ -128,11 +136,10 @@ We need to do the following before we can end Puente:
 
    This should probably be broken up into more steps as we discover differences.
 
-3. Ditch Puente's merge for pybabel's update?
+4. Ditch Puente's merge for pybabel's update?
 
-4. Need a nice way to use Django settings for pybabel configuration. For
+5. Need a nice way to use Django settings for pybabel configuration. For
    example, I'd rather not have to define the list of Jinja2 extensions to use
    in two places.
 
-5. Is there anything else?
-
+6. Is there anything else?
diff --git a/docs/installation.rst b/docs/installation.rst
index 4e9996c..83ba771 100644
--- a/docs/installation.rst
+++ b/docs/installation.rst
@@ -342,6 +342,24 @@ Note that ``BASE_DIR`` is the path to the project root. It's in the
       }
 
 
+Templates
+=========
+
+We hope you're using Jinja2's newstyle gettext and ``autoescape = True``. If
+that's the case, then these docs will help:
+
+* `Jinja2 template i18n docs <http://jinja.pocoo.org/docs/dev/templates/#i18n>`_
+* `Jinja2 template newstyle docs <http://jinja.pocoo.org/docs/dev/extensions/#newstyle-gettext>`_
+
+Further, Puente adds support for ``pgettext`` and ``npgettext`` in templates::
+
+    {{ pgettext("some context", "message string") }}
+    {{ npgettext("some context", "singular message", "plural message", 5) }}
+
+
+FIXME: Expand on this and talk about escaping and ``|safe``.
+
+
 Extract and merge usage
 =======================
 
diff --git a/docs/migratingfromtower.rst b/docs/migratingfromtower.rst
index 891c07b..04dc570 100644
--- a/docs/migratingfromtower.rst
+++ b/docs/migratingfromtower.rst
@@ -182,6 +182,7 @@ to do something like the following to switch to Puente.
                  ('**.js', 'javascript')
              ]
          }
+         STANDALONE_DOMAINS = ['django']
 
 
       The equivalent Puente configuration is something like this:
diff --git a/puente/commands.py b/puente/commands.py
index 24f07fe..16e5939 100644
--- a/puente/commands.py
+++ b/puente/commands.py
@@ -1,4 +1,5 @@
 import os
+import tempfile
 from subprocess import PIPE, Popen, call
 from tempfile import TemporaryFile
 
@@ -11,6 +12,9 @@ from django.core.management.base import CommandError
 from puente.utils import monkeypatch_i18n
 
 
+DEFAULT_DOMAIN_VALUE = 'all'
+
+
 def generate_options_map():
     """Generate an ``options_map` to pass to ``extract_from_dir``
 
@@ -64,15 +68,16 @@ def generate_options_map():
     )
 
 
-def extract_command(outputdir, domain_methods, text_domain, keywords,
-                    comment_tags, base_dir, project, version,
-                    msgid_bugs_address):
+def extract_command(domain, outputdir, domain_methods, standalone_domains,
+                    text_domain, keywords, comment_tags, base_dir,
+                    project, version, msgid_bugs_address):
     """Extracts strings into .pot files
 
     :arg domain: domains to generate strings for or 'all' for all domains
     :arg outputdir: output dir for .pot files; usually
         locale/templates/LC_MESSAGES/
     :arg domain_methods: DOMAIN_METHODS setting
+    :arg standalone_domains: STANDALONE_DOMAINS setting
     :arg text_domain: TEXT_DOMAIN settings
     :arg keywords: KEYWORDS setting
     :arg comment_tags: COMMENT_TAGS setting
@@ -91,7 +96,11 @@ def extract_command(outputdir, domain_methods, text_domain, keywords,
         print('Creating output dir %s ...' % outputdir)
         os.makedirs(outputdir)
 
-    domains = domain_methods.keys()
+    # Figure out what domains to extract
+    if domain == DEFAULT_DOMAIN_VALUE:
+        domains = domain_methods.keys()
+    else:
+        domains = [domain]
 
     def callback(filename, method, options):
         if method != 'ignore':
@@ -126,15 +135,49 @@ def extract_command(outputdir, domain_methods, text_domain, keywords,
         with open(os.path.join(outputdir, '%s.pot' % domain), 'wb') as fp:
             write_po(fp, catalog, width=80)
 
+    not_standalone_domains = [
+        dom for dom in domains
+        if dom not in standalone_domains
+    ]
+
+    pot_files = []
+    for dom in not_standalone_domains:
+        pot_files.append(os.path.join(outputdir, '%s.pot' % dom))
+
+    if len(pot_files) > 1:
+        pot_file = text_domain + '.pot'
+        print('Concatenating the non-standalone domains into %s' % pot_file)
+
+        final_out = os.path.join(outputdir, pot_file)
+
+        # We add final_out back on because msgcat will combine all
+        # specified files.  We'll redirect everything back in to
+        # final_out in a minute.
+        pot_files.append(final_out)
+
+        meltingpot = tempfile.TemporaryFile()
+        p1 = Popen(['msgcat'] + pot_files, stdout=meltingpot)
+        p1.communicate()
+        meltingpot.seek(0)
+
+        # w+ truncates the file first
+        with open(final_out, 'w+') as final:
+            final.write(meltingpot.read())
+
+        meltingpot.close()
+
+        for dom in not_standalone_domains:
+            os.remove(os.path.join(outputdir, '%s.pot' % dom))
+
     print('Done')
 
 
-def merge_command(create, base_dir, domain_methods, languages):
+def merge_command(create, base_dir, standalone_domains, languages):
     """
     :arg create: whether or not to create directories if they don't
         exist
     :arg base_dir: BASE_DIR setting
-    :arg domain_methods: DOMAIN_METHODS setting
+    :arg standalone_domains: STANDALONE_DOMAINS setting
     :arg languages: LANGUAGES setting
 
     """
@@ -166,9 +209,8 @@ def merge_command(create, base_dir, domain_methods, languages):
             if not os.path.exists(d):
                 os.makedirs(d)
 
-    domains = domain_methods.keys()
-    for domain in domains:
-        print 'Merging %s strings to each locale...' % domain
+    for domain in standalone_domains:
+        print('Merging %s strings to each locale...' % domain)
         domain_pot = os.path.join(locale_dir, 'templates', 'LC_MESSAGES',
                                   '%s.pot' % domain)
         if not os.path.isfile(domain_pot):
diff --git a/puente/ext.py b/puente/ext.py
index 8f3e7a7..ea6714b 100644
--- a/puente/ext.py
+++ b/puente/ext.py
@@ -1,8 +1,28 @@
+from django.utils.translation import pgettext as pgettext_real, npgettext as npgettext_real
+
 from jinja2.ext import InternationalizationExtension
+from jinja2.utils import contextfunction, Markup
 
 from puente.utils import collapse_whitespace
 
 
+@contextfunction
+def pgettext(__context, context, message, **variables):
+    rv = pgettext_real(context, message)
+    if __context.eval_ctx.autoescape:
+        rv = Markup(rv)
+    return rv % variables
+
+
+@contextfunction
+def npgettext(__context, context, singular, plural, number, **variables):
+    variables.setdefault('num', number)
+    rv = npgettext_real(context, singular, plural, number)
+    if __context.eval_ctx.autoescape:
+        rv = Markup(rv)
+    return rv % variables
+
+
 class PuenteI18nExtension(InternationalizationExtension):
     """Provides whitespace collapsing trans behavior
 
@@ -12,6 +32,11 @@ class PuenteI18nExtension(InternationalizationExtension):
     Jinja2 templates.
 
     """
+    def __init__(self, environment):
+        super(PuenteI18nExtension, self).__init__(environment)
+        environment.globals['pgettext'] = pgettext
+        environment.globals['npgettext'] = npgettext
+
     def _parse_block(self, parser, allow_pluralize):
         parse_block = InternationalizationExtension._parse_block
         ref, buffer = parse_block(self, parser, allow_pluralize)
diff --git a/puente/management/commands/extract.py b/puente/management/commands/extract.py
index c688034..de8a5c2 100644
--- a/puente/management/commands/extract.py
+++ b/puente/management/commands/extract.py
@@ -3,7 +3,7 @@ from optparse import make_option
 
 from django.core.management.base import BaseCommand
 
-from puente.commands import extract_command
+from puente.commands import DEFAULT_DOMAIN_VALUE, extract_command
 from puente.settings import get_setting
 
 
@@ -11,6 +11,15 @@ class Command(BaseCommand):
     help = 'Extracts strings for translation.'
 
     option_list = BaseCommand.option_list + (
+        make_option(
+            '--domain', '-d', default=DEFAULT_DOMAIN_VALUE,
+            dest='domain',
+            help=(
+                'The domain of the message files. If "all" '
+                'everything will be extracted and combined into '
+                '%s.pot. (default: %%default).' % get_setting('TEXT_DOMAIN')
+            )
+        ),
         make_option(
             '--output-dir', '-o',
             default=os.path.join(get_setting('BASE_DIR'), 'locale',
@@ -20,15 +29,17 @@ class Command(BaseCommand):
                 'The directory where extracted files will be placed. '
                 '(Default: %default)'
             )
-        ),
+        )
     )
 
     def handle(self, *args, **options):
         return extract_command(
             # Command line arguments
+            domain=options.get('domain'),
             outputdir=options.get('outputdir'),
             # From settings.py
             domain_methods=get_setting('DOMAIN_METHODS'),
+            standalone_domains=get_setting('STANDALONE_DOMAINS'),
             text_domain=get_setting('TEXT_DOMAIN'),
             keywords=get_setting('KEYWORDS'),
             comment_tags=get_setting('COMMENT_TAGS'),
diff --git a/puente/management/commands/merge.py b/puente/management/commands/merge.py
index 74de7d9..b34c2e4 100644
--- a/puente/management/commands/merge.py
+++ b/puente/management/commands/merge.py
@@ -38,7 +38,7 @@ class Command(BaseCommand):
         return merge_command(
             create=options.get('create'),
             base_dir=get_setting('BASE_DIR'),
-            domain_methods=get_setting('DOMAIN_METHODS'),
+            standalone_domains=get_setting('STANDALONE_DOMAINS'),
             languages=getattr(settings, 'LANGUAGES', [])
         )
 
diff --git a/puente/settings.py b/puente/settings.py
index 55609ca..edf5321 100644
--- a/puente/settings.py
+++ b/puente/settings.py
@@ -7,6 +7,12 @@ TEXT_DOMAIN = 'django'
 # Keywords indicating gettext calls
 KEYWORDS = generate_keywords()
 
+# By default, all the domains you speficy will be merged into one big django.po
+# file. If you want to separate a domain from the main .po file, specify it in
+# this list. Make sure to include TEXT_DOMAIN in this list, even if you have
+# other .po files you're generating
+STANDALONE_DOMAINS = [TEXT_DOMAIN]
+
 # Prefixes that indicate a comment tag intended for localizers
 COMMENT_TAGS = ['L10n:', 'L10N:', 'l10n:', 'l10N:', 'Translators:']
 


Test patch:
diff --git a/tests/test_ext.py b/tests/test_ext.py
index c6ebc92..aa912f3 100644
--- a/tests/test_ext.py
+++ b/tests/test_ext.py
@@ -107,6 +107,74 @@ class TestPuenteI18nExtension:
         )
         assert render(tmpl) == '<b>multiple <i>bar</i></b>'
 
+    def test_pgettext(self):
+        tmpl = '{{ pgettext("context", "message") }}'
+        assert render(tmpl) == 'message'
+
+    def test_pgettext_is_safe(self):
+        tmpl = '{{ pgettext("context", "<b>foo</b>") }}'
+        assert render(tmpl) == '<b>foo</b>'
+
+    def test_pgettext_variable_value_notsafe(self):
+        tmpl = '{{ pgettext("context", "<b>%(foo)s</b>", foo="<i>bar</i>") }}'
+        assert render(tmpl) == '<b>&lt;i&gt;bar&lt;/i&gt;</b>'
+
+    def test_pgettext_variable_value_marked_safe_is_safe(self):
+        tmpl = '{{ pgettext("context", "<b>%(foo)s</b>", foo="<i>bar</i>"|safe) }}'
+        assert render(tmpl) == '<b><i>bar</i></b>'
+
+    def test_pgettext_variable_values_autoescape_false(self):
+        tmpl = (
+            '{% autoescape False %}'
+            '{{ pgettext("context", "<b>%(foo)s</b>", foo="<i>bar</i>") }}'
+            '{% endautoescape %}'
+        )
+        assert render(tmpl) == '<b><i>bar</i></b>'
+
+    def test_npgettext(self):
+        tmpl = '{{ npgettext("context", "sing", "plur", 1) }}'
+        assert render(tmpl) == "sing"
+        tmpl = '{{ npgettext("context", "sing", "plur", 2) }}'
+        assert render(tmpl) == "plur"
+
+    def test_npgettext_is_safe(self):
+        tmpl = '{{ npgettext("context", "<b>sing</b>", "<b>plur</b>", 1) }}'
+        assert render(tmpl) == "<b>sing</b>"
+        tmpl = '{{ npgettext("context", "<b>sing</b>", "<b>plur</b>", 2) }}'
+        assert render(tmpl) == "<b>plur</b>"
+
+    def test_npgettext_variable_num(self):
+        tmpl = '{{ npgettext("context", "<b>sing %(num)s</b>", "<b>plur %(num)s</b>", 1) }}'
+        assert render(tmpl) == "<b>sing 1</b>"
+        tmpl = '{{ npgettext("context", "<b>sing %(num)s</b>", "<b>plur %(num)s</b>", 2) }}'
+        assert render(tmpl) == "<b>plur 2</b>"
+
+    def test_npgettext_variable_values_notsafe(self):
+        tmpl = '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 1, foo="<i>bar</i>") }}'
+        assert render(tmpl) == '<b>sing &lt;i&gt;bar&lt;/i&gt;</b>'
+        tmpl = '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 2, foo="<i>bar</i>") }}'
+        assert render(tmpl) == '<b>plur &lt;i&gt;bar&lt;/i&gt;</b>'
+
+    def test_npgettext_variable_value_marked_safe_is_safe(self):
+        tmpl = '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 1, foo="<i>bar</i>"|safe) }}'
+        assert render(tmpl) == '<b>sing <i>bar</i></b>'
+        tmpl = '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 2, foo="<i>bar</i>"|safe) }}'
+        assert render(tmpl) == '<b>plur <i>bar</i></b>'
+
+    def test_npgettext_variable_values_autoescape_false(self):
+        tmpl = (
+            '{% autoescape False %}'
+            '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 1, foo="<i>bar</i>") }}'
+            '{% endautoescape %}'
+        )
+        assert render(tmpl) == '<b>sing <i>bar</i></b>'
+        tmpl = (
+            '{% autoescape False %}'
+            '{{ npgettext("context", "<b>sing %(foo)s</b>", "<b>plur %(foo)s</b>", 2, foo="<i>bar</i>") }}'
+            '{% endautoescape %}'
+        )
+        assert render(tmpl) == '<b>plur <i>bar</i></b>'
+
     def test_trans(self):
         tmpl = '<div>{% trans %}puente rules!{% endtrans %}</div>'
         assert render(tmpl) == '<div>puente rules!</div>'
diff --git a/tests/test_extract.py b/tests/test_extract.py
index bc91f54..0a5a4c5 100644
--- a/tests/test_extract.py
+++ b/tests/test_extract.py
@@ -41,6 +41,7 @@ class TestExtractCommand:
 
         # Extract
         extract_command(
+            domain='all',
             outputdir=str(tmpdir),
             domain_methods={
                 'django': [
@@ -48,6 +49,7 @@ class TestExtractCommand:
                     ('*.html', 'jinja2'),
                 ]
             },
+            standalone_domains=puente_settings.STANDALONE_DOMAINS,
             text_domain=puente_settings.TEXT_DOMAIN,
             keywords=puente_settings.KEYWORDS,
             comment_tags=puente_settings.COMMENT_TAGS,
@@ -81,6 +83,7 @@ class TestExtractCommand:
     def test_header(self, tmpdir):
         # Extract
         extract_command(
+            domain='all',
             outputdir=str(tmpdir),
             domain_methods={
                 'django': [
@@ -88,6 +91,7 @@ class TestExtractCommand:
                     ('*.html', 'jinja2'),
                 ]
             },
+            standalone_domains=puente_settings.STANDALONE_DOMAINS,
             text_domain=puente_settings.TEXT_DOMAIN,
             keywords=puente_settings.KEYWORDS,
             comment_tags=puente_settings.COMMENT_TAGS,
@@ -139,6 +143,7 @@ class TestExtractCommand:
 
         # Extract
         extract_command(
+            domain='all',
             outputdir=str(tmpdir),
             domain_methods={
                 'django': [
@@ -146,6 +151,7 @@ class TestExtractCommand:
                     ('*.html', 'jinja2'),
                 ]
             },
+            standalone_domains=puente_settings.STANDALONE_DOMAINS,
             text_domain=puente_settings.TEXT_DOMAIN,
             keywords=puente_settings.KEYWORDS,
             comment_tags=puente_settings.COMMENT_TAGS,
@@ -187,6 +193,7 @@ class TestExtractCommand:
 
         # Extract
         extract_command(
+            domain='all',
             outputdir=str(tmpdir),
             domain_methods={
                 'django': [
@@ -194,6 +201,7 @@ class TestExtractCommand:
                     ('*.html', 'jinja2'),
                 ]
             },
+            standalone_domains=puente_settings.STANDALONE_DOMAINS,
             text_domain=puente_settings.TEXT_DOMAIN,
             keywords=puente_settings.KEYWORDS,
             comment_tags=puente_settings.COMMENT_TAGS,
@@ -237,6 +245,7 @@ class TestExtractCommand:
 
         # Extract
         extract_command(
+            domain='all',
             outputdir=str(tmpdir),
             domain_methods={
                 'django': [
@@ -244,6 +253,7 @@ class TestExtractCommand:
                     ('*.html', 'jinja2'),
                 ]
             },
+            standalone_domains=puente_settings.STANDALONE_DOMAINS,
             text_domain=puente_settings.TEXT_DOMAIN,
             keywords=puente_settings.KEYWORDS,
             comment_tags=puente_settings.COMMENT_TAGS,
diff --git a/tests/test_merge.py b/tests/test_merge.py
index 29e1456..0278ba1 100644
--- a/tests/test_merge.py
+++ b/tests/test_merge.py
@@ -8,6 +8,7 @@ from django.core.management import CommandError
 from django.test import TestCase
 
 from puente.commands import merge_command
+from puente.settings import get_setting
 
 
 class TestManageMerge(TestCase):
@@ -66,12 +67,7 @@ class TestMergecommand:
         merge_command(
             create=True,
             base_dir=str(tmpdir),
-            domain_methods={
-                'django': [
-                    ('*.py', 'python'),
-                    ('*.html', 'jinja2'),
-                ]
-            },
+            standalone_domains=get_setting('STANDALONE_DOMAINS'),
             languages=['de', 'en-US', 'fr']
         )
 
@@ -84,11 +80,6 @@ class TestMergecommand:
             merge_command(
                 create=True,
                 base_dir=str(tmpdir),
-                domain_methods={
-                    'django': [
-                        ('*.py', 'python'),
-                        ('*.html', 'jinja2'),
-                    ]
-                },
+                standalone_domains=get_setting('STANDALONE_DOMAINS'),
                 languages=['de', 'en-US', 'fr']
             )