Opened 12 years ago
Closed 12 years ago
#20276 closed Cleanup/optimization (fixed)
bool(MergeDict()) should evaluate to False if component dicts are empty
| Reported by: | Tilman Koschnick | Owned by: | nobody |
|---|---|---|---|
| Component: | Utilities | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | antonbaklanov@…, bmispelon@… | 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
Currently, MergeDict instances are considered True, no matter what the contents of the component dicts are. So you could have 'request.GET or request.POST' evaluate to False and 'request.REQUEST' to True on the same request, which is counter-intuitive.
Adding a __nonzero__ method would fix this.
Attachments (1)
Change History (6)
by , 12 years ago
| Attachment: | datastructures.diff added |
|---|
comment:1 by , 12 years ago
| Component: | HTTP handling → Utilities |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 12 years ago
| Cc: | added |
|---|---|
| Version: | 1.5 → master |
comment:3 by , 12 years ago
| Cc: | added |
|---|
The patch looks good (I added a few minor comments to the PR on github).
Before marking this as RFC, I was wondering if there are potential backwards-compatibility issues? Are there places where MergeDict is used that this patch could impact?
Thanks.
comment:4 by , 12 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Grepping through django's code, it seems the MergeDict is only used to build request.REQUEST.
I don't see how this patch could break existing code, so I'm marking it as RFC.
Thanks.
comment:5 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
pull request with py3 support and tests