Opened 17 years ago

Closed 14 years ago

#4311 closed (fixed)

urlresolvers.resolve and friends should return the name they resolved to.

Reported by: Jeremy Dunck <jdunck@…> Owned by: nobody
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jeremy Dunck)

Now that urlresolvers.reverse and named URLPatterns are in place, I think it'd be good for resolve to return the name in addition to the callable and args/kwargs.

This would allow app-level branching depending on URL to be simpler.

As an example from a django-users list thread:

  is_login = request.path in (
  if ((not is_login) and ...

If urlresolvers.resolve() returned, (callable, args, kwargs, name) rather than (callable, args, kwargs), (where name is the name assigned in the URLPattern definition), you could have simpler code that inspected URL names and decided what to do with less muss.

if urlresolvers.resolve(url)[3].startswith('contrib.auth.urls.') 

The 3 is icky and auth doesn't actually include an urls file, but you get the point. ;-)

Further, it'd be good if names were nested based on includes in a way that mirrors URLResolver includes so that the name of anything included by an app was prefixed by that app name.

For example:

urlpatterns = (
 ('^admin/', include('django.contrib.admin.urls')

Assuming the admin urls definition was changed to include things like this:

    ('^([^/]+)/([^/]+)/$', 'django.contrib.admin.views.main.change_list', name='change_list'),

Then a request path resolving to the admin change list would include a return name of 'django.contrib.admin.urls.change_list'.

I imagine the advice that urlconf names be global-ish is just due to current implementation limitation. :)

Change History (9)

comment:1 by Jeremy Dunck, 17 years ago

Description: modified (diff)

comment:2 by Jeremy Dunck, 17 years ago

Description: modified (diff)

comment:3 by Jeremy Dunck, 17 years ago

Description: modified (diff)

comment:4 by Jeremy Dunck, 17 years ago

Description: modified (diff)

comment:5 by Malcolm Tredinnick, 17 years ago

Triage Stage: UnreviewedDesign decision needed

The second part is not a good idea. The reason why names are global was described at much length on django-developers in the thread where we discussed the design. Names cannot depend on installation location (or import path, rather), otherwise code and templates that uses the name has to change if you change the installation location (which basically means it's useless for apps that are designed to be self-cotnained and used by third-parties).

The first request might have legs, though. Is there much in the way of backwards-incompatibility impact here? I haven't looked in that part of the code much lately to be familiar with who's calling it, but I would suspect it's mostly internal-only use except for slightly left-field applications like the ones you are apparently writing.

comment:6 by Jeremy Dunck, 17 years ago

Ah, you're right on the naming stability point, of course.

As for the left-field apps, I'm not writing it, but rather making a ticket for something another user wished for.

The application (to me) is probably restricted to middleware, and maybe even just request middleware. In any case, it's a small matter, but also looks to me to be a small change.

comment:7 by Malcolm Tredinnick, 17 years ago

For future ticket readers, the motivation is this django-users thread.

I wonder if this is the right solution to the problem, though? A side-effect of this ticket and the uses in middleware is that we're going to have seemingingly endless requests to put names on every URL in core and the contrib modules, just for random middleware uses. This is going to feel like redundant tagging after a while, I suspect, because the "name" feature is meant to be a differentiator, not merely "yet another name".

So, yeah, I can't see the harm in the change, the impact is probably small and it's a one-line change. +1 on those grounds. However, I'm also not sure it's the right solution to the problem trying to be solved in that thread. I've been wrong before, though; this could be another one of those times.

Let's think about this for a couple of days to see if a better idea pops out of the primordial ooze.

comment:8 by Russell Keith-Magee, 14 years ago

This was fixed by [13479].

comment:9 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top