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

Using capture groups in path regexes has unexpected behaviour #143

Closed
endophage opened this issue Jan 19, 2016 · 7 comments
Closed

Using capture groups in path regexes has unexpected behaviour #143

endophage opened this issue Jan 19, 2016 · 7 comments
Assignees

Comments

@endophage
Copy link

I think this is probably just a docs improvement. Unless it's buried somewhere, the docs don't specify that regexes shouldn't be wrapped in a capture group. Doing so causes unexpected behavior when used in any location but the last regex.

Examples:

  • Path: "/{name:.*}.{ext:([a-zA-Z0-9]+)}"
    • input "/foo.bar"
    • vars equals map[name:foo ext:bar]
  • Path: "/{name:(.*)}.{ext:[a-zA-Z0-9]+}"
    • input "/foo.bar"
    • vars equals map[name:foo ext:foo]

It would be great if the docs clearly stated that the regexes shouldn't have an enclosing capture group. At the moment it can only be inferred from the examples, which becomes especially confusing when it works some of the time (i.e. when the only capture group is the final regex).

@elithrar
Copy link
Contributor

Thanks for reporting this! I'll fix the docs to make this clearer.

@elithrar elithrar self-assigned this Jan 21, 2016
@elithrar
Copy link
Contributor

This may have changed since #144 and #155 - can you confirm?

@endophage
Copy link
Author

Appears to still be broken. I ran my tests against the older version (e444e69) I had in my go vendoring, saw the expected failure, updated to mux's head of master, ran the same tests and got the same failure.

@elithrar
Copy link
Contributor

Did you want to submit a PR updating the docs? I won't have time for a while.

@endophage
Copy link
Author

I'm a little overbooked right now too. No promised but if I get time I'll submit an update.

@olt
Copy link
Contributor

olt commented Sep 20, 2016

I was hit by this issue, but it's possible to workaround with non-capturing groups. #196 contains an update to the documentation.

kisielk added a commit that referenced this issue Sep 20, 2016
document non-capturing groups (closes #143)
@elithrar
Copy link
Contributor

Refer to #200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants