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