From 47ddd6a4082d55d8856b7e6beac553485dd627f7 Mon Sep 17 00:00:00 2001 From: Ramiro Morales Date: Sun, 20 Jan 2013 15:07:10 -0300 Subject: [PATCH] Fixed #19552 -- Enhanced makemessages handling of ``{# #}``-style template comments. They are simply ignored now. This allows for a more correct behavior when they are placed before translatable constructs on the same line. Previously, the latter were wrongly ignored because the former were preserved when converting template code to the internal Python-syntax form later fed to xgettext but Python has no ``/* ... */``-style comments. Also, special comments directed to translators are now only taken in account when they are located at the end of a line. e.g.:: {# Translators: ignored #}{% trans "Literal A" %}{# Translators: valid, associated with "Literal B" below #} {% trans "Literal B" %} Behavior of ``{% comment %}...{% endcomment %}``tags remains unchanged. Thanks juneih at redpill-linpro dot com for the report and Claude for his work on the issue. --- django/utils/translation/__init__.py | 5 ++ django/utils/translation/trans_real.py | 31 +++++++- docs/releases/1.6.txt | 20 +++++ docs/topics/i18n/translation.txt | 75 +++++++++++++++++-- docs/topics/templates.txt | 2 + .../i18n/commands/extraction.py | 60 +++++++++++++++ .../i18n/commands/templates/comments.thtml | 13 ++++ 7 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 tests/regressiontests/i18n/commands/templates/comments.thtml diff --git a/django/utils/translation/__init__.py b/django/utils/translation/__init__.py index f3cc6348f6..803bbb746a 100644 --- a/django/utils/translation/__init__.py +++ b/django/utils/translation/__init__.py @@ -21,6 +21,11 @@ __all__ = [ 'npgettext', 'npgettext_lazy', ] + +class TranslatorCommentWarning(SyntaxWarning): + pass + + # Here be dragons, so a short explanation of the logic won't hurt: # We are trying to solve two problems: (1) access settings, in particular # settings.USE_I18N, as late as possible, so that modules can be imported diff --git a/django/utils/translation/trans_real.py b/django/utils/translation/trans_real.py index cf6270cc0c..8014b5ea3a 100644 --- a/django/utils/translation/trans_real.py +++ b/django/utils/translation/trans_real.py @@ -7,6 +7,7 @@ import re import sys import gettext as gettext_module from threading import local +import warnings from django.utils.importlib import import_module from django.utils.encoding import force_str, force_text @@ -14,6 +15,7 @@ from django.utils._os import upath from django.utils.safestring import mark_safe, SafeData from django.utils import six from django.utils.six import StringIO +from django.utils.translation import TranslatorCommentWarning # Translations are cached in a dictionary for every language+app tuple. @@ -41,6 +43,7 @@ accept_language_re = re.compile(r''' language_code_prefix_re = re.compile(r'^/([\w-]+)(/|$)') + def to_locale(language, to_lower=False): """ Turns a language name (en-us) into a locale name (en_US). If 'to_lower' is @@ -468,6 +471,9 @@ def templatize(src, origin=None): plural = [] incomment = False comment = [] + lineno_comment_map = {} + comment_lineno_cache = None + for t in Lexer(src, origin).tokenize(): if incomment: if t.token_type == TOKEN_BLOCK and t.contents == 'endcomment': @@ -529,7 +535,27 @@ def templatize(src, origin=None): plural.append(contents) else: singular.append(contents) + else: + # Handle comment tokens (`{# ... #}`) plus other constructs on + # the same line: + if comment_lineno_cache is not None: + cur_lineno = t.lineno + t.contents.count('\n') + if comment_lineno_cache == cur_lineno: + if t.token_type != TOKEN_COMMENT: + for c in lineno_comment_map[comment_lineno_cache]: + filemsg = '' + if origin: + filemsg = 'file %s, ' % origin + warn_msg = ("The translator-targeted comment '%s' " + "(%sline %d) was ignored, because it wasn't the last item " + "on the line.") % (c, filemsg, comment_lineno_cache) + warnings.warn(warn_msg, TranslatorCommentWarning) + lineno_comment_map[comment_lineno_cache] = [] + else: + out.write('# %s' % ' | '.join(lineno_comment_map[comment_lineno_cache])) + comment_lineno_cache = None + if t.token_type == TOKEN_BLOCK: imatch = inline_re.match(t.contents) bmatch = block_re.match(t.contents) @@ -586,7 +612,10 @@ def templatize(src, origin=None): else: out.write(blankout(p, 'F')) elif t.token_type == TOKEN_COMMENT: - out.write(' # %s' % t.contents) + if t.contents.lstrip().startswith(TRANSLATOR_COMMENT_MARK): + lineno_comment_map.setdefault(t.lineno, + []).append(t.contents) + comment_lineno_cache = t.lineno else: out.write(blankout(t.contents, 'X')) return force_str(out.getvalue()) diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index e0c07c40fe..79fa3ffb86 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -47,6 +47,26 @@ Backwards incompatible changes in 1.6 should review it as ``type='text'`` widgets might be now output as ``type='email'`` or ``type='url'`` depending on their corresponding field type. +* Extraction of translatable literals from templates with the + :djadmin:`makemessages` command now correctly detects i18n constructs when + they are located after a ``{#`` / ``#}``-type comment on the same line. E.g.: + + .. code-block:: html+django + + {# A comment #}{% trans "This literal was incorrectly ignored. Not anymore" %} + +* (Related to the above item.) Validation of the placement of + :ref:`translator-comments-in-templates` specified using ``{#`` / ``#}`` is now + stricter. All translator comments not located at the end of their respective + lines in a template are ignored and a warning is generated by + :djadmin:`makemessages` when it finds them. E.g.: + + .. code-block:: html+django + + {# Translators: This is ignored #}{% trans "Translate me" %} + {{ title }}{# Translators: Extracted and associated with 'Welcome' below #} +

{% trans "Welcome" %}

+ .. warning:: In addition to the changes outlined in this section, be sure to review the diff --git a/docs/topics/i18n/translation.txt b/docs/topics/i18n/translation.txt index 01f168bc10..3cf08e7ddf 100644 --- a/docs/topics/i18n/translation.txt +++ b/docs/topics/i18n/translation.txt @@ -142,14 +142,22 @@ preceding the string, e.g.:: # Translators: This message appears on the home page only output = ugettext("Welcome to my site.") -This also works in templates with the :ttag:`comment` tag: +The comment will then appear in the resulting ``.po`` file associated with the +translatable contruct located below it and should also be displayed by most +translation tools. -.. code-block:: html+django +.. note:: Just for completeness, this is the corresponding fragment of the + resulting ``.po`` file: - {% comment %}Translators: This is a text of the base template {% endcomment %} + .. code-block:: po -The comment will then appear in the resulting ``.po`` file and should also be -displayed by most translation tools. + #. Translators: This message appears on the home page only + # path/to/python/file.py:123 + msgid "Welcome to my site." + msgstr "" + +This also works in templates. See :ref:`translator-comments-in-templates` for +more details. Marking strings as no-op ------------------------ @@ -620,6 +628,63 @@ markers` using the ``context`` keyword: {% blocktrans with name=user.username context "greeting" %}Hi {{ name }}{% endblocktrans %} +.. _translator-comments-in-templates: + +Comments for translators in templates +------------------------------------- + +Just like with :ref:`Python code `, these notes for +translators can be specified using comments, either with the :ttag:`comment` +tag: + +.. code-block:: html+django + + {% comment %}Translators: View verb{% endcomment %} + {% trans "View" %} + + {% comment %}Translators: Short intro blurb{% endcomment %} +

{% blocktrans %}A multiline translatable + literal.{% endblocktrans %}

+ +or with the ``{#`` ... ``#}`` :ref:`one-line comment constructs `: + +.. code-block:: html+django + + {# Translators: Label of a button that triggers search{% endcomment #} + + + {# Translators: This is a text of the base template #} + {% blocktrans %}Ambiguous translatable block of text{% endtransblock %} + +.. note:: Just for completeness, these are the corresponding fragments of the + resulting ``.po`` file: + + .. code-block:: po + + #. Translators: View verb + # path/to/template/file.html:10 + msgid "View" + msgstr "" + + #. Translators: Short intro blurb + # path/to/template/file.html:13 + msgid "" + "A multiline translatable" + "literal." + msgstr "" + + # ... + + #. Translators: Label of a button that triggers search + # path/to/template/file.html:100 + msgid "Go" + msgstr "" + + #. Translators: + # path/to/template/file.html:103 + msgid "Ambiguous translatable block of text" + msgstr "" + .. _template-translation-vars: Other tags diff --git a/docs/topics/templates.txt b/docs/topics/templates.txt index fb2119515b..58a3ee9870 100644 --- a/docs/topics/templates.txt +++ b/docs/topics/templates.txt @@ -250,6 +250,8 @@ You can also create your own custom template tags; see tags and filters available for a given site. See :doc:`/ref/contrib/admin/admindocs`. +.. _template-comments: + Comments ======== diff --git a/tests/regressiontests/i18n/commands/extraction.py b/tests/regressiontests/i18n/commands/extraction.py index ac8d8c1a09..0367d23ec6 100644 --- a/tests/regressiontests/i18n/commands/extraction.py +++ b/tests/regressiontests/i18n/commands/extraction.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals import os import re import shutil +import warnings from django.core import management from django.test import SimpleTestCase @@ -11,6 +12,7 @@ from django.utils.encoding import force_text from django.utils._os import upath from django.utils import six from django.utils.six import StringIO +from django.utils.translation import TranslatorCommentWarning LOCALE='de' @@ -120,6 +122,7 @@ class BasicExtractorTests(ExtractorTests): self.assertFalse(os.path.exists('./templates/template_with_error.tpl.py')) def test_extraction_warning(self): + """test xgettext warning about multiple bare interpolation placeholders""" os.chdir(self.test_dir) shutil.copyfile('./code.sample', './code_sample.py') stdout = StringIO() @@ -172,6 +175,63 @@ class BasicExtractorTests(ExtractorTests): self.assertTrue('msgctxt "Special blocktrans context wrapped in double quotes"' in po_contents) self.assertTrue('msgctxt "Special blocktrans context wrapped in single quotes"' in po_contents) + def test_template_comments(self): + """Template comment tags on the same line of other constructs (#19552)""" + os.chdir(self.test_dir) + # Test detection/end user reporting of old, incorrect templates + # translator comments syntax + with warnings.catch_warnings(record=True) as ws: + warnings.simplefilter('always') + management.call_command('makemessages', locale=LOCALE, extensions=['thtml'], verbosity=0) + self.assertEqual(len(ws), 3) + for w in ws: + self.assertTrue(issubclass(w.category, TranslatorCommentWarning)) + six.assertRegex(self, str(ws[0].message), + r"The translator-targeted comment 'Translators: ignored i18n comment #1' \(file templates/comments.thtml, line 4\) was ignored, because it wasn't the last item on the line\." + ) + six.assertRegex(self, str(ws[1].message), + r"The translator-targeted comment 'Translators: ignored i18n comment #3' \(file templates/comments.thtml, line 6\) was ignored, because it wasn't the last item on the line\." + ) + six.assertRegex(self, str(ws[2].message), + r"The translator-targeted comment 'Translators: ignored i18n comment #4' \(file templates/comments.thtml, line 8\) was ignored, because it wasn't the last item on the line\." + ) + # Now test .po file contents + self.assertTrue(os.path.exists(self.PO_FILE)) + with open(self.PO_FILE, 'r') as fp: + po_contents = force_text(fp.read()) + + self.assertMsgId('Translatable literal #9a', po_contents) + self.assertFalse('ignored comment #1' in po_contents) + + self.assertFalse('Translators: ignored i18n comment #1' in po_contents) + self.assertMsgId("Translatable literal #9b", po_contents) + + self.assertFalse('ignored i18n comment #2' in po_contents) + self.assertFalse('ignored comment #2' in po_contents) + self.assertMsgId('Translatable literal #9c', po_contents) + + self.assertFalse('ignored comment #3' in po_contents) + self.assertFalse('ignored i18n comment #3' in po_contents) + self.assertMsgId('Translatable literal #9d', po_contents) + + self.assertFalse('ignored comment #4' in po_contents) + self.assertMsgId('Translatable literal #9e', po_contents) + self.assertFalse('ignored comment #5' in po_contents) + + self.assertFalse('ignored i18n comment #4' in po_contents) + self.assertMsgId('Translatable literal #9f', po_contents) + self.assertTrue('#. Translators: valid i18n comment #5' in po_contents) + + self.assertMsgId('Translatable literal #9g', po_contents) + self.assertTrue('#. Translators: valid i18n comment #6' in po_contents) + self.assertMsgId('Translatable literal #9h', po_contents) + self.assertTrue('#. Translators: valid i18n comment #7' in po_contents) + self.assertMsgId('Translatable literal #9i', po_contents) + + six.assertRegex(self, po_contents, r'#\..+Translators: valid i18n comment #8') + six.assertRegex(self, po_contents, r'#\..+Translators: valid i18n comment #9') + self.assertMsgId("Translatable literal #9j", po_contents) + class JavascriptExtractorTests(ExtractorTests): diff --git a/tests/regressiontests/i18n/commands/templates/comments.thtml b/tests/regressiontests/i18n/commands/templates/comments.thtml new file mode 100644 index 0000000000..90eb5f1792 --- /dev/null +++ b/tests/regressiontests/i18n/commands/templates/comments.thtml @@ -0,0 +1,13 @@ +{% load i18n %} + +{# ignored comment #1 #}{% trans "Translatable literal #9a" %} +{# Translators: ignored i18n comment #1 #}{% trans "Translatable literal #9b" %} +{# Translators: ignored i18n comment #2 #}{# ignored comment #2 #}{% trans "Translatable literal #9c" %} +{# ignored comment #3 #}{# Translators: ignored i18n comment #3 #}{% trans "Translatable literal #9d" %} +{# ignored comment #4 #}{% trans "Translatable literal #9e" %}{# ignored comment #5 #} +{# Translators: ignored i18n comment #4 #}{% trans "Translatable literal #9f" %}{# Translators: valid i18n comment #5 #} +{% trans "Translatable literal #9g" %}{# Translators: valid i18n comment #6 #} +{# ignored comment #6 #}{% trans "Translatable literal #9h" %}{# Translators: valid i18n comment #7 #} +{% trans "Translatable literal #9i" %} +{# Translators: valid i18n comment #8 #}{# Translators: valid i18n comment #9 #} +{% trans "Translatable literal #9j" %}