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

Implement Grape::API.recognize_path #1339

Merged
merged 2 commits into from
Mar 28, 2016

Conversation

namusyaka
Copy link
Contributor

ref #1072
I've tried to implement Grape::DSL::InsideRoute#recognize_path.
It returns value conforming the format like [original pattern, expanded params].

This is a first step. I'd like to hear your opinions.
Thanks.

@@ -120,6 +120,7 @@ def reset_routes!
end

def mount_in(router)
@router ||= router
Copy link
Member

Choose a reason for hiding this comment

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

We didn't address mounting an endpoint in multiple places, but eventually we will. So maybe we don't want @router?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, right. Removed.

@dblock
Copy link
Member

dblock commented Mar 26, 2016

It's on the right track, but shouldn't recognize_path return an endpoint?

@namusyaka
Copy link
Contributor Author

@dblock Now recognize_path returns an instance of Grape::Endpoint. How about this?

@dblock
Copy link
Member

dblock commented Mar 27, 2016

Very good. I think the last thing we need is a README entry that explains how you can use recognize_path from the outside of an API, so given a API < Grape::API, calling recognize_path to get a endpoint that would match.

@namusyaka
Copy link
Contributor Author

I think the last thing we need is a README entry that explains how you can use recognize_path from the outside of an API, so given a API < Grape::API, calling recognize_path to get a endpoint that would match.

Hmm, makes sense. If I understand thing correctly, recognize_path should be implemented as a class method of Grape::API.
Maybe we can't call recognize_path outside of an API, without tricky code.
So I moved the method place from InsideRoute to API.
Could you please check this?

@namusyaka namusyaka changed the title Implement Grape::DSL::InsideRoute#recognize_path Implement Grape::API.recognize_path Mar 27, 2016
@dblock dblock merged commit 4ec7789 into ruby-grape:master Mar 28, 2016
@dblock
Copy link
Member

dblock commented Mar 28, 2016

Perfect, merging.

@namusyaka namusyaka deleted the implement-recognize_path branch March 28, 2016 11:07
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.

2 participants