Opened 13 years ago
Closed 7 years ago
#17379 closed Cleanup/optimization (fixed)
Don't deactivate translations by default in management commands
Reported by: | Chris Chambers | Owned by: | Ramiro Morales |
---|---|---|---|
Component: | Internationalization | Version: | dev |
Severity: | Normal | Keywords: | management, shell, i18n, l10n, en-us, translation, documentation |
Cc: | lpiatek, neaf | 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
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:
- forcing
translation.activate(settings.LANGUAGE_CODE)
during the./manage.py shell
command. - Adding some visual feedback/documentation if this turns out to be impractical for whatever reason.
Attachments (4)
Change History (28)
comment:1 by , 13 years ago
Type: | Uncategorized → Bug |
---|
comment:3 by , 13 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Triage Stage: | Unreviewed → Accepted |
Version: | 1.3 → SVN |
Attached patch with a possible approach to the problem.
by , 13 years ago
Attachment: | 17379-2.diff added |
---|
comment:4 by , 13 years ago
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 by , 13 years ago
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.
by , 13 years ago
Attachment: | 17379-3.diff added |
---|
decouple can_import_settings from force_en_us_locale
comment:6 by , 13 years ago
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 by , 13 years ago
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 by , 13 years ago
Patch needs improvement: | unset |
---|
comment:9 by , 13 years ago
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.
comment:10 by , 13 years ago
Cc: | added |
---|
comment:11 by , 13 years ago
Cc: | added |
---|
comment:12 by , 13 years ago
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 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:14 by , 12 years ago
Summary: | Silent translation of all ./manage.py commands to en-us unexpected/undocumented → 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 by , 12 years ago
#19737 proposes deprecating the shell command. If accepted, that would turn this ticket irrelevant.
comment:16 by , 12 years ago
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 by , 11 years ago
Easy pickings: | unset |
---|
comment:18 by , 10 years ago
Summary: | Don't force output of the 'shell' management command to en-us → Don't deactivate translations by default in management commands |
---|
comment:19 by , 7 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
I updated my branch and made it a PR.
comment:20 by , 7 years ago
Needs tests: | set |
---|
comment:21 by , 7 years ago
Patch needs improvement: | set |
---|
comment:22 by , 7 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:23 by , 7 years ago
Triage Stage: | Accepted → Ready for checkin |
---|---|
Type: | Bug → Cleanup/optimization |
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).