-
-
reviewboard/reviews/urls.py (Diff revision 1) There was no need to change this one; it's fine as is.
Do not use capturing groups when unnecessary in URLs
Review Request #1469 — Created Nov. 20, 2018 and updated
Information | |
---|---|
guest3011 | |
Review Board | |
c342f6c... | |
Reviewers | |
guest375 |
Given a URL pattern of the form `((?P<capture>[a-z]+)/)?`, a call to reverse with the kwargs `{'capture': 'foo'}` will result in a `NoReverseMatch` exception. This occurs because the `capture` param gets hidden behind a positional parameter and Django will expect the positional parameter intead of the named parameter for the call to reverse. We now no longer use capturing groups -- we explicltly mark the outer group as non-capturing (e.g. `(?:(?P<capture>[a-z]+)/)?`, which works as expected. We previously were also using capturing groups in the form of `?(P<capture>(a|b))`. Not only will a non-capturing group work here, the group isn't needed at all: a expression of the form `(?P<capture>a|b)` is sufficient. All instances of this have been fixed. Testing Done: - Ran unit tests. - Viewed the diff fragment view for an interdiff. - Viewed the e-mail preview views in html and text formats. Reviewed at https://reviews.reviewboard.org/r/10273/
Description | From | Last Updated |
---|---|---|
What about other formats? |
|