Opened 14 years ago

Closed 8 years ago

#13936 closed Bug (needsinfo)

django-admin makemessages generates PO files with an incorrect path to source code files

Reported by: Cristian Ciupitu Owned by: nobody
Component: Core (Management commands) Version: 1.2
Severity: Normal Keywords:
Cc: vslavik@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Steps to reproduce:

  1. Run django-admin makemessages -l ro -e html,txt,rml above the locale directory to generate the PO files.
  1. Open a PO file with Poedit, e.g. locale/ro/LC_MESSAGES/django.po.
  1. Try to see where the message is being referenced/used.

Actual result:

Poedit will not be able to find the source code.

Expected result:

Poedit should be able to find the source code.

Additional info:

The paths from the catalog look like this:

#: admin.py:12 admin.py:23

so Poedit tries to open locale/ro/LC_MESSAGES/admin.py which of course does not exist. I propose using a relative path, e.g ../../../admin.py.

I'm using Django-1.2.1-5.fc13.noarch and I asked about this on both StackOverflow and the Poedit users mailing list.

Attachments (2)

makemessages.py.patch (2.3 KB ) - added by Cristian Ciupitu 14 years ago.
Patch for "makemessages.py"
makemessages.py.diff (5.6 KB ) - added by Cristian Ciupitu 14 years ago.
Patch for "makemessages.py"

Download all attachments as: .zip

Change History (23)

in reply to:  description ; comment:1 by Ramiro Morales, 14 years ago

Replying to ciupicri:

Steps to reproduce:

  1. Run django-admin makemessages -l ro -e html,txt,rml above the locale directory to generate the PO files.

What do you mean with above the locale directory?. Please provide more detail about that specific part of your report.

in reply to:  1 ; comment:2 by Cristian Ciupitu, 14 years ago

Replying to ramiro:

Replying to ciupicri:

Steps to reproduce:

  1. Run django-admin makemessages -l ro -e html,txt,rml above the locale directory to generate the PO files.

What do you mean with above the locale directory?. Please provide more detail about that specific part of your report.

The root of the application. Here for example (as instructed in the documentation for Satchmo).

in reply to:  2 ; comment:3 by Ramiro Morales, 14 years ago

Resolution: invalid
Status: newclosed

Replying to ciupicri:

Replying to ramiro:
The root of the application. Here for example (as instructed in the documentation for Satchmo).

Contrarily to what I understand from the reply Poedit author gave you, the xgettext utility doesn't use paths relative to the location of the .po file being created/updated in the #: admin.py:12 comments. It uses paths relative to the path where it was invoked from.

makemessages isn't working in a way different from xgettext here (in fact under the hood it executes xgettext recursively starting from the path you executed by following the the Satchmo docs you linked and the Django documentation, the resulting comments in the .po file are the same you'd get if you ran xgettext by hand and then merged all the resulting catalogs) and, although what you want to achieve as a translator usability detail is an acceptable wish, IMHO it isn't worth breaking that compatibility to accommodate one translator tool or workflow particularity.

So I'd be inclined to say this isn't a Django bug and I'm closing it. Please reopen it if you find any error in my reasoning.

Workaround: If you want to be able to open admin.py (or, e.g. templatetags/satchmo_brands.py) from poedit you should execute it from the same directory you ran makemessages:

$ pwd
/home/user/src/satchmo/apps/satchmo_ext/brand

$ poedit locale/ro/LC_MESSAGES/django.po

(disclaimer: I haven't tried this myself, I'm assuming Poedit open these files using paths relative to its current working dir and not relative to the .po file path).

in reply to:  3 ; comment:4 by Cristian Ciupitu, 14 years ago

Replying to ramiro:

Replying to ciupicri:

Replying to ramiro:
The root of the application. Here for example (as instructed in the documentation for Satchmo).

Contrarily to what I understand from the reply Poedit author gave you, the xgettext utility doesn't use paths relative to the location of the .po file being created/updated in the #: admin.py:12 comments. It uses paths relative to the path where it was invoked from.

I think that xgettext could be invoked from the directory of the PO file, but I'm not the Poedit author nor a (x)gettext expert. I've just started with this translation thing.

makemessages isn't working in a way different from xgettext here (in fact under the hood it executes xgettext recursively starting from the path you executed by following the the Satchmo docs you linked and the Django documentation, the resulting comments in the .po file are the same you'd get if you ran xgettext by hand and then merged all the resulting catalogs) and, although what you want to achieve as a translator usability detail is an acceptable wish, IMHO it isn't worth breaking that compatibility to accommodate one translator tool or workflow particularity.

Break compatibility with what? As far as I know these are just comments.

So I'd be inclined to say this isn't a Django bug and I'm closing it. Please reopen it if you find any error in my reasoning.

In this case what other translation tools would you recommend? Also, what about transforming it into a request for enhancement for 1.3 if the compatibility thing is such a deal breaker?

Workaround: If you want to be able to open admin.py (or, e.g. templatetags/satchmo_brands.py) from poedit you should execute it from the same directory you ran makemessages:

$ pwd
/home/user/src/satchmo/apps/satchmo_ext/brand

$ poedit locale/ro/LC_MESSAGES/django.po

(disclaimer: I haven't tried this myself, I'm assuming Poedit open these files using paths relative to its current working dir and not relative to the .po file path).

I've already tested this workaround before reporting the bug and it doesn't work. The paths are relative to the .po file (as expected at least by me).

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

Replying to ciupicri:

Break compatibility with what? As far as I know these are just comments.

Not free form comments, as indicated bi the leading ': '. See http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files

I can't find any formal specification of the for of .po[t] files but the document I linked above (official GNU getext manual) shows an example containing a '#: lib/error.c:116' program source code reference comment.

In this case what other translation tools would you recommend?

See http://www.gnu.org/software/gettext/manual/gettext.html#Editing

in reply to:  5 comment:6 by Cristian Ciupitu, 14 years ago

Resolution: invalid
Status: closedreopened

Replying to ramiro:

Replying to ciupicri:

Break compatibility with what? As far as I know these are just comments.

Not free form comments, as indicated bi the leading ': '. See http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files

I can't find any formal specification of the for of .po[t] files but the document I linked above (official GNU getext manual) shows an example containing a '#: lib/error.c:116' program source code reference comment.

I was only trying to say that these paths are not mandatory and even if they're "broken", the catalogs can still be compiled and work fine. They're just hints.

In this case what other translation tools would you recommend?

See http://www.gnu.org/software/gettext/manual/gettext.html#Editing

I tried Emacs and it failed to show the program sources. When I used the paths suggested here, it worked fine.
I also tried GTranslator with the same result.
I would have tried the new KBabel - Lokalize - too, but it can not show the program source right now.

by Cristian Ciupitu, 14 years ago

Attachment: makemessages.py.patch added

Patch for "makemessages.py"

comment:7 by Cristian Ciupitu, 14 years ago

It seems that os.path.rel is new in Python 2.6, so the last patch will need a backport of this function.

in reply to:  3 ; comment:8 by vslavik@…, 14 years ago

Cc: vslavik@… added

Replying to ramiro:

Contrarily to what I understand from the reply Poedit author gave you, the xgettext utility doesn't use paths relative to the location of the .po file being created/updated in the #: admin.py:12 comments. It uses paths relative to the path where it was invoked from.

(Poedit author here.) That's how I meant it, sorry for being unclear. It is a de-facto standard (see Automake and/or libintl makefiles) to invoke xgettext from the PO files directory. As indicated in a comment above, Poedit isn't the only tool that assumes this. When you think about it, it's the only way to express source code references that would be self-contained and would work without some additional externally provided information.

makemessages isn't working in a way different from xgettext here

Is it possible to invoke it from the PO files directory?

Even if it is, it would IMHO make a lot of sense to use the attached patch, because PO files location is well-known in Django and unlike with direct use of xgettext, it's not (I think) common to change into a specific directory when using django-admin.

in reply to:  8 comment:9 by Ramiro Morales, 14 years ago

Patch needs improvement: set

Replying to vslavik@fastmail.fm:

Replying to ramiro:

Contrarily to what I understand from the reply Poedit author gave you, the xgettext utility doesn't use paths relative to the location of the .po file being created/updated in the #: admin.py:12 comments. It uses paths relative to the path where it was invoked from.

(Poedit author here.) That's how I meant it, sorry for being unclear. It is a de-facto standard (see Automake and/or libintl makefiles) to invoke xgettext from the PO files directory. As indicated in a comment above, Poedit isn't the only tool that assumes this. When you think about it, it's the only way to express source code references that would be self-contained and would work without some additional externally provided information.

makemessages isn't working in a way different from xgettext here

Is it possible to invoke it from the PO files directory?

No, makemssages is a tool that is run in a project or application scope to collect translatable literal from several source file (and btw what I meant is that when it is runs if searchs recursively for the files candidate to extract translatable literals and the runs xgettext on them).

I won't oppose to a change being introduced for this ticket but I'm still rather unconvinced it is the correct thing to do. A few more examples of other projects:

all contain source code file reference comments relative to the root of the project tree (the latter two are traditional Python-based, and, like Django, use their own custom Python i18n toolchains.

I'm also marking it needs better patch because the os.path.relpath() available obly on Python >= 2.6 issue I pointed to the OP on #django-dev.

by Cristian Ciupitu, 14 years ago

Attachment: makemessages.py.diff added

Patch for "makemessages.py"

comment:10 by Cristian Ciupitu, 14 years ago

I've attached a new patch that works on older versions of Python, too. It's not pretty, but it works and it fixes a bug of the old patch.

comment:11 by Cristian Ciupitu, 14 years ago

Has patch: set
Patch needs improvement: unset

comment:12 by Cristian Ciupitu, 14 years ago

I tried to translate gettext and Mailman using Emacs and GTranslator, but it didn't work just like in the case of Django.

comment:13 by Russell Keith-Magee, 13 years ago

Triage Stage: UnreviewedDesign decision needed

I'm with Ramiro on this; Django is doing the same thing with it's PO files as many other tools. This is the first time that I recall someone reporting problems because of the annotations in Django's PO files.

My inclination is to mark this wontfix, but I'm not sufficiently familiar with translation issues to be certain, so I'm move it to DDN for the moment.

comment:14 by Cristian Ciupitu, 13 years ago

The change might be major, but the issue is pretty straightforward: the PO files generated by Django don't work 100% with 3 translation tools: Emacs, Poedit and GTranslator.

comment:15 by Graham King, 13 years ago

Severity: Normal
Type: Bug

comment:16 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:17 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:18 by Aymeric Augustin, 11 years ago

Status: reopenednew

comment:19 by Claude Paroz, 11 years ago

Patch needs improvement: set
Triage Stage: Design decision neededAccepted

If we can make source paths relative to the po directory, I think we should do it.

comment:20 by vslavik@…, 11 years ago

FIY, Poedit will handle this case correctly starting with 1.5.6 release. Sorry it took so long to fix on my side.

comment:21 by Tobias McNulty, 8 years ago

Resolution: needsinfo
Status: newclosed

The original case has been fixed outside Django and this hasn't been touched in 3 years. If this is still an issue for you, please re-open with current steps to reproduce.

Another consideration is that many tools have probably, like Poedit, adapted to Django's existing source code path convention (along with other projects that use the same convention). Changing it now would be a backwards-incompatible change that may cause more confusion than its worth.

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