Django

Code

Ticket #3228 (closed: fixed)

Opened 2 years ago

Last modified 7 months ago

Added SmartSlashMiddleware

Reported by: mihai_preda@yahoo.com Assigned to: mtredinnick
Milestone: Component: Core framework
Version: SVN Keywords: APPEND_SLASH middleware
Cc: jesse.lovelace@gmail.com, silassewell@gmail.com, andy-django@thecablelounge.com, real.human@mrmachine.net Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

smartslash.diff (1.0 kB) - added by mihai_preda@yahoo.com on 01/04/07 07:44:28.
SmartSlashMiddleware?
smartslash-2.diff (7.6 kB) - added by andy-django@thecablelounge.com on 07/22/07 02:37:32.
changes.diff (1.9 kB) - added by Andy Gayton <andy-django@thecablelounge.com> on 12/02/07 12:41:15.
changes since smartslash-2.diff was created
smartslash-3.diff (10.2 kB) - added by Andy Gayton <andy-django@thecablelounge.com> on 12/02/07 15:55:46.
smartslash-3.1.diff (10.2 kB) - added by Andy Gayton <andy-django@thecablelounge.com> on 12/02/07 15:58:34.
fixed typo in docs/middleware.txt

Change History

01/04/07 07:44:28 changed by mihai_preda@yahoo.com

  • attachment smartslash.diff added.

SmartSlashMiddleware?

01/18/07 15:00:52 changed by SmileyChris

  • stage changed from Unreviewed to Design decision needed.

02/10/07 04:32:25 changed by mtredinnick

  • needs_docs set to 1.

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.

04/02/07 14:34:29 changed by gsf@perfectlygood.net

+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.

04/11/07 07:53:37 changed by anonymous

  • cc set to jesse.lovelace@gmail.com.

06/13/07 18:21:58 changed by anonymous

  • cc changed from jesse.lovelace@gmail.com to jesse.lovelace@gmail.com, silassewell@gmail.com.

06/17/07 14:18:41 changed by anonymous

  • cc changed from jesse.lovelace@gmail.com, silassewell@gmail.com to jesse.lovelace@gmail.com, silassewell@gmail.com, andy-django@thecablelounge.com.

06/22/07 03:01:04 changed by mrmachine

  • cc changed from jesse.lovelace@gmail.com, silassewell@gmail.com, andy-django@thecablelounge.com to jesse.lovelace@gmail.com, silassewell@gmail.com, andy-django@thecablelounge.com, real.human@mrmachine.net.

+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.

06/22/07 17:26:20 changed by andy-django@thecablelounge.com

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

07/21/07 09:48:32 changed by Simon G. <dev@simon.net.nz>

  • needs_better_patch set to 1.
  • summary changed from [patch] Added SmartSlashMiddleware to Added SmartSlashMiddleware.

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

07/21/07 12:59:01 changed by andy-django@thecablelounge.com

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?

07/22/07 02:37:32 changed by andy-django@thecablelounge.com

  • attachment smartslash-2.diff added.

07/22/07 02:41:29 changed by andy-django@thecablelounge.com

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,

09/14/07 00:04:30 changed by andy-django@thecablelounge.com

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

09/29/07 20:50:28 changed by cablehead

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!

12/01/07 08:09:17 changed by jacob

  • stage changed from Design decision needed to Ready for checkin.

12/02/07 11:30:05 changed by mtredinnick

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.

12/02/07 11:30:13 changed by mtredinnick

  • stage changed from Ready for checkin to Accepted.

12/02/07 11:49:50 changed by Andy Gayton <andy-django@thecablelounge.com>

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.

12/02/07 11:57:29 changed by mtredinnick

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

12/02/07 12:00:55 changed by Andy Gayton <andy-django@thecablelounge.com>

  • owner changed from nobody to cablehead.

12/02/07 12:41:15 changed by Andy Gayton <andy-django@thecablelounge.com>

  • attachment changes.diff added.

changes since smartslash-2.diff was created

12/02/07 15:54:51 changed by Andy Gayton <andy-django@thecablelounge.com>

  • 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

12/02/07 15:55:46 changed by Andy Gayton <andy-django@thecablelounge.com>

  • attachment smartslash-3.diff added.

12/02/07 15:58:34 changed by Andy Gayton <andy-django@thecablelounge.com>

  • attachment smartslash-3.1.diff added.

fixed typo in docs/middleware.txt

12/02/07 16:00:13 changed by Andy Gayton <andy-django@thecablelounge.com>

  • owner changed from cablehead to mtredinnick.
  • needs_better_patch deleted.
  • stage changed from Accepted to Ready for checkin.
  • needs_docs deleted.

12/02/07 16:04:24 changed by Andy Gayton <andy-django@thecablelounge.com>

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

12/02/07 17:25:55 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(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.

Add/Change #3228 (Added SmartSlashMiddleware)




Change Properties
Action