Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26341 closed Bug (fixed)

Weird comments in PO files (.html.py filenames)

Reported by: Sylvain Garancher Owned by: Veranika
Component: Internationalization Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I just upgraded Django to 1.9.4 from 1.8.10, and sometimes, the filenames in PO files comments contain ".html.py" extensions.

This is visible on django's main repository :
https://github.com/django/django/blob/ae4d932b1ac12651a7c57d89742c25483ee8c9f9/django/contrib/admin/locale/en/LC_MESSAGES/django.po#L282
https://github.com/django/django/blob/4323676ea5ab6994feb1385522665069d84f397b/django/contrib/admin/locale/en/LC_MESSAGES/django.po#L302

In this example, the "contrib/admin/templates/admin/base_site.html" file is now named "contrib/admin/templates/admin/base_site.html.py" (with a trailing ".py") in the po file.
This seems to appear only on lines with a python file before the template html file.

claudep found that this could be the faulty commit : https://github.com/django/django/commit/e75882332c

Change History (15)

comment:1 by Claude Paroz, 8 years ago

Triage Stage: UnreviewedAccepted

We used to create messages file by file, and then we could make a simple replace to remove that temporary .py extension. Now that we are calling gettext for a bunch of files, some source comments might contain more than one source on a single line. The replacement algorithm in postprocess_messages of the makemessages command doesn't catch those occurrences.

comment:2 by Sergey Kolosov, 8 years ago

Owner: changed from nobody to Sergey Kolosov
Status: newassigned

Working on it

comment:3 by Veranika, 8 years ago

Has patch: set
Owner: changed from Sergey Kolosov to Veranika

comment:4 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

Left a few cosmetic comments. Claude, could you check it too?

comment:5 by Tim Graham, 8 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

There's a test failure in the latest iteration of the patch.

comment:6 by Claude Paroz, 8 years ago

Patch needs improvement: unset

New PR.

comment:7 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Claude Paroz, 8 years ago

Should I backport to 1.9?
The note could be:
Fixed a ``makemessages`` regression where temporary ``.py`` extensions were leaked in source file paths (:ticket:`26341`).

Last edited 8 years ago by Tim Graham (previous) (diff)

comment:9 by Tim Graham, 8 years ago

Fine by me. Note looks good.

comment:10 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In b16b1249:

Fixed #26341 -- Fixed makemessages breaking location comments for HTML files

Thanks Sylvain Garancher for the report and Veranika Sabiashchanskaya for the
initial patch.

comment:11 by Claude Paroz <claude@…>, 8 years ago

In 4a6bdff4:

[1.9.x] Fixed #26341 -- Fixed makemessages breaking location comments for HTML files

Thanks Sylvain Garancher for the report and Veranika Sabiashchanskaya for the
initial patch.
Backport of b16b124996 from master.

comment:12 by Claude Paroz <claude@…>, 8 years ago

In 004fa09d:

[1.9.x] Fixed source path in contrib.admin translation catalog

Refs #26341.

comment:13 by Claude Paroz <claude@…>, 8 years ago

In b0068af:

Fixed source path in contrib.admin translation catalog

Refs #26341.

comment:14 by Claude Paroz <claude@…>, 8 years ago

In 4731e9d:

Fixed #26341 (again) -- Addressed multiple occurrences per line use case

comment:15 by Claude Paroz <claude@…>, 8 years ago

In 9c12eb5:

[1.9.x] Fixed #26341 (again) -- Addressed multiple occurrences per line use case

Backport of 4731e9d8 from master.

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