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

Extend routes --grep to also filter routes by matching against path #45874

Merged
merged 1 commit into from Aug 25, 2022

Conversation

orhantoy
Copy link
Contributor

@orhantoy orhantoy commented Aug 23, 2022

Summary

When looking at a path like /users/orhantoy/settings it's not always obvious which controller-action corresponds to this route. So, I thought it would be useful to be able to look up what controller-action matches a given path. Turns out this is actually possible via /rails/info/routes (news to me!) but I thought it would also come in handy to have the same functionality in rails routes.

Example

Revised after #45874 (comment) (I originally introduced a separate --path option):

$ bin/rails routes -g /cats/1
Prefix Verb   URI Pattern         Controller#Action
   cat GET    /cats/:id(.:format) cats#show
       PATCH  /cats/:id(.:format) cats#update
       PUT    /cats/:id(.:format) cats#update
       DELETE /cats/:id(.:format) cats#destroy

@flavorjones flavorjones added the ready PRs ready to merge label Aug 23, 2022
@flavorjones
Copy link
Member

flavorjones commented Aug 23, 2022

Thank you for submitting this! Looks ready for review by a core committer, I've tagged it as such.

@p8
Copy link
Member

p8 commented Aug 23, 2022

Hi @orhantoy,
Would it maybe make sense to extend the functionality of the existing --grep option to support parsing ids?
So we could do bin/rails routes --grep /cats/1, instead of adding a new option?

@orhantoy
Copy link
Contributor Author

orhantoy commented Aug 23, 2022

@p8 That could work but there is a downside: --grep does a fuzzy match so if we also wanted to do a fuzzy path match, similar to

fuzzy: match_route { |it| it.spec.to_s.match path }

we would also get matches for nested routes like /cats/1/profile.

Both exact and fuzzy would be useful so perhaps I could extend the --grep option to do the fuzzy match but also keep --path for the exact match; what do you think?

The reason why I would advocate for the exact match, is because a very common question when starting out with Rails or when trying to understand a new Rails app is: what controller-action is called when I go to /some/specific/123/path?


Update: I might be wrong about how the current fuzzy match works in /rails/info/routes

fuzzy: match_route { |it| it.spec.to_s.match path }

It seems to match against the path pattern, like /cats/:id, not an actual patch. I will test this out and report back.

@simi
Copy link
Contributor

simi commented Aug 23, 2022

Why to not use system grep like bin/rails routes | grep /cats/:id?

@orhantoy
Copy link
Contributor Author

@simi If you're talking about the feature I'm proposing here, which is to see which routes a given path resolves to, then grep cannot help. With this feature you can take a real URL path, like /cats/1/friends, and see the associated routes.

If you're talking about the --grep option, that already exists for rails routes. Personally, I didn't know about it until recently, so I have also been using grep like you're showing.

@orhantoy
Copy link
Contributor Author

@p8 I think reusing the --grep option can work. And I checked the fuzzy match in /rails/info/routes and that works just like --grep (meaning it matches the path pattern).

I have updated the PR with the relevant changes. Let me know what you think.

Copy link
Member

@byroot byroot left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one very minor style concern.

Please squash your commits and I'll merge.

actionpack/lib/action_dispatch/routing/inspector.rb Outdated Show resolved Hide resolved
@orhantoy orhantoy force-pushed the cmd-find-routes-matching-path branch from ed57028 to 930e36f Compare August 25, 2022 08:35
@orhantoy orhantoy changed the title rails routes: filter routes by matching against path Extend routes --grep to also filter routes by matching against path Aug 25, 2022
@orhantoy orhantoy requested a review from byroot August 25, 2022 08:49
@byroot byroot merged commit a6bf6d5 into rails:main Aug 25, 2022
@p8
Copy link
Member

p8 commented Aug 25, 2022

Looks great @orhantoy !

@orhantoy orhantoy deleted the cmd-find-routes-matching-path branch August 25, 2022 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants