Do not use capturing groups when unnecessary in URLs

Review Request #10273 — Created Oct. 24, 2018 and submitted

Information

Review Board
release-4.0.x
c75339d...

Reviewers

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.

  • Ran unit tests.
  • Viewed the diff fragment view for an interdiff.
  • Viewed the e-mail preview views in html and text formats.
Description From Last Updated

Typo in the description: "paramter" -> "parameter"

chipx86chipx86
chipx86
  1. 
      
  2. Typo in the description: "paramter" -> "parameter"

  3. 
      
brennie
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (c342f6c)
Loading...