Georg Brandl <[EMAIL PROTECTED]> added the comment: Review:
* Why is an empty string not printable? In any case, the empty string should be among the test cases for isprintable(). * Why not use PyUnicode_DecodeASCII instead of PyUnicode_FromEncodedObject? It should be a bit faster. * If old-style string formatting gets "%a", .format() must get a "!a" specifier. * The ascii() and repr() tests should be expanded so that both test the same set of objects, and the expected differences. Are there tests for failing cases? * This is just "return ascii" (in builtin_ascii): + if (ascii == NULL) + return NULL; + + return ascii; * For PyBool_FromLong(1) and PyBool_FromLong(0) there is Py_RETURN_TRUE and Py_RETURN_FALSE. (You're not to blame, the rest of unicodeobject.c seems to use them too, probably a legacy.) * There appear to be some space indentations in tab-indented files like bltinmodule.c and vice versa (unicodeobject.c). * C docs/isprintable() docs: The spec + Characters defined in the Unicode character database as "Other" + or "Separator" other than ASCII space(0x20) are not considered + printable. is unclear, better say "All character except those ... are considered printable". * ascii() docs: + the non-ASCII + characters in the string returned by :func:`ascii`() are hex-escaped + to generate a same string as :func:`repr` in Python 2. should be "the non-ASCII characters in the string returned by :func:`repr` are backslash-escaped (with ``\x``, ``\u`` or ``\U``) to generate ...". * makeunicodedata: len(list(n for n in names if n is not None)) could better be expressed as sum(1 for n in names if n is not None). Otherwise, the patch is fine IMO. (I'm surprised that only so few tests needed adaptation, that's a sign that we're not testing Unicode enough.) ---------- nosy: +georg.brandl _______________________________________ Python tracker <[EMAIL PROTECTED]> <http://bugs.python.org/issue2630> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
