Code

Opened 7 years ago

Closed 7 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
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 7 years ago.
Patch to add content_type argument without breaking mimetype

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Simon Willison

Patch to add content_type argument without breaking mimetype

comment:1 Changed 7 years ago by Simon Willison

  • Has patch set
  • Needs documentation set
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 7 years ago by Simon Willison

  • Needs tests set

comment:3 Changed 7 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 7 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

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 7 years ago by mtredinnick

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

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

  • Triage Stage changed from Accepted to Ready for checkin

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

  • Needs tests unset

Want to write a few lines for the docs too?

comment:8 Changed 7 years ago by mtredinnick

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 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.