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

Major changes #114

Merged
merged 1 commit into from
Aug 23, 2017
Merged

Major changes #114

merged 1 commit into from
Aug 23, 2017

Conversation

blakeembrey
Copy link
Member

@blakeembrey blakeembrey commented Aug 22, 2017

  • New option! Ability to set endsWith to match paths like /test?query=string up to the query string
  • New option! Set delimiters for specific characters to be treated as parameter prefixes (e.g. /:test)
  • Remove isarray dependency
  • Explicitly handle trailing delimiters instead of trimming them (e.g. /test/ is now treated as /test/ instead of /test when matching)
  • Remove overloaded keys argument that accepted options
  • Remove keys list attached to the RegExp output
  • Remove asterisk functionality (it's a real pain to properly encode)
  • Change tokensToFunction (e.g. compile) to accept an encode function for pretty encoding (e.g. pass your own implementation)

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 0291bd7 on 2.0 into e97122d on master.

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 53c648f on 2.0 into e97122d on master.

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 8a6dab4 on 2.0 into e97122d on master.

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 8a6dab4 on 2.0 into e97122d on master.

* New option! Ability to set `endsWith` to match paths like `/test?query=string` and ignore the query string
* New option! Set `delimiters` for only specific characters to be treated as parameter prefixes (e.g. `/:test`)
* Remove `isarray`
* Explicitly handle trailing delimiters instead of trimming them (e.g. `/test/` is now treated as `/test/` instead of `/test` when matching)
* Remove overloaded `keys` argument that accepts `options`
* Remove `keys` list attached to the `RegExp` output
* Remove asterisk functionality (it's a real pain to properly encode)
* Change `tokensToFunction` (e.g. `compile`) to accept an `encode` function for pretty encoding (e.g. pass your own implementation)
@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 515656d on 2.0 into e97122d on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 100.0% when pulling 515656d on 2.0 into e97122d on master.

@oliviertassinari
Copy link

Remove asterisk functionality (it's a real pain to properly encode)

How should we handle this feature now?

@blakeembrey
Copy link
Member Author

blakeembrey commented Aug 28, 2017

@oliviertassinari Did you try the README? https://github.com/pillarjs/path-to-regexp#compatibility-with-express--4x. It's been this for a while, I only hacked the asterisk feature into the 1.x releases around 1.2, but realised that making it act like it use used in Express.js is a mistake that needed to be fixed properly for 2.0. The closest behaviour that exists closest to how it should act is currently in another PR.

@oliviertassinari
Copy link

Thanks, I was curious, I couldn't find the CHANGELOG. I agree, it's better this way.

@blakeembrey
Copy link
Member Author

There's a history file here: https://github.com/pillarjs/path-to-regexp/blob/master/History.md. I forgot 1.2 was actually so long ago!

@oliviertassinari
Copy link

By bad, I don't have the reflex to look for History.md. My first reflex when upgrading a lib is to look into the releases, then the CHANGELOG.md. I will also look for this file now. I haven't realized it's a convention in the express world.

@blakeembrey
Copy link
Member Author

No problem. I believe it's mostly convention from the people who created the projects initially. In every other projects I use releases also 😄

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.

3 participants