Code

Opened 16 months ago

Closed 2 months ago

Last modified 2 months 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 claudep 16 months ago.

Download all attachments as: .zip

Change History (15)

Changed 16 months ago by claudep

comment:1 Changed 16 months ago by claudep

  • Component changed from Core (Management commands) to Internationalization
  • Easy pickings set
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 15 months ago by ramiro

  • Triage Stage changed from Accepted to Ready for checkin

comment:3 follow-up: Changed 15 months ago by claudep

  • Triage Stage changed from Ready for checkin to Accepted

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?

comment:4 in reply to: ↑ 3 ; follow-up: Changed 15 months ago by ramiro

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/

comment:5 in reply to: ↑ 4 Changed 15 months ago by ramiro

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 15 months ago by ramiro (previous) (next) (diff)

comment:6 Changed 15 months ago by ramiro

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 Changed 15 months ago by ramiro

Hopefully finished work, in PR form:

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

comment:8 Changed 15 months ago by Ramiro Morales <cramm0@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 7 months ago by Ramiro Morales <cramm0@…>

In 651bed09182cc7f2fadc54026924fd5815fc47f1:

Made test for issue 19552 compatible with Windows.

Take in account platform path separator. Refs #19552.

comment:10 Changed 7 months ago by Ramiro Morales <cramm0@…>

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 Changed 3 months ago by Ihor Kaharlichenko <madkinder@…>

  • Resolution fixed deleted
  • Status changed from closed to new

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 Changed 3 months ago by Ihor Kaharlichenko <madkinder@…>

  • Cc madkinder@… added

comment:13 Changed 2 months ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 2 months ago by Ihor Kaharlichenko <madkinder@…>

Extracted as a separate issue: #21963.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.