Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#14924 closed (fixed)

I18N looks for translations in the reverse order of the apps

Reported by: Klaas van Schelven Owned by: nobody
Component: Internationalization Version: 1.2
Severity: 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

I18N should look for translation strings in the order of apps as given in INSTALLED_APPS. Translations in apps that appear earlier in that list should be used rather than translations later in that list in case there are conflicts.

As described in #14910 as "point 2". I'm reproducing the important parts here:

Django's default template loader is documented here:

http://docs.djangoproject.com/en/dev/ref/settings/?from=olddocs#template-loaders

('django.template.loaders.filesystem.Loader',
 'django.template.loaders.app_directories.Loader')

Having looked at the original tests the most desirable order appears to be:

  1. Project path locale
  2. Apps (in correct order)
  3. Locale-path
  4. Django's own translations

The attached patch is a reflection of

  1. The order mentioned above
  2. Non-destructive dictionary updates
  3. I had to move the monkey-patching code into the _merge function, because the first actually found locale is no longer known in advance.

Attachments (8)

with_test.patch (5.3 KB ) - added by Klaas van Schelven 13 years ago.
Patch with test.
docs.patch (2.0 KB ) - added by Klaas van Schelven 13 years ago.
Docs patch for the current documentation
docs.after.14910.patch (1.0 KB ) - added by Klaas van Schelven 13 years ago.
The docs patch to apply if #14910 is fixed
with_tests_2.patch (5.3 KB ) - added by Klaas van Schelven 13 years ago.
Patch file syntax correct
django.mo (89 bytes ) - added by Klaas van Schelven 13 years ago.
.mo that goes with the .po (part of the patch)
14924.1.diff (13.7 KB ) - added by Ramiro Morales 13 years ago.
Patch with modification of loading precedence as proposed by vanschelven + tests + docs mods. Remeber to regenerate the .mo files correspondig to the touche .po files
14924.2.diff (20.5 KB ) - added by Ramiro Morales 13 years ago.
New version of the patch, also includes fix for #10765. start deprecation path for loading of translations from the 'project' directory.. remeber to generate the .mo files under the tests/ subdir for the three modified .po files
14924.3.diff (26.7 KB ) - added by Ramiro Morales 13 years ago.
New patch: Removes count trickery in PendingDeprecationWarning code, expands documentation, adds release note blurbs.

Download all attachments as: .zip

Change History (19)

by Klaas van Schelven, 13 years ago

Attachment: with_test.patch added

Patch with test.

by Klaas van Schelven, 13 years ago

Attachment: docs.patch added

Docs patch for the current documentation

by Klaas van Schelven, 13 years ago

Attachment: docs.after.14910.patch added

The docs patch to apply if #14910 is fixed

by Klaas van Schelven, 13 years ago

Attachment: with_tests_2.patch added

Patch file syntax correct

by Klaas van Schelven, 13 years ago

Attachment: django.mo added

.mo that goes with the .po (part of the patch)

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

Triage Stage: UnreviewedAccepted

in reply to:  description comment:2 by Klaas van Schelven, 13 years ago

I'd like to have some discussion on my own comment below:

Having looked at the original tests the most desirable order appears to be:

  1. Project path locale
  2. Apps (in correct order)
  3. Locale-path
  4. Django's own translations

I've got the above patch running in some production code now, and I'm feeling this is not the most logical order. (The order was originally established by looking at the existing tests and code, and matching that order as closely as possible while making sure the apps are evaluated in the correct, not reverse, order).

In my mind, locale path should be the first to be evaluated. The reason someone would put up a specific path for locales pretty much has to be to override the existing behavior (which would be found in project and apps).

Input (discussion) on this is much appreciated. I'll gladly put in a new patch once a decision is reached. If this is a separate issue, I'm also fine on opening a separate ticket. (it could be argued that we can move forward on the original bug report as a separate issue more easily, since it's "obviously broken").

comment:3 by Ramiro Morales, 13 years ago

Component: UncategorizedInternationalization

by Ramiro Morales, 13 years ago

Attachment: 14924.1.diff added

Patch with modification of loading precedence as proposed by vanschelven + tests + docs mods. Remeber to regenerate the .mo files correspondig to the touche .po files

comment:4 by Ramiro Morales, 13 years ago

Has patch: set

The 14924.1.diff patch implements the modifcations discussed here and in the mailing list thread. Namely, to modify the precedence of loading of translations from compiled message files from the current situation (from higher to lesser):

  • The translation found in locale/ subdir of the the project dir.
  • The translations found in the apps listed in settings.INSTALLED_APPS with the latter ones having higher precedence.
  • The translations found in the paths listed in settings.LOCALE_PATHS with the latter ones having higher precedence.
  • The translations shipped with Django.

To (also from higher to lesser precedence):

  • The translations found in the paths listed in settings.LOCALE_PATHS with the ones located first having higher precedence.
  • The translation found in locale/ subdir of the the project dir.
  • The translations found in the apps listed in settings.INSTALLED_APPS with the ones located first having higher precedence.
  • The translations shipped with Django.

As proposed by vanschelven.

Tests and docs were modified accordingly. Some of these tests/docs mods are tweaks that can be extracted and applied even if this ticket isn't accepted or committing of the patch is deferred.

by Ramiro Morales, 13 years ago

Attachment: 14924.2.diff added

New version of the patch, also includes fix for #10765. start deprecation path for loading of translations from the 'project' directory.. remeber to generate the .mo files under the tests/ subdir for the three modified .po files

comment:5 by Ramiro Morales, 13 years ago

14924.2.diff (ignore the #10765 reference in the ticket description , should be #5494) contains a further evolution to the patch that:

  • Extends the changes to include building of JavaScript catalog in the javascript_catalog() view making it more consistent with the one used for Python/templates code translations (paths listed in LOCALE_PATHS is taken in account.)
  • As per discussion with Jannis and Claude Paroz, starts the deprecation path for inclusion of translations located under the project path. The LOCALE_PATH setting can be used for the same task by listing the filesystem path to the project level translations. Reationale for this:
    • The project path has always been an elusive concept (it is actually the directory containing the settings) and there has been a shift in other parts of the framework from using it as a reference for location of assets at runtime.
    • It fails when the settings module is a directory (#10765)
    • The project_dir/locale/ subdir can generate spurious error messages when the project directory is included in the python path (default behavior of manage.py runserver), e.g.: /usr/lib/python2.6/gettext.py:49: ImportWarning: Not importing directory '/path/to/project/dir/locale': missing __init__.py. See also http://groups.google.com/group/django-users/browse_thread/thread/e8bb9089d9e5be60?pli=1

comment:6 by Jannis Leidel, 13 years ago

milestone: 1.3
Triage Stage: AcceptedReady for checkin

by Ramiro Morales, 13 years ago

Attachment: 14924.3.diff added

New patch: Removes count trickery in PendingDeprecationWarning code, expands documentation, adds release note blurbs.

comment:7 by Ramiro Morales, 13 years ago

Resolution: fixed
Status: newclosed

In [15441]:

Fixed #5494, #10765, #14924 -- Modified the order in which translations are read when composing the final translation to offer at runtime.

This is slightly backward-incompatible (could result in changed final translations for literals appearing multiple times in different .po files but with different translations).

Translations are now read in the following order (from lower to higher priority):

For the 'django' gettext domain:

  • Django translations
  • INSTALLED_APPS apps translations (with the ones listed first having higher priority)
  • settings/project path translations (deprecated, see below)
  • LOCALE_PATHS translations (with the ones listed first having higher priority)

For the 'djangojs' gettext domain:

  • Python modules whose names are passed to the javascript_catalog view
  • LOCALE_PATHS translations (with the ones listed first having higher priority, previously they weren't included)

Also, automatic loading of translations from the 'locale' subdir of the settings/project path is now deprecated.

Thanks to vanschelven, vbmendes and an anonymous user for reporting issues, to vanschelven, Claude Paroz and an anonymous contributor for their initial work on fixes and to Jannis Leidel and Claude for review and discussion.

comment:30 by Ramiro Morales, 13 years ago

In [15513]:

Added a test for the PendingDeprecationWarning introduced in r15441. Refs #14924, #15286.

comment:3 by Ramiro Morales, 13 years ago

In [15514]:

Don't merge in translations twice from deprecated project level tree when it is also listed in LOCALE_PATHS. Thanks Claude Paroz. Refs #14924.
Also, removed some old unused variables as reported by pyflakes.

comment:4 by Jannis Leidel, 13 years ago

In [15529]:

Normalize the locale paths when considering merging a project language catalogue. Refs #14924.

comment:5 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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