Opened 17 years ago

Closed 16 years ago

#3228 closed enhancement (fixed)

Added SmartSlashMiddleware

Reported by: mihai_preda@… Owned by: Malcolm Tredinnick
Component: Core (Other) Version: dev
Severity: normal Keywords: APPEND_SLASH middleware
Cc: jesse.lovelace@…, silassewell@…, andy-django@…, real.human@… 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

Implements a new way of 'append slash redirect', as described here:
http://groups.google.com/group/django-developers/browse_thread/thread/d6ca6e1442658598

This patch requires some of the small refactorings from ticket #3224.

Attachments (5)

smartslash.diff (1.0 KB ) - added by mihai_preda@… 17 years ago.
SmartSlashMiddleware
smartslash-2.diff (7.6 KB ) - added by andy-django@… 17 years ago.
changes.diff (1.9 KB ) - added by Andy Gayton <andy-django@…> 16 years ago.
changes since smartslash-2.diff was created
smartslash-3.diff (10.2 KB ) - added by Andy Gayton <andy-django@…> 16 years ago.
smartslash-3.1.diff (10.2 KB ) - added by Andy Gayton <andy-django@…> 16 years ago.
fixed typo in docs/middleware.txt

Download all attachments as: .zip

Change History (28)

by mihai_preda@…, 17 years ago

Attachment: smartslash.diff added

SmartSlashMiddleware

comment:1 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedDesign decision needed

comment:2 by Malcolm Tredinnick, 17 years ago

Needs documentation: set

I'm +0 on this, I guess. If we do use it, I think it should replace the existing APPEND_SLASH behaviour in CommonMiddleware, rather than becoming yet another option. It looks like it will work fairly smoothly in cases where APPEND_SLASH is currently being used. However, we will need the facility to disable it entirely, just as with APPEND_SLASH.

comment:3 by gsf@…, 17 years ago

+1 on this patch replacing the existing APPEND_SLASH behavior. When I want to create a URL without an end slash, I shouldn't have to set a flag that breaks the admin interface. This patch puts the control of URLs where it should be -- in urls.py.

comment:4 by anonymous, 17 years ago

Cc: jesse.lovelace@… added

comment:5 by anonymous, 17 years ago

Cc: silassewell@… added

comment:6 by anonymous, 17 years ago

Cc: andy-django@… added

comment:7 by Tai Lee, 17 years ago

Cc: real.human@… added

+1 on this from me too. We should be able to define URLS that specifically do or do not end in a slash in urls.py, instead of having to choose one or the other and depending on files containing a period.

comment:8 by andy-django@…, 17 years ago

+1 something like this would help me out a lot as well

comment:9 by Simon G. <dev@…>, 17 years ago

Patch needs improvement: set
Summary: [patch] Added SmartSlashMiddlewareAdded SmartSlashMiddleware

Can one of the +1 commenters here want to fix up the current patch to trunk and implement Malcolm's suggestions above?

comment:10 by andy-django@…, 17 years ago

Just checking - Malcolm's suggestions are to:

  • port smartslash.diff to replace the existing APPEND_SLASH behaviour in CommonMiddleware?
  • and to provide a patch for the docs?

Are there any backwards compatibility concerns about changing the current behaviour?

by andy-django@…, 17 years ago

Attachment: smartslash-2.diff added

comment:11 by andy-django@…, 17 years ago

Attached is a new patch to replace the existing APPEND_SLASH behaviour in CommonMiddleware, with unit tests.

If this looks ok, I'll do up a patch as well for the docs.

cheers,

comment:12 by andy-django@…, 17 years ago

Just a quick note, smartslash-2.diff doesn't require the refactorings from ticket #3224.

comment:13 by cablehead, 16 years ago

Gentle bump for review - we're using this patch over at http://rst2a.com - it'd be great to get something which has this effect back into trunk.

Cheers!

comment:14 by Jacob, 16 years ago

Triage Stage: Design decision neededReady for checkin

comment:15 by Malcolm Tredinnick, 16 years ago

Two small problems before this can be checked in:

  1. lines 59-69 (the exception message) won't work as expected. It's not a multi-line string. Wrap things in parentheses or something.
  2. There should be a small documentation change to middleware.txt included to describe the new behaviour.

comment:16 by Malcolm Tredinnick, 16 years ago

Triage Stage: Ready for checkinAccepted

comment:17 by Andy Gayton <andy-django@…>, 16 years ago

Hey Malcolm, I can get those things fixed up in the next couple of hours if you like.

Not sure if you have started on these fixes yet?

I did an svn up the other day, and got some conflicts on this patch. I think they need to be resolved as well.

comment:18 by Malcolm Tredinnick, 16 years ago

Andy: I'm not fixing it, so go ahead.

comment:19 by Andy Gayton <andy-django@…>, 16 years ago

Owner: changed from nobody to cablehead

by Andy Gayton <andy-django@…>, 16 years ago

Attachment: changes.diff added

changes since smartslash-2.diff was created

comment:20 by Andy Gayton <andy-django@…>, 16 years ago

  • damn sorry, overlooked the lines 59-69 the, i've added a test for it and fixed
  • docs updated
  • resolved conflicts
    • http.get_host(request) updated to request.get_host()
    • added test for #5762 and reapplied the urlquote change
  • now making use of tests/urls.py which gets rid of the URLCONF hackery in the middleware test setUp

by Andy Gayton <andy-django@…>, 16 years ago

Attachment: smartslash-3.diff added

by Andy Gayton <andy-django@…>, 16 years ago

Attachment: smartslash-3.1.diff added

fixed typo in docs/middleware.txt

comment:21 by Andy Gayton <andy-django@…>, 16 years ago

Needs documentation: unset
Owner: changed from cablehead to Malcolm Tredinnick
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:22 by Andy Gayton <andy-django@…>, 16 years ago

Hope this is good to go, any issues, just assign back to me.

comment:23 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: newclosed

(In [6852]) Fixed #3228 -- Added new APPEND_SLASH handling behaviour in the common middleware. Makes customisation a bit easier. Thanks, Mihai Preda and Andy Gayton.

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