Benjamin Peterson <benja...@python.org> added the comment: 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 ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue3672> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
