#7206 closed (fixed)
django.core.urlresolvers.reverse doesn't work properly
Reported by: | Owned by: | Malcolm Tredinnick | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Keywords: | url reverse | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
here is the urlpatterns
url(r'^test/1/?',views.test1,name='test1'), url(r'^(?i)test/2/?$',views.test2,name='test2'),
then i test the reverse function as follows:
>>> from django.core.urlresolvers import reverse >>> print reverse('test1') /test/1/?
the url i expect is '/test/1/' instead of '/test/1/?'
the '?' in the urlpatterns was add for matching both '/test/1/' and '/test/1'
>>> print reverse('test2') Traceback (most recent call last): File "<console>", line 1, in <module> File "....django\django\core\urlresolvers.py", line 297, in reverse File "....django\django\core\urlresolvers.py", line 283, in reverse File "....django\django\core\urlresolvers.py", line 88, in reverse_helper File "C:\Python25\lib\re.py", line 142, in sub return _compile(pattern, 0).sub(repl, string, count) File "....\django\django\core\urlresolvers.py", line 124, in __call__ NoReverseMatch: Not enough positional arguments passed in
(?i) was added for matching '/test/2' or '/Test/2' and so forth
Attachments (2)
Change History (11)
comment:1 Changed 16 years ago by
comment:2 Changed 16 years ago by
Has patch: | set |
---|
got a error while attaching this patch file.
Index: django/core/urlresolvers.py =================================================================== --- django/core/urlresolvers.py (revision 7367) +++ django/core/urlresolvers.py (working copy) @@ -85,8 +85,9 @@ Raises NoReverseMatch if the args/kwargs aren't valid for the regex. """ # TODO: Handle nested parenthesis in the following regex. - result = re.sub(r'\(([^)]+)\)', MatchChecker(args, kwargs), regex.pattern) - return result.replace('^', '').replace('$', '') + pattern = re.sub(r'\(\?[iLmsux]+\)','',regex.pattern) + result = re.sub(r'\(([^)]+)\)', MatchChecker(args, kwargs), pattern) + return result.replace('^', '').replace('$', '').replace('?','') class MatchChecker(object): "Class used in reverse RegexURLPattern lookup."
comment:3 Changed 15 years ago by
Component: | Uncategorized → Core framework |
---|
comment:4 Changed 15 years ago by
milestone: | → 1.0 |
---|---|
Triage Stage: | Unreviewed → Accepted |
Changed 15 years ago by
Attachment: | 7206.patch added |
---|
comment:5 Changed 15 years ago by
This patch causes the following failure.
wireless-128-62-162-35:tests tlpinney$ ./runtests.py --settings=foobar.settings urlpatterns_reverse ====================================================================== FAIL: test_urlpattern_reverse (regressiontests.urlpatterns_reverse.tests.URLPatternReverse) ---------------------------------------------------------------------- Traceback (most recent call last): File "/Users/tlpinney/hg/django_trunk/tests/regressiontests/urlpatterns_reverse/tests.py", line 37, in test_urlpattern_reverse self.assertEquals(got, expected) AssertionError: 'hardcoded/doc\\.pdf' != 'hardcoded/doc.pdf' ---------------------------------------------------------------------- Ran 1 test in 0.004s FAILED (failures=1)
Changed 15 years ago by
Attachment: | 7206-2.patch added |
---|
Fixes problem with the test failing, adds a couple regression tests
comment:6 Changed 15 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
comment:7 Changed 15 years ago by
Owner: | changed from nobody to Malcolm Tredinnick |
---|---|
Triage Stage: | Ready for checkin → Accepted |
This is in the same boat as #4915, #6934, etc. It's unmaintainable to fix things piecemeal. I have a slightly larger rewrite that I'll finish up shortly. Leaving open so that I don't forget to consider this option (I'm not sure allowing ?i
in URL reg-exp patterns designed for reversing is a good idea, but I'll think about it).
comment:8 Changed 15 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [8760]) A rewrite of the reverse URL parsing: the reverse() call and the "url" template tag.
This is fully backwards compatible, but it fixes a bunch of little bugs. Thanks
to SmileyChris and Ilya Semenov for some early patches in this area that were
incorporated into this change.
The CommonMiddleare automatically appends slashes to the end of urls if one isn't already there, so matching both isn't necessary.
That's not to say this isn't a bug, of course.