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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Add counts to route:list command #42551

Merged
merged 3 commits into from May 31, 2022

Conversation

ash-jc-allen
Copy link
Contributor

Hi again! This is just another small PR to the route:list command that I think could come in handy. It adds 2 counts to the CLI output showing the amount of routes that are being displayed. One count is placed at the bottom and one is placed at the top.

Here's an example of how it could look if you don't have a lot of routes and can see both counts:

Screenshot 2022-05-28 at 02 01 30

Here's an example of how it could look if you have a lot of routes and can only see the bottom count:

Screenshot 2022-05-28 at 02 01 15

I think could just be another small UI nicety to make the route list even easier (and quicker) to read and understand.

I couldn't seem to get the tests fully working for this. I have a feeling that it might be something to do with me outputting the same line twice, but that's just a guess. But I have updated the tests to pass with what I've done so far.

If you think this is something that might be worth merging, please let me know if you'd like any changes making 馃槃

@DarkGhostHunter
Copy link
Contributor

DarkGhostHunter commented May 28, 2022

Could this also add the total?

Showing [4] of [88] routes.

If the total is the same, then this should say:

Showing all [88] routes.

@ash-jc-allen
Copy link
Contributor Author

I did consider adding the total but couldn't decide whether to include it. If this PR is something that Taylor might want to merge, I'm happy to add it if it's something that would be useful 馃槃

@taylorotwell
Copy link
Member

I would personally just show it at the bottom. Mark as ready for review when tests are passing. Thanks!

@taylorotwell taylorotwell marked this pull request as draft May 30, 2022 13:31
@ash-jc-allen
Copy link
Contributor Author

@taylorotwell No problem, I'll get this update later today :)

@ash-jc-allen ash-jc-allen marked this pull request as ready for review May 30, 2022 20:45
@ash-jc-allen
Copy link
Contributor Author

I've removed the count from the top now, so it's only at the bottom. I've also updated the tests so they should be passing now too :)

@taylorotwell taylorotwell merged commit 9a9c991 into laravel:9.x May 31, 2022
@ash-jc-allen ash-jc-allen deleted the feature/route-list-count branch June 6, 2022 12:11
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.

None yet

3 participants