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

Allowing case insensitive regexp groups after PR #144 #155

Merged
merged 1 commit into from
Mar 7, 2016

Conversation

jprobinson
Copy link
Contributor

Changes in #144 broke regexp matching for case insensitive groups.

This adds a test scenario and a small hunk of code reverted to what it was prior to the PR.

@jprobinson
Copy link
Contributor Author

Benchmark changes:

benchmark                             old ns/op     new ns/op     delta
BenchmarkMux-4                        2712          2569          -5.27%
BenchmarkMuxAlternativeInRegexp-4     3835          3860          +0.65%
BenchmarkManyPathVariables-4          4410          4476          +1.50%

@elithrar
Copy link
Contributor

elithrar commented Mar 7, 2016

Thanks @jprobinson—and thanks for adding the benchmarks (which I was just about to ask for). Happy to lose a few % if it means we maintain API compat, especially since request muxes are almost never bottlenecks in real applications.

elithrar added a commit that referenced this pull request Mar 7, 2016
[bugfix] Allow case insensitive regexp groups (fix regression in #144)
@elithrar elithrar merged commit acf3be1 into gorilla:master Mar 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants