Opened 18 years ago

Closed 18 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: dev
Severity: Keywords: httpresponse
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:How to create a pull request

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.

Change History (10)

by Simon Willison, 18 years ago

Attachment: content_type.diff added

Patch to add content_type argument without breaking mimetype

comment:1 by Simon Willison, 18 years ago

Has patch: set
Needs documentation: set

comment:2 by Simon Willison, 18 years ago

Needs tests: set

comment:3 by Chris Beaven, 18 years ago

Triage Stage: UnreviewedDesign decision needed

comment:4 by Malcolm Tredinnick, 18 years ago

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 by Malcolm Tredinnick, 18 years ago

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

comment:6 by Simon G. <dev@…>, 18 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Simon G. <dev@…>, 18 years ago

Needs tests: unset

Want to write a few lines for the docs too?

comment:8 by Malcolm Tredinnick, 18 years ago

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 by Malcolm Tredinnick, 18 years ago

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