TranslateBehaviour should check if locale is already set in I18n about cakephp HOT 7 CLOSED

5th Jan 2010, Mark Story said:

Why? What does this fix? What issues does it solve? Removing a function and replacing it with floating code seems like a poor idea to me.

from cakephp.

5th Jan 2010, NaMB said:

Sorry, you misread, I wasn't very clear, let me rephrase.

I think this portion of code in the _setLocale function :

            $I18n->l10n->get(Configure::read('Config.language'));

should be replaced with this :

            if (!$I18n->l10n->found)
                $I18n->l10n->get(Configure::read('Config.language'));

This allows to set the locale manually by calling l10n->get() prior to db calls, thereby overriding Config.language (which would remain as a fallback).

Amongst other advantages, this would alleviate the over-reliance on the config class to store the current language (which doesn't sound like the right place to store a runtime specific value, but that's a debate we can leave for another time).

from cakephp.

5th Jan 2010, Mark Story said:

Configure is the correct place to store runtime configuration. That's pretty much its sole purpose, to store runtime configuration.

from cakephp.

5th Jan 2010, NaMB said:

Mmh, ok, now I'm thinking over this, I realize I'm using Config.language as a default language, which isn't right either I guess. So I guess there are actually two different issues here, each of which should probably be tackled as a whole.

1. The runtime-language is stored in two different places : in Config.language and in l10n. I think it should be centralized in l10n. Especially since l10n doesn't keep the two in sync
2. The config class should, on the other hand, be used to define a default language. Currently, this is done with the DEFAULT_LANGUAGE global. That should be changed to Config::read('Config.defaultLanguage');. As a sidenote, I see no used to check for it twice :

    function __construct() {
        if (defined('DEFAULT_LANGUAGE')) {
            $this->default = DEFAULT_LANGUAGE;
        }
        parent::__construct();
    }

and

    function __setLanguage($language = null) {
        $langKey = null;
        if ($language !== null && isset($this->__l10nMap[$language]) && isset($this->__l10nCatalog[$this->__l10nMap[$language]])) {
            $langKey = $this->__l10nMap[$language];
        } else if ($language !== null && isset($this->__l10nCatalog[$language])) {
            $langKey = $language;
        } else if (defined('DEFAULT_LANGUAGE')) {
            $langKey = DEFAULT_LANGUAGE;
        }

so that last bit should be changed to :

@@@php
} else if ($this->default) {
$langKey = $this->default;
}

Should I open separate tickets ?

from cakephp.

5th Jan 2010, NaMB said:

Configure is the correct place to store runtime configuration.
Except that I would say that, in an internationalized website, the language isn't runtime configuration at all, it's a dynamic value.

from cakephp.

Config.language can be saved in and read from the session, which is documented in the CookBook:
http://book.cakephp.org/2.0/en/core-libraries/internationalization-and-localization.html#localization-in-cakephp

class AppController extends Controller {
    public function beforeFilter() {
        if ($this->Session->check('Config.language')) {
            Configure::write('Config.language', $this->Session->read('Config.language'));
        }
    }
}

Also DEFAULT_LANGUAGE is deprecated:
http://book.cakephp.org/2.0/en/appendices/2-4-migration-guide.html#l10n

The constant DEFAULT_LANGUAGE has been deprecated in favor of Configure value Config.language.

That makes the ticket obsolete, right?

from cakephp.

I think so 👍

from cakephp.