Opened 16 years ago

Closed 21 months ago

#6904 closed New feature (wontfix)

Dictsort filter should sort case insensitive

Reported by: michael@… Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: mpjung@…, bmispelon@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Dictsort filter currently sorts case sensitive and thus creates sometimes two groups depending on the case of the text being sorted. The filter should use

.sort(key=str.lower)

Attachments (3)

dictsort.patch (805 bytes ) - added by mathwizard 16 years ago.
Patch against current svn patching dictsort and dictsortreversed filters
6904_revision7393.diff (877 bytes ) - added by Michael Axiak 16 years ago.
New patch that works in python 2.3
6904_revision7393_better.diff (2.5 KB ) - added by Michael Axiak 16 years ago.
Better patch…

Download all attachments as: .zip

Change History (19)

comment:1 by anonymous, 16 years ago

Owner: changed from nobody to anonymous
Status: newassigned

the dictsort function should be like this:

def dictsort(value, arg):
    var_resolve = Variable(arg).resolve
    decorated = [(var_resolve(item), item) for item in value]
    import operator
    decorated.sort(key=lambda x: unicode.lower(x[0]))
    return [item[1] for item in decorated]
dictsort.is_safe = False

comment:2 by anonymous, 16 years ago

Has patch: set

comment:3 by mathwizard, 16 years ago

Owner: changed from anonymous to mathwizard
Status: assignednew

by mathwizard, 16 years ago

Attachment: dictsort.patch added

Patch against current svn patching dictsort and dictsortreversed filters

comment:4 by Michael Axiak, 16 years ago

Needs documentation: set
Owner: changed from mathwizard to Michael Axiak
Status: newassigned
Triage Stage: UnreviewedDesign decision needed

The current patch does not work in python 2.3 and unnecessarily imports operator. I'm uploading a new patch to address those issues.

Also, I'm not sure the benefits of this patch outweigh the backwards incompatible nature of this patch. I'm making it design decision needed until a core dev weighs in on it.

by Michael Axiak, 16 years ago

Attachment: 6904_revision7393.diff added

New patch that works in python 2.3

comment:5 by Michael Axiak, 16 years ago

Whoops...there are two things that are needed for this patch:

  1. Tests
  2. For it to work sanely for non-string objects (there's no guarantee that they are strings).

I am attaching a patch that addresses these two issues.

by Michael Axiak, 16 years ago

Better patch...

comment:6 by Michael Axiak, 16 years ago

Owner: changed from Michael Axiak to nobody
Status: assignednew

comment:7 by mathwizard, 16 years ago

Thanks for vastly improving the patch. I think this feature is good way to go, cause usually when you are searching for something on a page, you don't expect, that items beginning with small letter 'a' will be somewhere down the list instead of being at the top. And the filter IS used to sort things that are being displayed on a page (it's a template filter). So from the perspective of the user this is to good way to go.

in reply to:  7 comment:8 by Waylan Limberg, 14 years ago

Replying to mathwizard:

you don't expect, that items beginning with small letter 'a' will be somewhere down the list instead of being at the top.

Not necessarily. For example *nix file systems are case sensitive and if I am displaying a dict of files and directories (simulating a desktop file list), then they should be sorted case sensitive - the way the filter works now.

The point is, there is a use case for both types of sorting. Perhaps a new separate filter should be created to do case incentive sorting or a switch could be provided for the existing filter to turn case-insensitive sorting on and off. However, changing the default behavior of the existing filter is a bad idea. It could create some unexpected (an undesirable) results for existing sites when they update to the latest version of Django.

comment:9 by Michael P. Jung, 14 years ago

Cc: mpjung@… added
Patch needs improvement: set

Please don't just lower() the strings when sorting them. Use the collation of the locale.

http://groups.google.com/group/django-developers/browse_thread/thread/a6fa8df5787b9c32

comment:10 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

I'd call this a bug, from a user perspective. Potentially backwards incompatible but probably not a big deal.

comment:11 by anonymous, 13 years ago

Easy pickings: unset
UI/UX: unset

comment:12 by Carl Meyer, 13 years ago

Triage Stage: Design decision neededAccepted
Type: BugNew feature

I don't think case-sensitive ordering is so obviously wrong in all cases that we can just change the behavior backwards-incompatibly and force anyone who wants the current behavior to implement their own filter.

Accepting this on the basis that it would be reasonable to add something like "idictsort", since case-insensitive would frequently be preferable for strings. It would also be worth looking in more detail at whether there are any downsides to using locale collation for this new tag rather than simple .lower(), as Michael Jung suggests.

comment:13 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added

For what it's worth, it's already possible to do dictsort:'foo.lower' so I'm -1 on introducing a new tag.

comment:14 by Tim Graham, 11 years ago

Resolution: wontfix
Status: newclosed

Agreed with bmispelon. If there's a case where foo.lower won't work, please re-open.

comment:15 by Alex-siem, 21 months ago

Resolution: wontfix
Status: closednew

Reopening, because as of 4.0.1 dictsort:'foo.lower' is not possible anymore, due to CVE-2021-45116

comment:16 by Mariusz Felisiak, 21 months ago

Has patch: unset
Needs documentation: unset
Patch needs improvement: unset
Resolution: wontfix
Status: newclosed

The current thread is to keep Django a core framework, not providing every utility which might be useful. You can write that filter in your own library, e.g.

@register.filter(is_safe=False)
def dictsortlower(value):
    return sorted(value, key=str.lower)

You can start a discussion on DevelopersMailingList if you don't agree.

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