Author benjamin.peterson
Recipients Rhamphoryncus, benjamin.peterson, ezio.melotti, jwilk, lemburg, loewis, pitrou
Date 2009-05-01.21:30:47
SpamBayes Score 1.58247e-06
Marked as misclassified No
Message-id <1241213449.29.0.821681406925.issue3672@psf.upfronthosting.co.za>
In-reply-to
Content
http://codereview.appspot.com/52081/diff/1/5
File Doc/library/codecs.rst (right):

http://codereview.appspot.com/52081/diff/1/5#newcode326
Line 326: In addition, the following error handlers are specific to only
selected
"In addition, the following error handlers are specific to a single
codec." sounds better

http://codereview.appspot.com/52081/diff/1/5#newcode335
Line 335:
There should probably be a versionchanged directive indicating that
"surrogates" was added in 3.1.

http://codereview.appspot.com/52081/diff/1/6
File Lib/test/test_codecs.py (right):

http://codereview.appspot.com/52081/diff/1/6#newcode544
Line 544: def test_surrogates(self):
I think this should be split into 2 tests. "test_lone_surrogates" and
"test_surrogate_handler".

http://codereview.appspot.com/52081/diff/1/4
File Objects/unicodeobject.c (right):

http://codereview.appspot.com/52081/diff/1/4#newcode157
Line 157: static PyObject *unicode_encode_call_errorhandler(const char
*errors,
These prototypes are longer than 80 chars some places. I don't think the
arguments need to line up with the starting parenthesis.

http://codereview.appspot.com/52081/diff/1/4#newcode2393
Line 2393: s, size, &exc, i-1, i, &newpos);
"exc" is never Py_DECREFed.

http://codereview.appspot.com/52081/diff/1/4#newcode4110
Line 4110: if (!PyUnicode_Check(repunicode)) {
Is there a test of this case somewhere?

http://codereview.appspot.com/52081/diff/1/2
File Python/codecs.c (right):

http://codereview.appspot.com/52081/diff/1/2#newcode758
Line 758: if (PyObject_IsInstance(exc, PyExc_UnicodeEncodeError)) {
I believe PyErr_GivenExceptionMatches is more appropriate here, but
given the rest of the file uses PyObject_IsInstance, it likely doesn't
matter.

http://codereview.appspot.com/52081/diff/1/2#newcode771
Line 771: return NULL;
This is leaks "object".

http://codereview.appspot.com/52081
History
Date User Action Args
2009-05-01 21:30:49benjamin.petersonsetrecipients: + benjamin.peterson, lemburg, loewis, Rhamphoryncus, pitrou, jwilk, ezio.melotti
2009-05-01 21:30:49benjamin.petersonsetmessageid: <1241213449.29.0.821681406925.issue3672@psf.upfronthosting.co.za>
2009-05-01 21:30:48benjamin.petersonlinkissue3672 messages
2009-05-01 21:30:47benjamin.petersoncreate