Opened 17 years ago

Closed 14 years ago

Last modified 13 years ago

#5494 closed (fixed)

javascript_catalog generic view only search catalogs in application/locale and not in project/locale

Reported by: anonymous Owned by: Ramiro Morales
Component: Internationalization Version: dev
Severity: Keywords: javascript i18n
Cc: Robin, stodge@…, claude@…, djangobugs@…, jweyrich@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In i18n docs said that message files are searched in (fragment copied from i18n.txt):

    * The root ``django`` directory (not a Subversion checkout, but the one
      that is linked-to via ``$PYTHONPATH`` or is located somewhere on that
      path).
    * The root directory of your Django project.
    * The root directory of your Django app.

But it doesn't work for javascript catalogs. If you have a project named blogproj and an aplication named blog , you cannot put javascript catalogs in blogproj/locale . This urls.py doesn't work:

js_info_dict = {
    'domain': 'djangojs',
    'packages': ('blogproj',),
}

urlpatterns = patterns('',
    (r'^jsi18n/$', 'django.views.i18n.javascript_catalog', js_info_dict),

But if you have message files in blogproject/blog/locale , it works with this urls.py :

js_info_dict = {
    'domain': 'djangojs',
    'packages': ('blogproj.blog',),
}

urlpatterns = patterns('',
    (r'^jsi18n/$', 'django.views.i18n.javascript_catalog', js_info_dict),

Attachments (7)

jscatalog.diff (1.2 KB ) - added by Manuel Saelices 17 years ago.
A patch that fixes bug
root-locale-js-catalog.diff (1.1 KB ) - added by trevor@… 17 years ago.
Patch that supplies the option of specifying an empty string as a package for js l10n.
jscatalog_r8521.diff (735 bytes ) - added by Manuel Saelices 16 years ago.
A new patch thats not assumes settings is in projectdir
js_catalog_r9076.diff (2.3 KB ) - added by oggy 16 years ago.
i18n.diff (495 bytes ) - added by djangobugs@… 14 years ago.
Allow django javascript catalog to discover translation files as documented here: http://docs.djangoproject.com/en/dev/topics/i18n/deployment/#how-django-discovers-translations
i18n_and_tests.diff (2.6 KB ) - added by Claude Paroz 14 years ago.
Same as i18n.diff + tests
i18n_and_tests2.diff (1.6 KB ) - added by Claude Paroz 14 years ago.
Test in views instead of i18n

Download all attachments as: .zip

Change History (37)

comment:1 by Manuel Saelices, 17 years ago

Owner: changed from nobody to Manuel Saelices
Status: newassigned

comment:2 by Manuel Saelices, 17 years ago

I'll try to fix it

comment:3 by Manuel Saelices, 17 years ago

Triage Stage: UnreviewedAccepted

by Manuel Saelices, 17 years ago

Attachment: jscatalog.diff added

A patch that fixes bug

comment:4 by Manuel Saelices, 17 years ago

Has patch: set

comment:5 by Manuel Saelices, 17 years ago

Triage Stage: AcceptedReady for checkin

by trevor@…, 17 years ago

Attachment: root-locale-js-catalog.diff added

Patch that supplies the option of specifying an empty string as a package for js l10n.

comment:6 by trevor@…, 17 years ago

The regular localization has the feature of not having to specify the project name. Of course, the javascript localization has to obey the constraint that only requested message catalogs are made available, so I propose a special package name "" (empty string) that refers to the root locale dir, as in

    globalpath = os.path.join(os.path.dirname(sys.modules[settings.__module__].__file__), 'locale')

from the regular translation function (django gettext domain). Thus, one could specify a url like this:

    url(r'^jsi18n/$', javascript_catalog, {'packages': ('', 'apps.blog')}, name='jsi18n'),

This would enable the root catalog in addition to the apps.blog catalog. I've attached a patch against trunk revision 6739 to enable this functionality.

comment:7 by Malcolm Tredinnick, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

I think there are problems with both of these solutions. Manuel's one assumes the settings file is in the project directory, which is not really a good assumption. Trevor's solution introduces a string that isn't very Pythonic (there isn't a lot of precedent for the empty string meaning "here" in Python imports).

Let's go for a simple solution here. We're getting into trouble because the i18n framework is trying to enforce the fact that things have to be in INSTALLED_APPS or one of a few special cases. Instead, let's allow any module to specified in the values for the package key, providing that module contains a locale/ subdirectory.

The example given, then becomes

{'packages': ('blogproj', 'blogproj.blog')}

Yes, this means you could potentially include an entirely random translation file. That's relatively harmless and might even make things easier (by imposing less constraints) in some cases.

in reply to:  7 comment:8 by prairiedogg, 16 years ago

Just FYI to whoever undertakes writing this patch, there's a little thread on the django-users group describing a problem with placing your locale directory in the project directory if it is already on the $PYTHONPATH. If folks having their local directories in their project root is recommended, then the patch might want to address this as well, but I'm no expert - just wanted to toss it out there.

Link to discussion thread in django-users: http://groups.google.com/group/django-users/browse_thread/thread/e8bb9089d9e5be60

by Manuel Saelices, 16 years ago

Attachment: jscatalog_r8521.diff added

A new patch thats not assumes settings is in projectdir

comment:9 by Manuel Saelices, 16 years ago

Owner: changed from Manuel Saelices to Malcolm Tredinnick
Status: assignednew

@mtredinnick, I've just uploaded a new patch that not assumes settings file is in projectdir. The problem now is there is no settings for PROJECTDIR or something like this. Now the patch assumption is that if package doesn't contains a dot (i.e. 'blogproj'), we assume that is project dir.

Other thing is that is very difficult to put a test because we would have to create a locale dir just into regressiontests directory. I will reassign you this ticket. If you want to create a test creating this directory, I will do it (reassign me ticket again, please).

comment:10 by Malcolm Tredinnick, 16 years ago

Owner: Malcolm Tredinnick removed

by oggy, 16 years ago

Attachment: js_catalog_r9076.diff added

comment:11 by oggy, 16 years ago

I've created a new patch which:

  1. simply removes all checks of the 'packages' parameter
  2. improves efficency and DRYness of this module

Malcolm, if I understand your objection about Manuel's first patch correctly, the problem is that settings.py could basically be anywhere, and not in the project dir? And heck, there doesn't even need to be a "project dir". I'm asking because this method of determining it appear in several other places in Django (notably django.utils.translation.trans_real), but I guess that there's no better way to determine it programatically so you generally have to rely on people doing the sane thing.

But here we can make an exception because, well, we're once again relying on people to do the sane thing and pass us the correct module names. Correct?

comment:12 by Robin, 15 years ago

Cc: Robin added

comment:13 by Mike, 14 years ago

I *really* hate (and I apologise) to bring this ticket back to the surface, but is this broken again in the latest 1.1.x? I've followed all of the docs but I can't get translation strings in project/locale into the javascript catalog? Thanks

comment:14 by Mike, 14 years ago

Cc: stodge@… added

comment:15 by Jardel Weyrich, 14 years ago

Is someone working on it? A partial fix is welcome IMO.

comment:16 by EmilStenstrom, 14 years ago

Patch needs improvement: unset

Removing "Patch needs improvement" since there's a new patch waiting for review.

comment:17 by Mike, 14 years ago

Is this a patch for 1.1.x or 1.2?

comment:18 by Claude Paroz, 14 years ago

Cc: claude@… added

Would be great to address this issue for 1.3. What about using trevor's patch and replacing the empty string by "."?

comment:19 by Mike, 14 years ago

Is there a known workaround for this? This is a blocker for my application - even a working, usable fudge would be ok for now.

comment:20 by Claude Paroz, 14 years ago

In my particular case, I added a symbolic link in the app directory pointing to the project locale dir (myproj/myapp/locale -> ../locale). I don't know if this is scalable to other type of projects.

comment:21 by redliner, 14 years ago

Another "not so dirthy" workaround is to add 'conf' to your INSTALLED_APPS and then pretend it's an app.
And link to it:

js_info_dict = {
    'domain': 'djangojs',
    'packages': ('conf',),
}

This works for me but it's not final solution and probably has some disadvantages too ...
On the other hand one doesn't have to patch Django to use this view.

comment:22 by Jannis Leidel, 14 years ago

Needs tests: set

by djangobugs@…, 14 years ago

Attachment: i18n.diff added

Allow django javascript catalog to discover translation files as documented here: http://docs.djangoproject.com/en/dev/topics/i18n/deployment/#how-django-discovers-translations

comment:23 by anonymous, 14 years ago

Cc: djangobugs@… added

I've added a new diff, i18n.diff. While the previous patches provide for some workarounds, all of them require that you pass a non-package placeholder as a package argument, which doesn't seem to be very intuitive. Since we want the javascript catalog to read from our project's default locale location, and not from a particular package, we just need to NOT pass any package arguments and thus bypass all of the package verification that is causing the problem. Instead, we just have the javacript_catalog function add the project's setting.LOCALE_PATHS to the "paths" variable.

So in summary, the problem is simply that javascript_catalog isn't honoring a supported setting, setting.LOCALE_PATHS, which is a tuple of paths were you want Django to search for locale files. The setting should look something like this:

 settings.LOCALE_PATHS = ('/path/to/project/locale',)

and it is documented here: http://docs.djangoproject.com/en/dev/ref/settings/#locale-paths

To use the project's default locale location the urls.py entry should not pass any arguments to the view:

urlpatterns = patterns('',
    (r'^jsi18n/$', 'django.views.i18n.javascript_catalog'),
)

comment:24 by Jardel Weyrich, 14 years ago

Cc: jweyrich@… added

i18n.diff seems to tackle the problem while avoids unnecessary complexity. It's ready for check-in IMO.

by Claude Paroz, 14 years ago

Attachment: i18n_and_tests.diff added

Same as i18n.diff + tests

comment:25 by Claude Paroz, 14 years ago

milestone: 1.3
Needs tests: unset

I've added missing tests in i18n.diff patch. I think this is valuable in that it makes the javascript_catalog to honour LOCALE_PATH. This is not really resolving the issue that javascript is not taking project/locale into account automatically, but it adds at least a mean to add it manually.

comment:26 by Jannis Leidel, 14 years ago

Needs documentation: set

comment:27 by Jannis Leidel, 14 years ago

Patch needs improvement: set

The patch looks great generally, but the tests would be much better with the rest of the i18n views tests in regressiontests.views.tests.i18n.

by Claude Paroz, 14 years ago

Attachment: i18n_and_tests2.diff added

Test in views instead of i18n

comment:28 by Claude Paroz, 14 years ago

Patch needs improvement: unset

comment:29 by Ramiro Morales, 14 years ago

Owner: set to Ramiro Morales
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 Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

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