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

A few cases recognize-path doesn't handle. #44

Open
ghost opened this issue Aug 6, 2013 · 9 comments
Open

A few cases recognize-path doesn't handle. #44

ghost opened this issue Aug 6, 2013 · 9 comments

Comments

@ghost
Copy link

ghost commented Aug 6, 2013

Doesn't work:

http://example.com/foo/bar (routes a 404).
foo/bar (routes as 404).

Works fine:

/foo/bar
@ghost
Copy link
Author

ghost commented Aug 6, 2013

/cc @kyrylo

@ghost
Copy link
Author

ghost commented Aug 6, 2013

For the URL I don't think it should try to match anything from the domain but just take the path & use that.

@kyrylo
Copy link
Member

kyrylo commented Aug 6, 2013

foo/bar doesn't work by design.

http://example.com/foo/bar doesn't work because you specified the protocol. Yeah, it's not smart enough to detect whether you provided a protocol or not. It just automatically appends it. So it should be example.com/foo/bar instead.

@ghost
Copy link
Author

ghost commented Aug 6, 2013

@kyrylo can pry-rails not handle the cases Rails cannot? I copy+paste from my URL all the time, and I don't consult RFCs beforehand. I think it's not hard to handle these cases, they're very easy to solve before passing to the Rails API.

@ghost
Copy link
Author

ghost commented Aug 6, 2013

I'm thinking of something like this for the first case:

unless fragment.starts_with? "/"
  warn "Prepending / to #{fragment}"
  fragment.prepend "/"
end

For the second case:

fragment = URI.parse(fragment).path rescue fragment

@kyrylo
Copy link
Member

kyrylo commented Aug 6, 2013

Why not? Looks good to me. I had borne in mind these possible extensions to the command when I worked on it. However, I decided to make it as simple as it can be, because I wasn't sure the patch would be accepted.

@ghost
Copy link
Author

ghost commented Aug 6, 2013

@kyrylo ah I see. okay. should I make the pull request, or you? :-D I'd love for those cases to be handled because anytime I use the recongize-path command I'm copying from a browser's URL bar or somewhere like that. It'd be nice to copy+paste & it "just works".

@kyrylo
Copy link
Member

kyrylo commented Aug 6, 2013

Sure, it would be cool, go for it. You're probably the most active recognize-path user :-P

@ghost
Copy link
Author

ghost commented Aug 7, 2013

added to the never ending list ;-)

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

No branches or pull requests

1 participant