#9310 closed (fixed)
404 debug pages should show the name of the tried urlpattern if one exists.
| Reported by: | floguy | Owned by: | Tobias McNulty |
|---|---|---|---|
| Component: | Core (Other) | Version: | 1.0 |
| Severity: | Keywords: | ||
| Cc: | Tobias McNulty, Cyberj | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Sometimes it's useful to know what the names of the url patterns were that were being tried. I wrote a small little patch to show that information.
Attachments (3)
Change History (26)
by , 16 years ago
| Attachment: | named_404.diff added |
|---|
comment:1 by , 16 years ago
| Needs tests: | set |
|---|---|
| Patch needs improvement: | set |
You seem to have left off the part of the patch that contained the tests.
I'm fairly sure that when you write those tests you're going to find that line 183 is a problem. The existing code adds 't' to a string. Your new code is adding the same 't' to a list. That looks like a problem.
comment:2 by , 16 years ago
Also the new code on line 183 is using a generator expression, which isn't valid in version of python before 2.5. It needs to remain as a list comprehension.
comment:3 by , 16 years ago
You're right that it's missing tests. I'll work on that just as soon as I get some time in the next few days.
However I think that you're mistaken that line 183 is a problem. It's a recursive call, so by changing the type of the first argument, we're changing the type of all of the t instances. I can't prove it, because I don't have tests (thus reinforcing your first point), but it does work for me.
Good point about the generator expression, though. I've been spending too much time in py2.5 land at work, that I forget what's valid 2.3 code :)
comment:4 by , 16 years ago
| milestone: | → 1.1 |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:5 by , 16 years ago
| milestone: | 1.1 → 1.2 |
|---|
comment:6 by , 15 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
working on this at djangocon sprints with adrian nye
comment:7 by , 15 years ago
The code needs to make a list instead of a string so the template can iterate through the different resolvers and patterns (and hence have access to the final pattern's name). The template has been updated to work with the new format and I couldn't find anything else that depends on Resolver404's 'tried' arg.
I updated Eric's patch to apply to the latest trunk, converted the generator expression into a list comprehension, and created a test for the new feature.
Let me know what you think.
comment:8 by , 15 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
bumping back to nobody for review and testing by someone else
comment:9 by , 15 years ago
| Needs tests: | unset |
|---|
comment:10 by , 15 years ago
| Cc: | added |
|---|
comment:11 by , 15 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
looks like patch needs updating
comment:12 by , 15 years ago
| Patch needs improvement: | unset |
|---|
Tests are Ok for this patch, waiting for second review.
comment:13 by , 15 years ago
| Cc: | added |
|---|
comment:15 by , 15 years ago
| milestone: | 1.2 |
|---|
Though it's a minor feature request, this is still a feature request and thus not in scope for 1.2.
comment:16 by , 14 years ago
| milestone: | → 1.3 |
|---|
patch updated to apply to trunk; setting milestone to 1.3
comment:18 by , 14 years ago
I hate to be picky, but this patch doesn't apply to trunk! Things like urlresolvers.py have changed. Can you check your checkout has r13678 included, since that was the last time urlresolvers.py was changed.
comment:19 by , 14 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
by , 14 years ago
| Attachment: | named_404_with_test.diff added |
|---|
REALLY updated the patch to latest trunk
comment:20 by , 14 years ago
apparently I managed to clone the bitbucket.org/django/django repo this morning before it had been officially released, so I had a version from a couple weeks ago. I pulled the latest, resolved the conflict, and attached the updated patch.
comment:21 by , 14 years ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:22 by , 14 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Patch to enable this functionality.