Opened 9 years ago

Closed 5 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: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by jdunck)

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 Changed 9 years ago by jdunck

  • Description modified (diff)

comment:2 Changed 9 years ago by jdunck

  • Description modified (diff)

comment:3 Changed 9 years ago by jdunck

  • Description modified (diff)

comment:4 Changed 9 years ago by jdunck

  • Description modified (diff)

comment:5 Changed 9 years ago by mtredinnick

  • Triage Stage changed from Unreviewed to Design 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 Changed 9 years ago by jdunck

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 Changed 9 years ago by mtredinnick

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 Changed 5 years ago by russellm

This was fixed by [13479].

comment:9 Changed 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top