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

Allow escaping so that a literal ":" can be used in a route. #191

Closed
brianedgerton opened this issue May 1, 2013 · 6 comments
Closed

Allow escaping so that a literal ":" can be used in a route. #191

brianedgerton opened this issue May 1, 2013 · 6 comments

Comments

@brianedgerton
Copy link
Contributor

Use Case
I'm trying to create an OPTIONS endpoint that returns some API documentation with a url like:

/api/posts/:post/comments/:comment

No matter what I do, I can't escape the colons to prevent them from being turned into the catch-all regexps in the router.

Code
I've tracked the route string transformation all the way down to regifyString() in /lib/director/router.js. It would seem that the parsing there needs to be slightly changed. If this would be a welcomed change, I can work on a PR.

Thanks!
Brian Edgerton

@edef1c
Copy link

edef1c commented May 1, 2013

:: may be a nice syntax.

@brianedgerton
Copy link
Contributor Author

A syntax for escaping or for use in the URL? As it stands currently, /::post_id would still result in the regexp substitution. I tried that in my quest to get it to escape :)

@edef1c
Copy link

edef1c commented May 1, 2013

Yeah, just my proposed syntax. It seems reasonably unambiguous to me.

@brianedgerton
Copy link
Contributor Author

Ok, I'll play around with the code a little bit. Just so we're clear on the expectation, declaring a route like this:

/api/posts/::post/comments/::comment

Would produce a literal url like this:

/api/posts/:post/comments/:comment

Sound right?

@edef1c
Copy link

edef1c commented May 2, 2013

Yep.

@brianedgerton
Copy link
Contributor Author

@nathan7 I've submitted a PR with tests added to test/server/core/regifystring-test.js. Everything is passing.

@edef1c edef1c closed this as completed in 045fecd May 3, 2013
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

2 participants