Opened 7 years ago

Closed 4 years ago

Last modified 3 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
Component: Internationalization Version: master
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: UI/UX:

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 msaelices 7 years ago.
A patch that fixes bug
root-locale-js-catalog.diff (1.1 KB) - added by trevor@… 7 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 msaelices 7 years ago.
A new patch thats not assumes settings is in projectdir
js_catalog_r9076.diff (2.3 KB) - added by oggy 6 years ago.
i18n.diff (495 bytes) - added by djangobugs@… 4 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 claudep 4 years ago.
Same as i18n.diff + tests
i18n_and_tests2.diff (1.6 KB) - added by claudep 4 years ago.
Test in views instead of i18n

Download all attachments as: .zip

Change History (37)

comment:1 Changed 7 years ago by msaelices

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to msaelices
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 7 years ago by msaelices

I'll try to fix it

comment:3 Changed 7 years ago by msaelices

  • Triage Stage changed from Unreviewed to Accepted

Changed 7 years ago by msaelices

A patch that fixes bug

comment:4 Changed 7 years ago by msaelices

  • Has patch set

comment:5 Changed 7 years ago by msaelices

  • Triage Stage changed from Accepted to Ready for checkin

Changed 7 years ago by trevor@…

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

comment:6 Changed 7 years ago by trevor@…

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 follow-up: Changed 7 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

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.

comment:8 in reply to: ↑ 7 Changed 7 years ago by prairiedogg

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

Changed 7 years ago by msaelices

A new patch thats not assumes settings is in projectdir

comment:9 Changed 7 years ago by msaelices

  • Owner changed from msaelices to mtredinnick
  • Status changed from assigned to new

@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 Changed 7 years ago by mtredinnick

  • Owner mtredinnick deleted

Changed 6 years ago by oggy

comment:11 Changed 6 years ago by oggy

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 Changed 5 years ago by robin

  • Cc robin added

comment:13 Changed 5 years ago by stodge

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 Changed 5 years ago by stodge

  • Cc stodge@… added

comment:15 Changed 5 years ago by jweyrich

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

comment:16 Changed 5 years ago by EmilStenstrom

  • Patch needs improvement unset

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

comment:17 Changed 5 years ago by stodge

Is this a patch for 1.1.x or 1.2?

comment:18 Changed 5 years ago by claudep

  • 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 Changed 4 years ago by stodge

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 Changed 4 years ago by claudep

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 Changed 4 years ago by redliner

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 Changed 4 years ago by jezdez

  • Needs tests set

Changed 4 years ago by djangobugs@…

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 Changed 4 years ago by anonymous

  • 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 Changed 4 years ago by jweyrich

  • Cc jweyrich@… added

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

Changed 4 years ago by claudep

Same as i18n.diff + tests

comment:25 Changed 4 years ago by claudep

  • milestone set to 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 Changed 4 years ago by jezdez

  • Needs documentation set

comment:27 Changed 4 years ago by jezdez

  • 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.

Changed 4 years ago by claudep

Test in views instead of i18n

comment:28 Changed 4 years ago by claudep

  • Patch needs improvement unset

comment:29 Changed 4 years ago by ramiro

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

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 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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