Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for ticket #21963 and proper fix for previous issue #19552 #2410

Closed
wants to merge 1 commit into from

2 participants

@dzhibas

Inline translators comment and comment blocks in PO files now are in correct places and not ignored.

New test case added with months from ticket #21963, previous test for issue #19552 is modified because of incorrect behavior (ignoring inline comments in front and placing others for wrong translation strings)

@dzhibas dzhibas Fix for ticket #21963 and proper fix for previous issue #19552
Inline translators comment and comment block in PO files now are in correct places and not ignored
048771d
@timgraham
Owner

buildbot, test this please

@timgraham
Owner

Could you update your branch to merge cleanly? That should resolve at least some of the test failures.

@timgraham
Owner

Closing in absence of follow-up. Please send a new PR if you can update it, thanks!

See also our patch review checklist.

@timgraham timgraham closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 7, 2014
  1. @dzhibas

    Fix for ticket #21963 and proper fix for previous issue #19552

    dzhibas authored
    Inline translators comment and comment block in PO files now are in correct places and not ignored
This page is out of date. Refresh to see the latest.
View
44 django/utils/translation/trans_real.py
@@ -548,8 +548,6 @@ def templatize(src, origin=None):
plural = []
incomment = False
comment = []
- lineno_comment_map = {}
- comment_lineno_cache = None
def join_tokens(tokens, trim=False):
message = ''.join(tokens)
@@ -566,8 +564,18 @@ def join_tokens(tokens, trim=False):
if line.lstrip().startswith(TRANSLATOR_COMMENT_MARK):
translators_comment_start = lineno
for lineno, line in enumerate(content.splitlines(True)):
+ multiline_comment_block = len(content.splitlines(True)) > 1
if translators_comment_start is not None and lineno >= translators_comment_start:
- out.write(' # %s' % line)
+ if not multiline_comment_block:
+ # move comment into last new line at the end of line
+ new_line_pos = out.getvalue().rindex('\n')
+ out.seek(new_line_pos + 1)
+ temp_buffer = out.read()
+ out.seek(new_line_pos)
+ comment_block_content = ' # %s \n%s' % (line, temp_buffer)
+ else:
+ comment_block_content = ' # %s ' % line
+ out.write(comment_block_content)
else:
out.write(' #\n')
incomment = False
@@ -628,25 +636,6 @@ def join_tokens(tokens, trim=False):
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)
@@ -658,6 +647,7 @@ def join_tokens(tokens, trim=False):
elif g[0] == "'":
g = g.strip("'")
g = one_percent_re.sub('%%', g)
+
if imatch.group(2):
# A context is provided
context_match = context_re.match(imatch.group(2))
@@ -705,9 +695,13 @@ def join_tokens(tokens, trim=False):
out.write(blankout(p, 'F'))
elif t.token_type == TOKEN_COMMENT:
if t.contents.lstrip().startswith(TRANSLATOR_COMMENT_MARK):
- lineno_comment_map.setdefault(t.lineno,
- []).append(t.contents)
- comment_lineno_cache = t.lineno
+ new_line_pos = out.getvalue().rindex('\n')
+ out.seek(new_line_pos + 1)
+ temp_buffer = out.read()
+ out.seek(new_line_pos)
+ comment_block_content = '# %s ' % t.contents
+ out.write(comment_block_content + '\n' + temp_buffer)
+
else:
out.write(blankout(t.contents, 'X'))
return force_str(out.getvalue())
View
27 tests/i18n/commands/templates/21963_comments.thtml
@@ -0,0 +1,27 @@
+{% load i18n %}
+
+{# Translators: Abbreviated month name 'Jan' #}
+{% trans "Jan" %}
+
+{# Translators: Abbreviated month name 'Feb'#}{% trans "Feb" %}
+
+{% comment %}Translators: Abbreviated month name 'Mar'{% endcomment %}{% trans "Mar" %}
+
+{# Translators: Abbreviated month name 'Apr' #} {% trans "Apr" %}
+
+{% comment %}Translators: Abbreviated month name 'May'{% endcomment %} {% trans "May" %}
+
+{# Translators: Abbreviated month name 'Jun'#}
+{% trans "Jun" %}
+
+{# Translators: Abbreviated month name 'Jul'#}
+
+{% trans "Jul" %}
+
+{% comment %}Translators: Abbreviated month name 'Aug'{% endcomment %}
+
+{% trans "Aug" %}
+
+{% trans "Sep" %} {#Translators: Abbreviated month name 'Sep'#} some text after
+
+{% trans "Oct" %} some text in between {% comment %}Translators: Abbreviated month name 'Oct'{% endcomment %}
View
14 tests/i18n/commands/templates/comments.thtml
@@ -1,13 +1,19 @@
{% 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" %}
+{# Translators: i18n comment #1 #}{% trans "Translatable literal #9b" %}
+{# Translators: i18n comment #2 #}{# ignored comment #2 #}{% trans "Translatable literal #9c" %}
+{# ignored comment #3 #}{# Translators: 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 #}
+{# Translators: 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" %}
+{% comment %}Translators: i18n comment #9w{% endcomment %}{% trans "Translatable literal #9w" %} {% comment %}Translators: i18n comment #9w after{% endcomment %}
+something {% comment %}Translators: i18n comment #9z{% endcomment %} DEMO DATA IN BETWEEN {% trans "Translatable literal #9z" %}
+{% comment %}Translators: i18n comment #9x{% endcomment %}
+{% trans "Translatable literal #9x" %}
+
+{# Translators: valid i18n comment #10 #}{# Translators: valid i18n comment #11 #} {% trans "Translatable literal #10a" %}
View
96 tests/i18n/test_extraction.py
@@ -254,29 +254,68 @@ def test_context_in_single_quotes(self):
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)"""
+ def test_inline_translators_template_comments(self):
+ # ticket #21963 and #19552 replacement
os.chdir(self.test_dir)
- # Test detection/end user reporting of old, incorrect templates
- # translator comments syntax
+
+ # should be 0 warnings captured during command call
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\."
- )
+ self.assertEqual(len(ws), 0)
+
+ # Now test .po file contents and if all translations is in correct places
+ # with correct inline comments
+ self.assertTrue(os.path.exists(self.PO_FILE))
+ with open(self.PO_FILE, 'r') as fp:
+ po_contents = force_text(fp.read())
+
+ self.assertMsgId('Jan', po_contents)
+ self.assertTrue("#. Translators: Abbreviated month name 'Jan'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 4, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Feb', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Feb'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 6, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Mar', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Mar'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 8, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Apr', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Apr'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 10, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('May', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'May'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 12, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Jun', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Jun'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 15, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Jul', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Jul'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 19, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Aug', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Aug'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 23, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Sep', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Sep'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 25, 'templates', '21963_comments.thtml')
+
+ self.assertMsgId('Oct', po_contents)
+ self.assertTrue("Translators: Abbreviated month name 'Oct'" in po_contents)
+ self.assertLocationCommentPresent(self.PO_FILE, 27, 'templates', '21963_comments.thtml')
+
+ def test_template_comments(self):
+ """Template comment tags on the same line of other constructs (#19552)"""
+ os.chdir(self.test_dir)
+
+ management.call_command('makemessages', locale=[LOCALE], extensions=['thtml'], verbosity=0)
+
# Now test .po file contents
self.assertTrue(os.path.exists(self.PO_FILE))
with open(self.PO_FILE, 'r') as fp:
@@ -285,24 +324,24 @@ def test_template_comments(self):
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.assertTrue('Translators: i18n comment #1' in po_contents)
self.assertMsgId("Translatable literal #9b", po_contents)
- self.assertFalse('ignored i18n comment #2' in po_contents)
+ self.assertTrue('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.assertTrue('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.assertTrue('i18n comment #4' in po_contents)
+ self.assertTrue('valid i18n comment #5' 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)
@@ -312,8 +351,17 @@ def test_template_comments(self):
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)
+ six.assertRegex(self, po_contents, r'#\..+Translators: i18n comment #9w')
+ self.assertMsgId("Translatable literal #9w", po_contents)
+
+ six.assertRegex(self, po_contents, r'#\..+Translators: i18n comment #9z')
+ self.assertMsgId("Translatable literal #9z", po_contents)
+
+ six.assertRegex(self, po_contents, r'#\..+Translators: i18n comment #9x')
+ self.assertMsgId("Translatable literal #9x", po_contents)
class JavascriptExtractorTests(ExtractorTests):
Something went wrong with that request. Please try again.