BoltTranslate / labels

Bolt Labels extension - Translatable labels for Bolt
https://bolt.cm/
18 stars 12 forks source link

Extracting the language from uri #14

Closed phpetra closed 8 years ago

phpetra commented 8 years ago

The old code for extracting the language through the SERVER['HTTP_HOST'] when having the routing defined as /en/page/some-name was not working for me.

I don't think the path or request_uri is inside the HTTP_HOST var but (at least on nginx) it is inside $_SERVER['REQUEST_URI']. The above fix makes all my routes with the language prefix work.

I do think however that this needs to be tested in Apache (and possibly others) environments as well, because the SERVER variable might have a different name.

Please test in a different environment with multilingual routing as [described in the docs[(https://docs.bolt.cm/howto/building-multilingual-websites#defining-routes)

GwendolenLynch commented 8 years ago

This has taken a bit of digging to get things understood here… and I am not convinced this approach is correct… I have submitted #15 to clean up the extension, but I deliberately left this function (mostly) alone so you're free to keep going and I'll do what I can to help.

The conversation on IRC was basically

<gawainlynch> I am reading that the browser lang has priority, followed by GET query, 
followed by config option?
<Bopp> gawainlynch: i think… if the default lang is 'en' for example.. 
<Bopp> and you go to nl.example.org, or example.org?lang=nl, then it will set that as 
the language. 
<Bopp> so yes, what you said. 

So it looks as if the priority of the language should be something like:

  1. example.org?lang=nl would set the language to "NL"
  2. If the host name part of the FQDN, e.g. nl.example.org, matches one of the configured languages set the language to "NL"
  3. If a call to $request->getLocale() returns a language we support, e.g. nl_NL then we set the language to "NL"
  4. Use the value stored in $this->config['default']

A slight problem with the approach you mention is that /en/page/some-name is a route and that is an important distinction. My question is, do we want to be looking at the route and making a decision based on that?

I ask as it could get sticky quickly and we're probably better off sticking with GET query parameters… @bobdenotter ?

OK… So… We should not ever use PHP globals… people will have a bad time for one reason or another… "but it works on my system like this"… followed by a response like "But! Mine doesn't!" :grin:

That said, we probably want the before() function to look like this:

    public function before(Request $request)
    {
$lang = $request->query->get('lang');

        if (!$lang = $this->extractLanguage($lang)) {
            $lang = $this->extractLanguage($request->getHost());
        }

        if (!$lang = $this->extractLanguage($lang)) {
            $lang = $this->extractLanguage($request->getRequestUri());
        }

        if (!$this->isValidLanguage($lang)) {
            $lang = $request->getLocale();
        }

        if (!$this->isValidLanguage($lang)) {
            $lang = $this->config['default'];
        }

        $this->app['session']->set('lang', $lang);
        $this->setCurrentLanguage($lang);
    }

Note: See the change in https://github.com/bolt/labels/commit/e59135dd56749bd5d289babd477adb30105995c2 if you want to test this before that PR is merged.

EDIT Added a missing condition in the example.

phpetra commented 8 years ago

Guys, sorry I'm just seeing this now and will try to have a look (and update) tomorrow

@GawainLynch I agree completely with the flow above and not using the globals. From first glance it looks like it will work fine for me, but I'll test anyway ;-)

GwendolenLynch commented 8 years ago

Yeah… It's typed out and most certainly not tested… but yeah , let's get this little kitten purring! :grin:

phpetra commented 8 years ago

@GawainLynch This should be it now ;-) I hope. Tested it for all but the host part (but that part has not really changed)

phpetra commented 8 years ago

Glad you like it ;-)

GwendolenLynch commented 8 years ago

@bobdenotter This has several ticks of approval from me! Any objections your side?

bobdenotter commented 8 years ago

@GawainLynch Nope, no objections. @phpetra and I talked about this on slack this afternoon. I'm perfectly fine with this!