Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#19552 closed Bug (fixed)

makemessages ignores translations in templates with inline comment tags

Reported by: juneih@… Owned by: nobody
Component: Internationalization Version: 1.4
Severity: Normal Keywords: makemessages, template, gettext, xgettext
Cc: madkinder@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

I'm using the management command makemessages to create the po-file for translations in my Django project. I'm having trouble with translations in templates being ignored when they contain in-line comments. Example:

{# Nice comment #} {%trans "Translate me" %}

The string 'Translate me' is not added to the po-file. I've traced the reason for this to the templatize method in trans_real.py. At the bottom of the method you'll find the following lines:

When running the makemessages management command

 elif t.token_type == TOKEN_COMMENT:
    out.write(' # %s' % t.contents)

It appears that the comment is not blanked out, but kept and passed on to xgettext. The input for xgettext with the previous example will be:

# nice comment gettext('Translate me')

Which means xgettext interprets the entire line as a comment. I'm not really sure why gettext would need the comment in the first place, but the fix for me has been to just add a new line after the string, like so:

 elif t.token_type == TOKEN_COMMENT:
    out.write(' # %s\n' % t.contents)

Now the input for xgettext is:

# nice comment 
gettext('Translate me')

Attachments (1)

19552.diff (3.0 KB ) - added by Claude Paroz 12 years ago.

Download all attachments as: .zip

Change History (15)

by Claude Paroz, 12 years ago

Attachment: 19552.diff added

comment:1 by Claude Paroz, 12 years ago

Component: Core (Management commands)Internationalization
Easy pickings: set
Has patch: set
Triage Stage: UnreviewedAccepted

comment:2 by Ramiro Morales, 12 years ago

Triage Stage: AcceptedReady for checkin

comment:3 by Claude Paroz, 12 years ago

Triage Stage: Ready for checkinAccepted

I realized that my patch (with the OP proposal) has one major drawback, it doesn't preserve the line numbers. We've made considerable efforts in templatize to preserve line numbers, so as line numbers in po files match line numbers in the source file. For comments which are not translator comments, we could replace # by ''' ''', but this doesn't work for translator comments. Ramiro, any idea?

in reply to:  3 ; comment:4 by Ramiro Morales, 12 years ago

Replying to claudep:

I realized that my patch (with the OP proposal) has one major drawback, it doesn't preserve the line numbers. We've made considerable efforts in templatize to preserve line numbers, so as line numbers in po files match line numbers in the source file. For comments which are not translator comments, we could replace # by ''' ''', but this doesn't work for translator comments. Ramiro, any idea?

Maybe something like this? (needs validation of the idea, polishing and tests): http://dpaste.de/hJepF/

in reply to:  4 comment:5 by Ramiro Morales, 12 years ago

Replying to ramiro:

Maybe something like this? (needs validation of the idea, polishing and tests): http://dpaste.de/hJepF/

Ignore that patch. Moving comments to the end of the line helps in the general case but breaks association of translator comments with the right translatable content.

I believe we should:

1) Ignore all the template comments.
2) Get any no-comment tags that might be located in the same line as a comment one to be properly recognized and extracted and
3) Start requiring translator comments to be on a line by their own by ignoring those that don't comply just like as in 1 but possibly making makemessages emit a warning. I have some code along these lines, will upload it tomorrow.

Version 1, edited 12 years ago by Ramiro Morales (previous) (next) (diff)

comment:6 by Ramiro Morales, 12 years ago

New proposed fix:

https://github.com/ramiro/django/compare/57d439e...ramiro:ticket-19552

Compared with item 3 of the previous comment proposal, this implementation doesn't disallow translator comments in the same line with other tokens, but ignores/warns if they aren't located at the end.

comment:7 by Ramiro Morales, 12 years ago

Hopefully finished work, in PR form:

https://github.com/django/django/pull/677

comment:8 by Ramiro Morales <cramm0@…>, 12 years ago

Resolution: fixed
Status: newclosed

In 47ddd6a4082d55d8856b7e6beac553485dd627f7:

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.

comment:9 by Ramiro Morales <cramm0@…>, 11 years ago

In 651bed09182cc7f2fadc54026924fd5815fc47f1:

Made test for issue 19552 compatible with Windows.

Take in account platform path separator. Refs #19552.

comment:10 by Ramiro Morales <cramm0@…>, 11 years ago

In 4290cc1d6e3cbbb77d4098e15bd4db6c65ee904a:

[1.6.x] Made test for issue 19552 compatible with Windows.

Take in account platform path separator. Refs #19552.

651bed0918 from master.

comment:11 by Ihor Kaharlichenko <madkinder@…>, 11 years ago

Resolution: fixed
Status: closednew

Unfortunately the bug seems to be fixed partially. Here's the test case:

Template:

{% load i18n %}
 
{# Translators: Abbreviated month name #}
{% trans "Jan" %}
 
{# Translators: Abbreviated month name #}{% trans "Feb" %}
 
{% comment %}Translators: Abbreviated month name{% endcomment %}{% trans "Mar" %}
 
{# Translators: Abbreviated month name #} {% trans "Apr" %}
 
{% comment %}Translators: Abbreviated month name{% endcomment %} {% trans "May" %}

{# Translators: Abbreviated month name #}
{% trans "Jun" %}

{# Translators: Abbreviated month name #}

{% trans "Jul" %}

{% comment %}Translators: Abbreviated month name{% endcomment %}

{% trans "Aug" %}

The extraction session:

$ ./manage.py version
1.6.1
$ ./manage.py makemessages -l ru -i venv
.../trans_real.py:585: TranslatorCommentWarning: The translator-targeted comment 'Translators: Abbreviated month name' (file app1/templates/i18n_test.html, line 6) was ignored, because it wasn't the last item on the line.
  warnings.warn(warn_msg, TranslatorCommentWarning)

.../trans_real.py:585: TranslatorCommentWarning: The translator-targeted comment 'Translators: Abbreviated month name' (file app1/templates/i18n_test.html, line 10) was ignored, because it wasn't the last item on the line.
  warnings.warn(warn_msg, TranslatorCommentWarning)

processing locale ru

Resulting po file:

#. Translators: Abbreviated month name
#: app1/templates/i18n_test.html:4
msgid "Jan"
msgstr ""

#: app1/templates/i18n_test.html:6
msgid "Feb"
msgstr ""

#. Translators: Abbreviated month name gettext(u'Mar')
#: app1/templates/i18n_test.html:10
msgid "Apr"
msgstr ""

#. Translators: Abbreviated month name  gettext(u'May')
#. Translators: Abbreviated month name
#: app1/templates/i18n_test.html:15
msgid "Jun"
msgstr ""

#. Translators: Abbreviated month name
#: app1/templates/i18n_test.html:19
msgid "Jul"
msgstr ""

#. Translators: Abbreviated month name
#: app1/templates/i18n_test.html:23
msgid "Aug"
msgstr ""

The problems:

  • translations for "Mar" and "May" were completely skipped
  • there was a warning for single-tag comments (for Feb and Apr), but not for block comments (for Mar and May), which is inconsistent
  • Apr and Jun got wrong comments

comment:12 by Ihor Kaharlichenko <madkinder@…>, 11 years ago

Cc: madkinder@… added

comment:13 by Tim Graham, 11 years ago

Resolution: fixed
Status: newclosed

As noted in the previous commit message: "Behavior of {% comment %}...{% endcomment %}tags remains unchanged." I think that means inline translations aren't supported for that tag.

I'm not sure if this is something that could or should be changed, or rather if the limitation should simplify be documented if it's not already. In either case, a new ticket is better than reopening this old one, thanks.

comment:14 by Ihor Kaharlichenko <madkinder@…>, 11 years ago

Extracted as a separate issue: #21963.

Note: See TracTickets for help on using tickets.
Back to Top