Opened 4 years ago

Last modified 4 months ago

#17379 assigned Bug

Don't deactivate translations by default in management commands

Reported by: chrischambers Owned by: ramiro
Component: Internationalization Version: master
Severity: Normal Keywords: management, shell, i18n, l10n, en-us, translation, documentation
Cc: lpiatek, neaf Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I see the rationale in django.management.core.base, but this is unexpected behaviour for ./manage.py shell, for example - particularly if you're running with a LANGUAGE_CODE like 'en-gb' as there's no visual feedback/documentation to illustrate that l10n/i18n are off. Granted, this won't make affect many people, but it can cause all kinds of debugging woes.

I'd like to suggest the simple fix of either:

  1. forcing translation.activate(settings.LANGUAGE_CODE) during the ./manage.py shell command.
  2. Adding some visual feedback/documentation if this turns out to be impractical for whatever reason.

Attachments (4)

17379-1.diff (1.8 KB) - added by claudep 4 years ago.
Add switch to commands to keep active language
17379-2.diff (2.0 KB) - added by lpiatek 3 years ago.
17379-3.diff (5.6 KB) - added by lpiatek 3 years ago.
decouple can_import_settings from force_en_us_locale
17379-4.diff (7.0 KB) - added by gabooo 3 years ago.
Fix i18n makemessages command

Download all attachments as: .zip

Change History (22)

comment:1 Changed 4 years ago by chrischambers

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Type changed from Uncategorized to Bug

comment:2 Changed 4 years ago by chrischambers

See also: https://code.djangoproject.com/ticket/11648, which might require the same fix (albeit for a different reason) if the underlying issue isn't/can't be corrected (https://code.djangoproject.com/ticket/9340).

Changed 4 years ago by claudep

Add switch to commands to keep active language

comment:3 Changed 4 years ago by claudep

  • Has patch set
  • Needs documentation set
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.3 to SVN

Attached patch with a possible approach to the problem.

Changed 3 years ago by lpiatek

comment:4 Changed 3 years ago by lpiatek

Comment on https://code.djangoproject.com/attachment/ticket/17379/17379-1.diff patch:

  • keep_active_language is not required, can_import_settings is only used for what we want to turn off. So maybe we do not need new flag?
  • printing sholdn't be done on stderr, but on stdout?
  • we changed information to: "Active language set to 'en-us'.\n"

Comment on whole:

  • there should be in BaseCommand Class functions like: _pre_execute(self), _post_execute(self) when such things like translations would be run, but this kind of change requires 'Design Decision', so we ommit it for now

#djangosprint-krakow

comment:5 Changed 3 years ago by claudep

  • Patch needs improvement set

Totally agree about changes related to stderr/stdout.

However I don't see how can_import_settings and keep_active_language are related. For me, those are two different unrelated flags, or am I missing something?

    ``can_import_settings``
        A boolean indicating whether the command needs to be able to
        import Django settings; if ``True``, ``execute()`` will verify
        that this is possible before proceeding. Default value is
        ``True``.

In some cases, we probably will have commands with can_import_settings = True, but where we don't want to switch to en-us.

Changed 3 years ago by lpiatek

decouple can_import_settings from force_en_us_locale

Changed 3 years ago by gabooo

Fix i18n makemessages command

comment:6 Changed 3 years ago by ramiro

I'd like to ask to the contributors working on this ticket what are the practical consequences (as in concrete use cases) for the 'shell'and 'makemessages' of being run always with the en-us locale.

It is not clear to me why both of them are particular among the rest of the commands and IMHO having that information would be helpful when it comes to solve/re-triage this ticket.

comment:7 Changed 3 years ago by neaf

The ticket requested shell to respect locale setting. Since it's an interface to your app there's no reason not to do that. You might want to see how your locale or other functionality that depends on i18n behaves.

can_import_settings was supposed to check if settings are available before execution of a command according to documentation (https://docs.djangoproject.com/en/dev/howto/custom-management-commands/#BaseCommand.can_import_settings). It was doing that only by coincidence… it was doing that by forcing en_us locale via translation module which relies on settings.

makemessages used that coincidental behavior to avoid forced locale and respect language set in your settings file.

The patch decouples can_import_settings from the locale forcing behavior to allow its proper usage as described in documentation.

makemessages implementation didn't change. The only difference is it now uses force_en_us_locale instead of can_import_settings to achieve the same thing.

comment:8 Changed 3 years ago by lpiatek

  • Patch needs improvement unset

comment:9 Changed 3 years ago by ramiro

  • Patch needs improvement set

Thanks for the explanation.

I'd like to:

  • Review if raising StandardError when force_en_us_locale = True and can_import_settings = False is the right thing to do
  • Decide if compilemessages (the only other one command that sets can_import_settings = False) does that because it needs to load settings or as a side effect of needing to (de)activate some locale(s), just as makemessages did.

Also needed is a change (addition) to the documentation.

Last edited 3 years ago by ramiro (previous) (diff)

comment:10 Changed 3 years ago by lpiatek

  • Cc lpiatek added

comment:11 Changed 3 years ago by neaf

  • Cc neaf added

comment:12 Changed 3 years ago by lpiatek

Raising StandardError when force_en_us_locale = True and can_import_settings = False:

  • for me it's wrong cause force_en_us_locale according to patch is set to True by default, so any command which now has can_import_settings = False would throw mentioned Exception. We just can't assume we need

Possible options:

  • set force_en_us_locale to False by default, and enable it where necessary (?), don't really know where and for what it is used (is it really necessary), we only have commen:
        # Switch to English, because django-admin.py creates database content
        # like permissions, and those shouldn't contain any translations.
        # But only do this if we can assume we have a working settings file,
        # because django.utils.translation requires settings.
  • don't see any other/better better

comment:13 Changed 2 years ago by ramiro

  • Owner changed from nobody to ramiro
  • Status changed from new to assigned

comment:14 Changed 2 years ago by ramiro

  • Summary changed from Silent translation of all ./manage.py commands to en-us unexpected/undocumented to Don't force output of the 'shell' management command to en-us

I've opened #19730 to track the unfolding of the meaning of the can_import_settings option.

I'm also changing the title of this one from "Silent translation of all ./manage.py commands to en-us unexpected/undocumented" "Don't force output of the 'shell' management command to en-us" because the fact that by default commands use the en_US locale isn't undocumented (see "Management commands and locales" note in https://docs.djangoproject.com/en/1.4/howto/custom-management-commands/) and hence it shouldn't be unexpected.

Let's discuss in this ticket if it's possible to the 'shell' command to use the locale configured by the developer for the project if USE_I18N is active.

comment:15 Changed 2 years ago by ramiro

#19737 proposes deprecating the shell command. If accepted, that would turn this ticket irrelevant.

comment:16 Changed 2 years ago by claudep

I really think that this isn't about the shell command only. We should only switch to a specific locale when it is required by the command. I worked on a branch (proof of concept, unfinished) demonstrating this here: https://github.com/claudep/django/compare/simplify_commands
I'm not totally convinced about the decorator pattern used, but you get the idea.

comment:17 Changed 22 months ago by timo

  • Easy pickings unset

comment:18 Changed 4 months ago by claudep

  • Summary changed from Don't force output of the 'shell' management command to en-us to Don't deactivate translations by default in management commands
Note: See TracTickets for help on using tickets.
Back to Top