zerospam / laravel-gettext

Adds localization support to laravel applications using PoEdit and GNU-Gettext.
99 stars 56 forks source link

Symfony Translator doesn't use Session storage #2

Closed Anahkiasen closed 6 years ago

Anahkiasen commented 7 years ago

So every page the locale is forgotten. I think maybe this happened when storage was split to a separate service? Seems the Gettext service was made to use it but not the Symfony one, is that on purpose? Did some tests and seems the issue started with this commit

Belphemur commented 7 years ago

Hello @Anahkiasen,

Can you try with the 5.0.2 that I just released?

I've setup a clean project using the basic Auth of laravel and I have no issue with the 5.0.2, the locale is persisted in the session and used accordingly with the Symfony backend.

Anahkiasen commented 7 years ago

Still the same issue but different: doing some debugging it seems the locale is now properly set on the LaravelGettext and Translators\Symfony classes but not on the actual Symfony translator that is set on the latter.

That being said if I redo LaravelGettext::setLocale('en_US') and call _i('foobar') then I do get the translation. So it seems it's just when it sets the locale from session.

This is on a fresh Laravel project so there's no interference or any code from my end, this is literally all I have:

**routes/web.php** ```php use Xinax\LaravelGettext\Facades\LaravelGettext; Route::get('lang/{lang}', function ($lang) { LaravelGettext::setLocale($lang); return redirect()->back(); }); Route::get('/', function () { return view('welcome'); }); ``` **views/welcome.blade.php** ```php Document EN FR {{ _i('Déconnexion') }} ``` Middleware is there and all and it works from tinker: ```php $ laravel artisan tinker Psy Shell v0.8.3 (PHP 7.1.3 — cli) by Justin Hileman >>> _i('Déconnexion') => "Déconnexion" >>> app('laravel-gettext')->setLocale('en_US') => Xinax\LaravelGettext\LaravelGettext {#682} >>> _i('Déconnexion') => "Logout" ``` But not when clicking on "EN" on my test page
Belphemur commented 7 years ago

a locale is the language name and the country, not just the language name.

You should have en_US and fr_FR in the "lang selector".

Belphemur commented 7 years ago

You should use this snippet of code to do your language selector:

<ul>
      @foreach(Config::get('laravel-gettext.supported-locales') as $locale)
            <li><a href="/lang/{{$locale}}">{{$locale}}</a></li>
      @endforeach
</ul>
Anahkiasen commented 7 years ago

Hm that's weird, in the actual project I have the bug on the locale switchers are using the full locale name (en_US/fr_FR), must come from somewhere else, will continue to investigate.

Anahkiasen commented 7 years ago

Ok so the issue is I'm calling _i from somewhere too early in the call stack when the locale is not yet set from storage, more precisely in the routes/breadcrumbs.php file from laravel-breadcrumbs. What's weird is that this makes all translations fail and not just the breadcrumbs. While most classes have the correct locale on them, it seems the Symfony Translator instance on Symfony still has the default locale set on it, guessing because LaravelGettext is bound as a singleton so if you call it too early it'll be set incorrectly on the container for the rest of the request. But then I don't get it what is the purpose of the GettextMiddleware if it doesn't call setLocale?

Belphemur commented 7 years ago

In your list of providers, try to set LaravelGettext just after the providers of Laravel and before the one for the breadcrumbs.

It shouldn't be an issue, but since I have no idea how laravel-breadcrumbs works/register its services, it's hard to tell what's the issue between the two packages.

The laravel gettext middleware is only needed if you're using gettext as the translation engine as it has to set multiple system variables (like the syscall setlocale, etc ...) that are needed to ensure gettext is set correctly. When using with Symfony-translation, it's not mandatory as it doesn't rely on those syscalls.

If you're using the gettext provider, be sure to have the middleware loaded before any other middleware of other packages.

Anahkiasen commented 7 years ago

Oh ok since the package is also called Gettext I thought the middleware was mandatory in any case :D Good to know I can remove it.

But I mean I don't know enough about the internals of the package to know if what I'm describing is a bug or not – ie. if the package should be able to recover from being invoked too early. So since I solved the issue on my end by deferring getting the breadcrumbs translations, if you wanna close this it's fine by me.