Code

Opened 7 years ago

Closed 6 years ago

#4380 closed (wontfix)

Rename DEFAULT_CHARSET to OUTPUT_CHARSET (or similar)

Reported by: Simon Willison Owned by: ctrochalakis
Component: Core (Other) Version: master
Severity: Keywords:
Cc: floguy@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Just spotted this in the new unicode documentation (from the unicode branch):

Do not be fooled into thinking that if you DEFAULT_CHARSET setting is set
to something other than utf-8 you can use that encoding in your
bytestrings! The DEFAULT_CHARSET only applies to the strings generated as
the result of template rendering (and email). Django will always assume UTF-8
encoding for internal bytestrings. The reason for this is that the
DEFAULT_CHARSET setting is not actually under your control (if you are the
application developer). It is under the control of the person installing and
using your application and if they choose a different setting, your code must
still continue to work. Ergo, it cannot rely on that setting.

http://code.djangoproject.com/browser/django/branches/unicode/docs/unicode.txt?rev=5330#L57

How about renaming that setting to something more clear, like OUTPUT_CHARSET or DEFAULT_OUTPUT_CHARSET?

Attachments (4)

default_charset_deprecation.1.diff (12.8 KB) - added by floguy 6 years ago.
Replaced all DEFAULT_CHARSET references with OUTPUT_CHARSET, and added PendingDeprecationWarnings for referencing DEFAULT_CHARSET.
0001-Replace-DEFAULT_CHARSET-with-OUTPUT_CHARSET.patch (13.7 KB) - added by ctrochalakis 6 years ago.
replace-DEFAULT_CHARSET
0001-Replace-DEFAULT_CHARSET-with-OUTPUT_CHARSET.2.patch (14.0 KB) - added by ctrochalakis 6 years ago.
replace-DEFAULT_CHARSET+deprecation warning
output_charset.patch (13.7 KB) - added by ctrochalakis 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Marc Fargas <telenieko@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

+1 at least the setting will be a bit clearer on its purpose ;)
Maybe FILE_CHARSET could be moved to TEMPLATE_CHARSET as it only affects templates ;))

but you'd rather bring that to django-devs ;)

comment:2 Changed 7 years ago by mtredinnick

Well, that'll teach me to document things.

Might be worth changing for all the good reasons you mention. I thought about it, particularly when writing that passage, because it brings the unfortunate name into relief. On a theoretical level, I'm completely in agreement. But I hate making backwards incompatible changes. If we can fake a backwards compatible hack (e.g. populate OUTPUT_CHARSET from DEFAULT_CHARSET if the former doesn't exist in the settings module), I'd be happier. That's probably achievable, without thinking too hard about it.

I'm probably in favour, but I need some time to get used to it. :-)

Best plan of all: convince Adrian and/or Jacob so that I can avoid making a decision.

Marc: FILE_CHARSET was already discussed ad infinitum on django-dev. It's not going to change because it's not just used for template input (and that fact is even documented - look at the documentation for FILE_CHARSET in settings.txt).

comment:3 Changed 7 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

A bit of a wart, but I think I'm vaguely +0 on deprecating DEFAULT_CHARSET and switching to OUTPUT_CHARSET since Malcolm assures me that could be done in a backwards-compatible way.

comment:4 Changed 7 years ago by jacob

  • Version changed from other branch to SVN

comment:5 Changed 6 years ago by floguy

  • Cc floguy@… added
  • Has patch set
  • Needs documentation set
  • Owner changed from nobody to floguy
  • Status changed from new to assigned

Changed 6 years ago by floguy

Replaced all DEFAULT_CHARSET references with OUTPUT_CHARSET, and added PendingDeprecationWarnings for referencing DEFAULT_CHARSET.

Changed 6 years ago by ctrochalakis

replace-DEFAULT_CHARSET

comment:6 Changed 6 years ago by floguy

  • Owner changed from floguy to ctrochalakis
  • Status changed from assigned to new

That second patch is cleaner than my first one.

Changed 6 years ago by ctrochalakis

replace-DEFAULT_CHARSET+deprecation warning

comment:7 Changed 6 years ago by ctrochalakis

Added a deprecation warning as Eric did in the first patch. The warning is triggered if the DEFAULT_CHARSET is set.

comment:8 Changed 6 years ago by mtredinnick

At a minimum there needs be a "New in Django develpoment version" marker in the settings documentation. I'd also leave a docs entry in place for DEFAULT_CHARSET and refer it to OUTPUT_CHARSET with a note that this was the older way of doing it. That way people reading the docs and searching for the setting they have in their code will still find an entry.

comment:9 Changed 6 years ago by mtredinnick

  • Needs documentation unset
  • Patch needs improvement set

Changed 6 years ago by ctrochalakis

comment:10 Changed 6 years ago by ctrochalakis

  • Patch needs improvement unset

Added a "new in development version" marker. Also, DEFAULT_CHARSET section in retained in the settings.txt file, and refers to OUTPUT_CHARSET.

comment:11 Changed 6 years ago by mtredinnick

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

Not worth it now. We're going to have to support DEFAULT_CHARSET for a long time now that 1.0 is coming out and it doesn't seem to be causing a lot of confusion to people. Yes, I probably should have just fixed this when it was first raised, but I didn't. Sorry, but can't go back in time (time machine is only for bug fixes, not a cure for sloth).

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.