Opened 14 years ago
Closed 10 years ago
#16362 closed Cleanup/optimization (fixed)
Ignore, rather than disallow, negative lookahead assertions
Reported by: | Owned by: | Bas Peschier | |
---|---|---|---|
Component: | Core (URLs) | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | adam@…, bmihelac@…, DrMeers@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
At present, django.utils.regexhelper.normalize()
is defined and documented to raise an exception on encountering lookahead and lookbehind matches in the name of reversability. This behaviour unnecessarily restricts the range of valid configurations.
Consider the following case:
url(r'^(?P<city>[-\w._])/', name='city-landing'),
One may wish to assert that certain inputs are *certainly not* cities, and should never match this (in my particular use case, the desired outcome is actually a Resolver404
, such that adding an additional, earlier url()
entry to the urlconf is not an option):
url(r'^(?!NotACity)(?P<city>[-\w._])/', name='city-landing'),
This *does* mean that reverse('city-landing', kwargs={'name': 'NotACity'})
will result in an invalid result (a URL which, when resolved, does not in fact match the given reversed string). However, such a case clearly constitutes user error -- as the application developer trying to request a city-landing
URL for NotACity
is asking the impossible -- and does not constitute cause for making such patterns unsupported.
Attachments (1)
Change History (9)
by , 14 years ago
Attachment: | django-regex-allow-lookahead-16362.patch added |
---|
comment:1 by , 14 years ago
Cc: | added |
---|
comment:2 by , 13 years ago
Component: | Uncategorized → Core (Other) |
---|---|
Needs documentation: | set |
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Reading the comment on #2977 and duplicate tickets, I think we can accept this. I'm a bit surprised that the patch is so simple, but why not.
Until now, lookahead/behind assertions were forbidden, so it can't be backwards incompatible.
The docs probably needs updating.
comment:3 by , 13 years ago
Cc: | added |
---|
comment:4 by , 12 years ago
Component: | Core (Other) → Core (URLs) |
---|
comment:5 by , 11 years ago
Cc: | added |
---|
comment:6 by , 10 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Owner: | changed from | to
Status: | new → assigned |
It was never documented that it was not allowed except for the source comments above normalize
. These assertions are part of a valid regular expression, so I only added a release note.
comment:7 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Looks good, pending some cosmetic comments.
Initial proposed patch