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

Use symfony/finder to find Requests within Subdirectories #58

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

JonPurvis
Copy link
Contributor

Fixes #53

This PR fixes the issue raised in the Issue above, in that when running php artisan saloon:list, if your files live in a subdirectory, then it doesn't find them and actually fails:

Monosnap laravel10 – composer json 2024-02-05 02-27-24

It didn't even occur to me that some applications would do this 😆

Fortunately, symfony/finder comes to the rescue and makes it easy to find files in directories. This PR basically just swaps usages of glob() to instead use symfony/finder. With this branch, if you run php artisan saloon:list, you now get the expected output:

Monosnap laravel10 – saloon php 2024-02-05 02-25-38

Notice how my Requests live in subdirectories?

This PR actually makes a couple more changes:

  • Finding Connector classes doesn't rely on them having Connector as the file name suffix e.g. StripeConnector.php. If I was to just name it Stripe.php, then that would also work
  • I've opted to include the full path to the file in the output instead of just the filename.

@JonPurvis
Copy link
Contributor Author

cc @Sammyjo20

Another one for you! 🤠

Apologies if you'd already started on this one, if that's the case, feel free to dismiss this PR!

@JonPurvis JonPurvis changed the title use symfony finder to find files Use symfony/finder to find Requests within Subdirectories Feb 5, 2024
@Sammyjo20
Copy link
Member

Ah @JonPurvis you are a legend, thank you so much for doing both of these PRs! ❤️ It helps so much especially when I'm busy.

I'll have a look over the code but I'm sure it's great.

Copy link
Member

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks really good. I should probably invest in some proper tests for commands but for now I'll test it manually

@Sammyjo20 Sammyjo20 merged commit 9ae2f1f into saloonphp:v3 Feb 5, 2024
14 checks passed
@JonPurvis
Copy link
Contributor Author

Ah @JonPurvis you are a legend, thank you so much for doing both of these PRs! ❤️ It helps so much especially when I'm busy.

I'll have a look over the code but I'm sure it's great.

I love Saloon, use it every day, always happy to help out where I can! Especially when it's my initial code that has the issue 😆 I've never used symfony/finder before so it was cool getting to grips with a new package!

@Sammyjo20
Copy link
Member

Thanks man, as always it means a lot <3

@Sammyjo20
Copy link
Member

and yeah, Symfony Finder is super cool, and comes installed in Laravel by default!

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.

Saloon:list command doesn't handle directories
2 participants