Does not work on windows about laravel-js-localization HOT 9 CLOSED

Released: https://github.com/rmariuzzo/Laravel-JS-Localization/releases/tag/v1.3.4

from laravel-js-localization.

I partially run the test on Windows, because there were a bunch of edge cases that are like rabbit holes that goes deeper and deeper. Not directly related to this lib, but because of the dependencies on Windows.

from laravel-js-localization.

Perfect, I will check on it this weekend. I'm in the hospital right now for
a scheduled chemo.

On Oct 28, 2016 11:55 AM, "Oleg" [email protected] wrote:

Hello, there are small bug in the code.

Problem is that on windows directory separator is "" instead of "/".

That means, that following code you have won't work:

\Mariuzzo\LaravelJsLocalization\Generators\LangJsGenerator::
isMessagesExcluded

protected function isMessagesExcluded($filePath)
{
if (empty($this->messagesIncluded)) {
return false;
}

    $localeDirSeparatorPosition = strpos($filePath, '/');
    $filePath = substr($filePath, $localeDirSeparatorPosition);
    $filePath = ltrim($filePath, '/');
    $filePath = substr($filePath, 0, -4);

    if (in_array($filePath, $this->messagesIncluded)) {
        return false;
    }

    return true;
}

Please, fix it so it will work on windows too.

Easy fix would be to add following replace:
$filePath = str_replace(DIRECTORY_SEPARATOR,'/',$filePath);

However, I am not sure that this will be sufficient.

Please, take a look on this, I can test on windows, if needed.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#65, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2HzzBR0CTfoI22hnU0aJZtdGKtYHNks5q4hrTgaJpZM4Kjlqn
.

from laravel-js-localization.

@rmariuzzo I will check the code ASAP. That's my fault :}

from laravel-js-localization.

Solution is published, but I don't have any Windows machines to test it. Help required 🆘

from laravel-js-localization.

I can check on monday.

On 28 Oct 2016 22:12, "Pe Ell" [email protected] wrote:

Solution is published, but I don't have any Windows machines to test it.
Help required 🆘

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#65 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANrsHyp_X4JrxG3Pntl_cHeTdEtvr_SZks5q4kkggaJpZM4Kjlqn
.

from laravel-js-localization.

Nope, does not work.

Here is the link to my pull request: #67

from laravel-js-localization.

I'm configuring a Windows image to test this and release if it looks good.

from laravel-js-localization.

I'm still working on it. It is a pain to setup the env in Windows... 😞

from laravel-js-localization.