fix langs path selection

[ahmedsayedabdelsalam]

the last condition was a dead code cuz >=5 will override >=9

[rmariuzzo]

Thank you @bhanwarpsrathore

[fsasvari]

I don't see the new tag version 1.11.2 or something, so I cannot pull it to my project...

Can you pls tag it as new release?

[IP-Developer]

@rmariuzzo this actually breaks previous installations that were updated to the latest Laravel version without changing the directory structure. I was still able to adjust the command by specifying the source --source=resources/lang but it still seems wrong for a package functionality to break after a minor version update.

[TCM-RAN]

This should be fixed as soon as possible, it breaked the translations for my laravel project, although I'm on the latest version, the old translation directory structure should be also supported.

[bhanwarpsrathore]

@TCM-RAN As @IP-Developer said, you can adjust the command by specifying the source --source=resources/lang

[Chomiciak]

@rmariuzzo we meet again in the same tremendous situation haha; please don't introduce breaking changes into minor versions!
The default behaviour should be left as it was previously, pleaseeeeeeee 🙏
If you need second reviewer I'm happy to help
PS. tagging this right now as 1.11.2 broke CICD for over 10 apps for us 💀

[bhanwarpsrathore]

@Chomiciak I followed him for tagging as 1.11.2. What do you think we should do now, revert the changes? I have access to the package now

[fsasvari]

But this was a bug, because from Laravel 9+ the default lang directory changed.

[Chomiciak]

@Chomiciak I followed him for tagging as 1.11.2. What do you think we should do now, revert the changes? I have access to the package now

Hi, I'm sorry to reply late, but this week is extremely busy for me. I wanted to make a PR, but it has to wait for weekend I'm afraid.

Because the new tag is already up for over a week, reverting the changes would break compatibility for many users that set the library up during this period.

Therefore we need a third approach - that 'reverts' it in case the file/directory is not found. This way we don't break it for users synced for 1.11.1, nor for this who have 1.11.2.

I think this is the only way to handle this, acceptable because this has very low impact on performance due to the offline nature of this lib's functionality.

I'd be able to work on this on Saturday; if someone can do it faster feel free

[Chomiciak]

But this was a bug, because from Laravel 9+ the default lang directory changed.

Doesn't matter. A bug that many relies on cannot just be fixed to break the functionality for those who have taken it into account.

There are many projects upgraded between laravel versions (I hope most developers upgrade frameworks from time to time), and you can't simply say that this is a bug for them :)

[ahmedsayedabdelsalam]

#201

added a fix to support both like laravel did

i believe if added in a minor version too all will be happy

[lphilps]

This broke for me too. We've been on Laravel forever and never bothered to move our resource files from resources/lang to lang since the Laravel framework itself looks for the former and uses it - right through v12. See Application::bindPathsInContainer. It defaults to lang for new installations, but happily uses resources/lang for existing ones if that folder exists.

Regardless, in Laravel v5 a method was added to Application to return the langPath Laravel chose, so it seems to me the "right fix" to this code is just ask Laravel for the the langPath instead of trying to guess it.

if ($laravelMajorVersion === 4) {
    $langs = $app['path.base'].'/app/lang';
} elseif ($laravelMajorVersion >= 5) {
    $langs = $app->langPath();
}

[Chomiciak]

This broke for me too. We've been on Laravel forever and never bothered to move our resource files from resources/lang to lang since the Laravel framework itself looks for the former and uses it - right through v12. See Application::bindPathsInContainer. It defaults to lang for new installations, but happily uses resources/lang for existing ones if that folder exists.

Regardless, in Laravel v5 a method was added to Application to return the langPath Laravel chose, so it seems to me the "right fix" to this code is just ask Laravel for the the langPath instead of trying to guess it.

if ($laravelMajorVersion === 4) {
    $langs = $app['path.base'].'/app/lang';
} elseif ($laravelMajorVersion >= 5) {
    $langs = $app->langPath();
}

I completely agree for next version, however for now this should be kept with the weird fix I've proposed to ensure everyone, no matter when installed, will have the thing working

[bhanwarpsrathore]

@Chomiciak I just released beta version with laravel 12 support, Would you give it a try?