Opened 14 years ago
Closed 13 months ago
#14761 closed New feature (needsinfo)
URL resolving / reversing design doesn't allow alternate specs
Reported by: | samuel337 | Owned by: | Marten Kenbeek |
---|---|---|---|
Component: | Core (URLs) | Version: | dev |
Severity: | Normal | Keywords: | url resolve reverse |
Cc: | marten.knbk@… | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Django's URL resolution system is currently based on regexps and is represented by the RegexURLPattern class. There are cases where it would be preferable to subclass this and allow alternate ways of specifying URL patterns that ultimately compile down to regexps.
Right now, this works for URL resolving as during resolution, the resolve method on the RegexURLPattern class is called, and hence subclasses can modify that behaviour. It however, doesn't work for URL reversal, because the resolver class generates the possible matches itself, instead of calling a method on RegexURLPattern or its subclasses as is the case with URL resolution.
The attached patch simply refactors urlresolvers.py so the resolver calls a method on RegexURLPattern to get possible matches. It passes all the URL tests in Django, is completely backwards-compatible and introduces no new issues or quirks.
I don't believe any new tests are required, and because the new method on RegexURLPattern is marked private (hence subject to change), no new docs are required either. This should stay as an unsupported way to extend Django's URL system until it is fully revamped (support disjunctives, URI templates etc.).
The relevant thread on django-developers is here - http://groups.google.com/group/django-developers/browse_thread/thread/c94b551ebd57fbe6/65d79a336fef04b2
Attachments (1)
Change History (9)
by , 14 years ago
Attachment: | ticket-14761.diff added |
---|
comment:1 by , 14 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:5 by , 12 years ago
Component: | Core (Other) → Core (URLs) |
---|
comment:6 by , 9 years ago
Cc: | added |
---|---|
Has patch: | unset |
Needs tests: | unset |
Owner: | changed from | to
Patch needs improvement: | unset |
Status: | new → assigned |
I'll address this as part of https://groups.google.com/forum/#!topic/django-developers/WQoJN_MGTOg.
follow-up: 8 comment:7 by , 13 months ago
I'm not sure how much need for this ticket there is now that we have path converters?
comment:8 by , 13 months ago
Resolution: | → needsinfo |
---|---|
Status: | assigned → closed |
Triage Stage: | Accepted → Unreviewed |
Replying to Tom Carrick:
I'm not sure how much need for this ticket there is now that we have path converters?
That's true. Resolving URLs is not the same as it was 8 years ago, RegexURLResolver
doesn't exist anymore, we have path converters etc. This ticket should be redefined if any changes still need to be made
Of course tests are required; you're adding a new piece of functionality. You need to test that the new functionality works, and will continue to work. As it stands, if this patch were applied, a future patch could go back to just calling normalize instead of calling get_possibilities, and the test suite would continue to run, but any custom URLPattern classes would be broken.