Opened 15 years ago
Closed 10 years ago
#7571 closed Bug (fixed)
named groups in the regexes of include() urls break reverse()
Reported by: | Trevor Caira | Owned by: | afuna |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | flavio.curella@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
# In project urls.py: from django.conf.urls.defaults import * urlpatterns = patterns('', (r'^(?P<foo>\w+)/(?P<bar>\w+)/', include('asdf.urls')), ) # In asdf/urls.py: from django.conf.urls.defaults import * urlpatterns = patterns('', url(r'^(?P<bat>\w+)/$', lambda: None, name='test'), ) # Running this in ./manage.py shell... reverse('test', args=['foo', 'bar', 'bat']) # returns: '/foo/bar/foo/' # instead of: '/foo/bar/bat/'
Change History (14)
comment:1 Changed 15 years ago by
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 Changed 15 years ago by
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
If args
is this incorrect way to use reverse with include()
'd URLs, it should raise an exception, not give surprising results, and this should be documented. There is no mention of this behavior in [1]. Also, there is no way to use kwargs
with the {% url %}
template tag.
[1] http://www.djangoproject.com/documentation/url_dispatch/#including-other-urlconfs
comment:3 Changed 15 years ago by
Triage Stage: | Unreviewed → Design decision needed |
---|
Keyword arguments with the url template tag are documented. http://www.djangoproject.com/documentation/templates/#url
Leaving open so someone else can address if an error should be raised here.
comment:4 Changed 15 years ago by
Resolution: | → worksforme |
---|---|
Status: | reopened → closed |
The URL dispatch documentation explains quite clearly the difference between named and non-named groups, and the fact that they result in keyword and positional arguments, respectively. Meanwhile, the documentation for the 'url' tag quite clearly shows how to use keyword arguments.
Please do not reopen this ticket unless you can demonstrate a genuine bug.
comment:5 Changed 15 years ago by
Then why does reverse()
work at all with args
if only named groups have been given in the URL regex? In fact, they work perfectly well until someone tries to use include()
in this way.
I suspect many people might be relying on this undocumented feature, and may find it surprising when they try to use include()
like this. Perhaps the best solution would be to fully support matching named groups with positional parameters (as partial support already exists), and provide documentation of it.
Otherwise a developer might be lured into the trap of depending on this feature and be very surprised to learn that in order to factor out the matching of parts of the url above the application, they have to fundamentally alter the way those URLs are resolved.
comment:6 Changed 15 years ago by
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
Upon further testing, this issue doesn't seem to be related to named URL groups in particular. Witness:
# In project urls.py: from django.conf.urls.defaults import * urlpatterns = patterns('', (r'^(\w+)/', include('asdf.urls')), ) # In asdf/urls.py: from django.conf.urls.defaults import * urlpatterns = patterns('', url(r'^(\w+)/$', lambda: 0, name='asdf'), ) # ./manage.py shell: from django.core.urlresolvers import reverse reverse('asdf', args=['foo', 'bar']) # gives: '/foo/foo/' # and from django.core.urlresolvers import RegexURLResolver from django.conf import settings r = RegexURLResolver('^/', settings.ROOT_URLCONF) r.resolve('/foo/bar/') (<function <lambda> at 0x87a317c>, ('bar',), {})
comment:7 Changed 15 years ago by
Cc: | flavio.curella@… added |
---|
comment:9 Changed 13 years ago by
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:10 Changed 12 years ago by
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
trevor's comment demonstrates a genuine bug.
comment:11 Changed 11 years ago by
it seems okay now:
reverse('test', args=['foo', 'bar', 'bat']) # gives: '/foo/bar/bat/'
comment:12 Changed 11 years ago by
Status: | reopened → new |
---|
comment:13 Changed 10 years ago by
Owner: | changed from nobody to afuna |
---|---|
Status: | new → assigned |
So, it looks like this is actually two things.
The original issue as described has been fixed. This is a case of outer regex + included regex, both using named groups. There's a test case in urlpatterns_reverse/tests.py that matches and confirms:
('inner-extra', '/outer/42/extra/inner/', ['42', 'inner'], {}),
in test_data
where the relevant url patterns are: url(r'^outer/(?P<outer>\d+)/', include('urlpatterns_reverse.included_urls')),
and url(r'^extra/(?P<extra>\w+)/$', empty_view, name="inner-extra"),
comment:6 is a different thing and that one is still broken. That is a case of both the outer and inner regex using positional parameters, and the positions overlapping when the inner regex is appended to the outer regex. So for example:
outer regex: outer/(\d+)/
inner regx: inner/(\d+)/
arguments: 123, 456
resolves to /outer/123/inner/123/
comment:14 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Named groups are populated with keyword arguments, not positional arguments.