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

Documented the ability to specify RegExp for named parameter #1065

Closed
wants to merge 2 commits into from

Conversation

Jokero
Copy link
Contributor

@Jokero Jokero commented Apr 21, 2016

https://github.com/restify/node-restify/blob/5.x/lib/router.js#L90

I am amazed at how many things are not documented in restify...

@Jokero
Copy link
Contributor Author

Jokero commented Apr 21, 2016

But I can't use backslashes:

server.get('/hello/:name(\\d+)', send);

Because url module convert them to forward slash. Also can't use specific for url characters like "?" and "#"

@retrohacker
Copy link
Member

Hey @Jokero,

Yes, the restify documentation definately needs some love, as does the website design and overall branding of the project. It is absolutely on our radar. Some of the heavier technical challenges for 5.x will probably take precedence first followed closely by a documentation overhaul. The rationale is that restify is currently being used in production in a lot of business critical places, and the teams supporting and using restify have been using the code base as the definitive source of truth in lieu of proper documentation. This is working fine ™️ for now from the perspective of those heavily invested in the framework, but we understand this isn't ideal for many people and is hindering adoption. We welcome contributions like this one ❤️ they are definitely a step in the right direction.

I'm going to try to validate and proof read this PR today, will let you know soon!

Copy link
Member

@retrohacker retrohacker left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@retrohacker
Copy link
Member

Tested with:

var server = require('restify').createServer({ name: 'foobar' })

server.get('/hello/:name([a-zA-Z0-9]+)', function(req, res, next) {
  console.log(req.params)
  res.send(200)
  return next()
})

server.listen(8080)

@retrohacker
Copy link
Member

Hmm, terribly sorry @Jokero, after reviewing I went to resolve the conflicts and it appears this file no longer exists. Part of the work I described above is already underway and it seems the project has moved out from under this PR. I'm going to close this for now, we can revisit later. ❤️

@retrohacker retrohacker closed this May 2, 2017
@retrohacker
Copy link
Member

I'll add this to the FEATURE_REQUESTS.md file for now

retrohacker pushed a commit that referenced this pull request May 2, 2017
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

2 participants