#13828 closed (fixed)
Refactor `dictsort` and `dictsortreversed` to make them more DRY
Reported by: | Satoru Logic | Owned by: | Satoru Logic |
---|---|---|---|
Component: | Template system | Version: | 1.2 |
Severity: | Keywords: | defaultfilters | |
Cc: | satorulogic@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In django.template.defaultfilters
, dictsort
and dictsortreversed
are almost the same except for the invocation of reverse()
on the sorted list in the later function.
Now I have defined another function _dictsort(value, arg, reverse=False)
, and make dictsort
and dictsortreversed
call it.
Attachments (3)
Change History (9)
by , 14 years ago
Attachment: | 13828_dry_dictsort.diff added |
---|
comment:1 by , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 14 years ago
by , 14 years ago
Attachment: | 13828_dry_dictsort_2.diff added |
---|
comment:3 by , 14 years ago
I've attached a patch that is the same but with the diff generated from the root directory.
I can also confirm that the patch cleanly applies to trunk and tests pass.
comment:4 by , 14 years ago
Summary: | [patch] Refactor `dictsort` and `dictsortreversed` to make them more DRY → Refactor `dictsort` and `dictsortreversed` to make them more DRY |
---|
Both functions turn into one-liners in Python 2.4, so a common function isn't needed. The previous patch also had a subtle change in behavior: the original list was modified by use of filter. Current patch returns a new list instead. Also modified tests to explicitly check that the original list isn't modified.
PS. There already is a "has patch" flag. You don't need to write "patch" in the topic.
comment:5 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
It appears the diff attached was created from the template directory. diff's should be created against the base django directory.