Opened 15 years ago

Closed 14 years ago

Last modified 12 years ago

#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)

named_404.diff (1.7 KB ) - added by floguy 15 years ago.
Patch to enable this functionality.
named_404_with_test_2.diff (7.0 KB ) - added by Cyberj 14 years ago.
Patch improved to fit with rev12403
named_404_with_test.diff (7.0 KB ) - added by Tobias McNulty 14 years ago.
REALLY updated the patch to latest trunk

Download all attachments as: .zip

Change History (26)

by floguy, 15 years ago

Attachment: named_404.diff added

Patch to enable this functionality.

comment:1 by Malcolm Tredinnick, 15 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 Malcolm Tredinnick, 15 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 floguy, 15 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 Jacob, 15 years ago

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:5 by Jacob, 15 years ago

milestone: 1.11.2

comment:6 by Tobias McNulty, 15 years ago

Owner: changed from nobody to Tobias McNulty
Status: newassigned

working on this at djangocon sprints with adrian nye

comment:7 by Tobias McNulty, 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 Tobias McNulty, 15 years ago

Owner: changed from Tobias McNulty to nobody
Status: assignednew

bumping back to nobody for review and testing by someone else

comment:9 by Tobias McNulty, 15 years ago

Needs tests: unset

comment:10 by Tobias McNulty, 14 years ago

Cc: Tobias McNulty added

comment:11 by Tobias McNulty, 14 years ago

Owner: changed from nobody to Tobias McNulty
Status: newassigned

looks like patch needs updating

by Cyberj, 14 years ago

Attachment: named_404_with_test_2.diff added

Patch improved to fit with rev12403

comment:12 by Cyberj, 14 years ago

Patch needs improvement: unset

Tests are Ok for this patch, waiting for second review.

comment:13 by Cyberj, 14 years ago

Cc: Cyberj added

comment:14 by Tobias McNulty, 14 years ago

Applies against trunk and all tests pass

comment:15 by James Bennett, 14 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 Tobias McNulty, 14 years ago

milestone: 1.3

patch updated to apply to trunk; setting milestone to 1.3

comment:17 by Tobias McNulty, 14 years ago

Triage Stage: AcceptedReady for checkin

marking RFC per mtredinnick

comment:18 by Malcolm Tredinnick, 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 Malcolm Tredinnick, 14 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

by Tobias McNulty, 14 years ago

Attachment: named_404_with_test.diff added

REALLY updated the patch to latest trunk

comment:20 by Tobias McNulty, 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 Tobias McNulty, 14 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:22 by Malcolm Tredinnick, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [13769]) Debug 404 page now displays names of URL patterns, if they exist.

Thanks to Tobias McNulty for the patch. Fixed #9310.

comment:23 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top