Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel 9 lang folder location #48

Merged
merged 6 commits into from
Feb 22, 2022
Merged

Laravel 9 lang folder location #48

merged 6 commits into from
Feb 22, 2022

Conversation

voicecode-bv
Copy link

@voicecode-bv voicecode-bv commented Feb 12, 2022

No description provided.

Voicecode B.V added 2 commits February 12, 2022 10:32
Since Laravel 9 the lang folder has been moved outside the resources folder. Using the version compare method the target folder will be set to support older versions of Laravel.
@freekmurze
Copy link
Member

Seems like the tests are failing for this one. Could you take a look?

@riasvdv
Copy link
Member

riasvdv commented Feb 21, 2022

There's been a lang_path function since Laravel 8.x, is it possible to use that one instead? Maybe we can drop support for Laravel 7.x as well?

@freekmurze
Copy link
Member

Using lang_path is probably the way to go. We use laravel-package-tools for packages of our own that need to support older Laravel versions (such as ray).

@voicecode-bv
Copy link
Author

@freekmurze Updated the PR using the lang_path helper. Funny... I was not aware of the helper, it doesn't seem to be in the docs too.

src/PackageServiceProvider.php Outdated Show resolved Hide resolved
@voicecode-bv
Copy link
Author

Thanks @danilopolani! I've updated the path.

@freekmurze
Copy link
Member

Seems like Laravel 7 and lower do not have a lang_path function. Could you solve this by using the old lang path if we detect Laravel 7 or below?

@riasvdv
Copy link
Member

riasvdv commented Feb 22, 2022

Just a method_exists('lang_path') should work for detecting it

@freekmurze
Copy link
Member

@riasvdv that could work, but I think a Laravel version check could be better as it better communicates why lang_path does not exist.

But I have no strong opinion on it, @voicecode-bv: you can do what you prefer.

@voicecode-bv
Copy link
Author

@freekmurze @riasvdv I had the feeling I needed to research when the lang_helper function was added, and I think I found it here. It seems like earlier versions of Laravel 8 also don't have the helper function. So I've added a 8.64 version check.

@danilopolani
Copy link

At this point wouldn't be easy to just stick with the standard app()->langPath() / $this->app->langPath()? It's present in L7 and even L6, so you don't need to do magic version checks (?)

@voicecode-bv
Copy link
Author

@danilopolani if this works, it works. If another way is desired, please open another PR. Feels like this is taking too much time already :)

@freekmurze freekmurze merged commit 16a8de8 into spatie:main Feb 22, 2022
@freekmurze
Copy link
Member

It works! Thanks for your work on this! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants