netson / l4gettext

Package to add Gettext support to Laravel 4 applications
61 stars 8 forks source link

Function setlocale() on Windows #3

Closed rhulshof closed 10 years ago

rhulshof commented 11 years ago

We love your package! However, on our development team several members develop on Windows clients. Up until now this hasn't been a problem. However, setlocale() works differently on Windows clients than on Unix-based systems, causing the LocaleNotFoundException to be thrown (which is uncatchable from within the L4 app, unfortunately).

We have managed to use GetText and PoEdit on previous projects on Windows environments, but this package refuses to work properly on Windows.

https://github.com/netson/l4gettext/blob/master/src/Netson/L4gettext/L4gettext.php#L107-L109

Windows setlocale(E_ALL, ...) refuses to accept locales like "en_US.utf8" and "en_US". However, "English", "German", "Dutch", etc. seem to work perfectly fine.

Is Windows support under consideration?

netson commented 11 years ago

Have you tried using the l4gettext:fetch command to generate a list of locales installed on your current system? If the definition of the locale is the only problem, you should be able to fix it by simply changing the supported locales/encodings in your config file.

However, I would expect some issues with the command line functions on a windows system. This is something that is on the list to be changed (switching from my own l4shell package to the default Symfony Process package), but I haven't gotten around to doing that yet.

Also, I just did some googling and can't find any info on how to list the installed locales/encodings on a windows system (from the command line). Do you know if this is possible? If not, I will not be able to port all functionality to windows, which makes me wonder if I should invest the effort.

Would you be willing to do some testing on windows after I make some changes? I don't have access to a windows test system, so any help here is much appreciated! Any tested pull requests you may have are also more than welcome!

netson commented 11 years ago

I just committed some changes which should improve usability on Windows. I have removed the requirement for the L4shell package and replaced it with the Symfony Process component (included with Laravel by default). This should allow for better Windows support. However, the fetch command will throw an exception when executed on Windows, since I don't know how to access the installed locales on a Windows system.

If you are using the master branch, simply do a composer update, otherwise update the version number to 1.2.* and you should be good to go.

Let me know if you still run into any issues after upgrading!

rhulshof commented 10 years ago

Awesome, thank you very much! I will setup a test environment a.s.a.p..

For information about locale on Windows I found something here: http://www.zen-cart.com/showthread.php?133111-locale-not-available-on-local-server-maybe

The Windows machine needs locales installed in order to recognize a locale. You can do so using Windows Update: "Optional updates". By default Windows installs only the local locale and maybe the English locale.

Windows lacks support for utf8 by the way, so ommit setting the encoding. In my experience it then still works fine. I guess it just picks up the encoding set on the .po file anyway.

netson commented 10 years ago

Thanks for the link. It only confirms what I already suspected: there is no command similar to locale -a on Windows. Also, I am aware of the fact that Windows does not support UTF8, but it does allow you to set the encoding (codepage) when using php's setlocale function:

setlocale(LC_ALL, "dutch.1252");

should work just fine.

I believe the package should work just fine if you add the correct (and existing) locale names and codepages for a windows system.

For a list of all supported languages on Windows, check out this link. For a list of available codepages, check out this link.

Please let me know if you run into any issues on your test system, if there are package related issues, I will be happy to look into it!

netson commented 10 years ago

Any updates on this? Or can I close this issue?

rhulshof commented 10 years ago

Yeah I'll send you the updates a.s.a.p. I had some personal stuff to attend to. Sorry for the delay.

netson commented 10 years ago

No problem, just checking! :)

rhulshof commented 10 years ago

I've setup the testing environment, so it's quite easy for me to do some further testing from now on.

I have added 'dutch' to the locales list and added '1252' to the encodings list. I've changed the config file accordingly. This is what I get:

Netson \ L4gettext \ LocaleNotFoundException
The given locale [dutch.windows1252] could not be set; it seems it does not exist on this system

and

Netson \ L4gettext \ LocaleNotFoundException
The given locale [dutch.1252] could not be set; it seems it does not exist on this system

I have also tried "english", but that didn't work either. Both English and Dutch are installed locales on my Windows 7 machine.

netson commented 10 years ago

Hey, thanks for the feedback. Could you try changing the L4gettext.php file:

on line 124, you'll find a method called getLocaleAndEncoding; if you could change that to the following and see what happens:

public function getLocaleAndEncoding ()
    {
        // windows compatibility - use only the locale, not the encoding
        if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN')
            return $this->getLocale();
        else
            return $this->getLocale() . "." . $this->getEncoding();

    }

This way I don't have to add test code to the repository. From what I found online, it seems Windows is perfectly happy with getting just the locale information without the encoding, so I've added a windows specific fix.

If this works, I can add the fix to the package.

netson commented 10 years ago

Maybe you can also try running the unittests on windows, see if that shows any errors that may help us locate the problem

rhulshof commented 10 years ago

Hi! I've tested this. It works indeed.

However, that would mean 'Dutch' on the Windows development machines, whilst its 'nl_NL' on the CentOS production servers. Not really consistent.

Recently I tried adding GetText into Laravel manually, so I can confirm that it is possible to have an OS-independant way of implementing internationalisation. This is the filter I created for that; perhaps you can copy&paste the appropriate stuff in order to make your package work OS-independantly:

Route::filter('i18n', function()
{

    // Get default language from Laravel configuration
    $lang = Config::get('app.locale');

    if (Session::has('i18n'))
    {
        // Get language from session
        $lang = Session::get('i18n');
    }
    elseif ($browser_language = $_SERVER['HTTP_ACCEPT_LANGUAGE'])
    {
        $browser_language = substr($browser_language, 0, 2);
        $available_languages = Config::get('app.languages');

        if (in_array($browser_language, $available_languages))
        {
            // Get language from browser
            $lang = $browser_language;
        }
    }

    // Store language in session
    Session::put('i18n', $lang);

    // Use language in current request
    Config::set('app.locale', $lang);

    switch ($lang)
    {
        case 'nl':
            $lang = 'nl_NL';
            break;
        default:
            $lang = 'en_US';
            break;
    }

    // Tell GetText() which language to use
    putenv('LC_ALL=' . $lang);
    setlocale(LC_ALL, $lang . '.utf8');
    bindtextdomain('messages', app_path() . '/locale');
    bind_textdomain_codeset('messages', 'utf8');
    textdomain('messages');

});
netson commented 10 years ago

Are you sure this works on Windows? UTF8 is not supported by any Windows version, so I doubt that works. You should test to see what the setlocale() function returns; if it returns false, the locale/encoding does not exist on the system; this behaviour is consistent on both windows and linux. Windows will fall back on the default language setting in this case.

As for having a different setting on each server, that is correct. That is one of the nasty side effects of setlocale, as this text from the php website explains:

Return Values

Returns the new current locale, or FALSE if the locale functionality is not implemented on your platform, the specified locale does not exist or the category name is invalid.

An invalid category name also causes a warning message. Category/locale names can be found in » RFC 1766 and » ISO 639. Different systems have different naming schemes for locales.

Note:

The return value of setlocale() depends on the system that PHP is running. It returns exactly what the system setlocale function returns.

Reference: http://php.net/manual/en/function.setlocale.php

rhulshof commented 10 years ago

Yes I understand, but I guarantee that it works. On our project it works for weeks now; 100% stable.

So I guess the difference is that your code acts upon the return value (false) and throws an Exception. My code simply ignores the return value. In that case it seems to work smoothly.

netson commented 10 years ago

That's my point, if setlocale() returns false, it means the locale and/or encondig has not been set successfully. So even though it doesn't throw an exception, it doesn't work either: on windows it doesn't actually set/change any language settings in that case.

Your code suggestion ignores the error, but it also ignores the failure the setlocale has encountered. Your suggestion may enable you to use the package on windows without it throwing exceptions, but it's not actually working.

Have you actually translated (part of) your application using poedit or some other tool and tried setting the locale to en_US or nl_NL to see if you see the translated text on your Windows system? Because I would find that difficult to believe based on what I have read about setlocale() so far (which is quite significant).

I think it is inevitable that you have to set different locales and/or encodings on different systems, because that is the way the php setlocale function works; I cannot change that. That is why I added the fetch CLI command, so that it is easy to populate the locales and encodings files; unfortunately, that also doesn't work on windows. The way gettext is designed is that it uses the OS's native locale functions for speed purposes, but the drawback of it is that you have to make sure gettext and the locales/encodings are properly installed and configured on each system.

If you really want to use 1 locale definition on all systems, I suggest you build in a failsafe in your code which checks for Windows and catches the exception if it is thrown, but that may result in odd behaviour on other systems.

rhulshof commented 10 years ago

On our project we've added the GetText function <?php echo _('example'); ?> with English content all over the place. We have translated those strings using POedit. When I set my browser language to Dutch, my page is displayed in Dutch. In case of any other setting, the page remains in English. The only code I have in place is the one I copy&pasted above already. I've tested on both Windows and CentOS.

netson commented 10 years ago

Hmm, odd, because I would expect the utf8 encoding to be problematic and the nl_NL locale as well. Would you humor me and see what the setlocale() function in your code sample returns? False or the locale?

If your code sample works, I would expect my package to work as well, as it sets the exact same locale/encoding with the same php function...

netson commented 10 years ago

Any updates on this? Or can I close the issue?

rhulshof commented 10 years ago

As I've mentioned above, the setlocale() returns false, even though it sets the locale correctly in Windows. The problem with this, is that indeed you'll never know if it has been set correctly or not. A stupid window flaw, I know.

Further, nope, no updates from me :)

daniel-rodas commented 7 years ago

When setting locale on Windows you must uses a dash in the locale string instead of a dash. So. setlocale( LC_ALL, "en-US" );

Notice "en-US" instead of "en_US"