fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Lang::load() loads multiple lang files #1820

Closed keech closed 9 years ago

keech commented 9 years ago

I have a question.

If I do below,

\Lang::load('fuga');

FuelPHP gets multiple lang files indicated by below two properties now.

Is this correct??

I think FuelPHP should check a file existing, which is "not" setted as "fallback_language". And only when it does not exist, a lang file setted as "fallback_language" should be loaded.

How do you think?

WanWizard commented 9 years ago

No, this is by design.

You should define your default or native language code as fallback. This is the one you develop in, and it should always be complete.

Your language is then either the same as the fallback, or different. If different, the two language files are merged to create the complete language file.

Reasoning behind this is that translated files could be incomplete or out of date (might happen in a distributed environment) or simply not there (in case of third party modules for example).

This implementation will then show the fallback language instead (i.e. the page appears untranslated), your suggestion will result in an empty page (all language strings missing).

keech commented 9 years ago

Thanks for your comment.

I've checked and comfirmed what you said.

    public function load($overwrite = false)
    {
        $paths = $this->find_file();

        $lang = array();

        foreach ($paths as $path)
        {
            $lang = $overwrite ?
                array_merge($lang, $this->load_file($path)) :
                \Arr::merge($lang, $this->load_file($path));
        }

        return $lang;
    }

But I found out there was another question. Is there any rules that we mustn't set a numeric keys in lang files? If we set a numeric keys in lang files, the lang parameters will not be merged.

namespace Fuel\Core;

class Arr
{
    public static function merge()
    {
        $array  = func_get_arg(0);
        $arrays = array_slice(func_get_args(), 1);

        if ( ! is_array($array))
        {
            throw new \InvalidArgumentException('Arr::merge() - all arguments must be arrays.');
        }

        foreach ($arrays as $arr)
        {
            if ( ! is_array($arr))
            {
                throw new \InvalidArgumentException('Arr::merge() - all arguments must be arrays.');
            }

            foreach ($arr as $k => $v)
            {
                // numeric keys are appended
                if (is_int($k))
                {
                    array_key_exists($k, $array) ? $array[] = $v : $array[$k] = $v;
                }
                elseif (is_array($v) and array_key_exists($k, $array) and is_array($array[$k]))
                {
                    $array[$k] = static::merge($array[$k], $v);
                }
                else
                {
                    $array[$k] = $v;
                }
            }
        }

        return $array;
    }
}
WanWizard commented 9 years ago

That is correct, it's the way Arr::merge() works.

Besides that, lang files are defined as assoc arrays, it is not designed to have numbers are lang keys.

keech commented 9 years ago

Oh, Sorry, I didn't know that rule.

Thank you for your help :)

WanWizard commented 9 years ago

Note really a rule, more a design decision.

It's not very logical to use Lang::get(723620); in your code, Lang::get('customer_name'); is more descriptive.

keech commented 9 years ago

But why you don't admit merging if keys are numeric in lang files?

I wonder there are cases people want to indicate keys setted numeric. e.g. managing keys as numeric constants.

WanWizard commented 9 years ago

You're the first in 5 years complaining about that, so I guess the answer is no. ;-)

As said, it is how Arr::merge() works. If you want to use array_merge() instead, you can use the $overwrite flag of the load() method. But that doesn't do recursive merging, so it has serious side effects.

Also, as said, the design decision was made, since it is not very useful to have numeric indexes in your lang definitions, not at the top level (as in my example above), and not lower down, like this.is.a.key.752. Using keys like this will make your app a nightmare to maintain in the future.