Opened 10 years ago

Closed 9 years ago

#3526 closed (fixed)

HttpResponse object should have 'content_type' argument (in additon to / instead of 'mimetype')

Reported by: Simon Willison Owned by: Adrian Holovaty
Component: Core (Other) Version: master
Severity: Keywords: httpresponse
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I can never remember the 'mimetype' argument to HttpResponse; I always have to look it up. The corresponding HTTP header is 'Content-Type'; I think that content_type would be a more memorable and more accurate argument.

For example:

return HttpResponse(simplejson.dumps({
    'airports': airports
}), mimetype='application/json; charset=utf-8')

Should be:

return HttpResponse(simplejson.dumps({
    'airports': airports
}), content_type='application/json; charset=utf-8')

The charset=utf-8 bit further demonstrates that mimetype should be renamed - charset=utf-8 is a parameter, not part of the actual mime media type.

I suggest adding content_type as an alternative to mimetype; I see no particular need to deprecate mimetype and break existing code, although it may be something that should be removed in the future.

Attachments (1)

content_type.diff (1.3 KB) - added by Simon Willison 10 years ago.
Patch to add content_type argument without breaking mimetype

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by Simon Willison

Attachment: content_type.diff added

Patch to add content_type argument without breaking mimetype

comment:1 Changed 10 years ago by Simon Willison

Has patch: set
Needs documentation: set

comment:2 Changed 10 years ago by Simon Willison

Needs tests: set

comment:3 Changed 10 years ago by Chris Beaven

Triage Stage: UnreviewedDesign decision needed

comment:4 Changed 10 years ago by Malcolm Tredinnick

Triage Stage: Design decision neededAccepted

This looks reasonable. However, isn't the patch broken? If you pass in mimetype, but not content_type, then lines 162 and 163 will stomp all over line 161 in the new version. Looks like 163 should be "elif". Otherwise, looks like the right idea.

comment:5 Changed 10 years ago by Malcolm Tredinnick

OK, I'm an idiot. The patch is not broken. Nothing to see here.

comment:6 Changed 9 years ago by Simon G. <dev@…>

Triage Stage: AcceptedReady for checkin

comment:7 Changed 9 years ago by Simon G. <dev@…>

Needs tests: unset

Want to write a few lines for the docs too?

comment:8 Changed 9 years ago by Malcolm Tredinnick

The only change I made in the version I'm about to commit is to move the content_type parameter to the end of the constructor list to preserve backwards compatibility for people using positional arguments.

comment:9 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [5844]) Fixed #3526 -- Added content_type as an alias for mimetype to the HttpResponse constructor. It's a slightly more accurate name. Based on a patch from Simon Willison. Fully backwards compatible.

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