Martin v. Löwis <mar...@v.loewis.de> added the comment: http://codereview.appspot.com/7311/diff/1/8 File Lib/json/decoder.py (right):
http://codereview.appspot.com/7311/diff/1/8#newcode55 Line 55: def py_scanstring(s, end, encoding=None, strict=True, _b=BACKSLASH, _m=STRINGCHUNK.match): This function should get some comments what all the various cases are (preferably speaking with the terms of JSON spec, i.e. chars, char, ...) http://codereview.appspot.com/7311/diff/1/8#newcode71 Line 71: _append(content) # 3 cases: end of string, control character, escape sequence http://codereview.appspot.com/7311/diff/1/8#newcode76 Line 76: msg = "Invalid control character {0!r} at".format(esc) esc isn't assigned until a few lines later. Is this really correct? http://codereview.appspot.com/7311/diff/1/8#newcode104 Line 104: raise ValueError No message? http://codereview.appspot.com/7311/diff/1/8#newcode107 Line 107: raise ValueError No message? http://codereview.appspot.com/7311/diff/1/8#newcode111 Line 111: m = unichr(uni) What's the purpose of m? http://codereview.appspot.com/7311/diff/1/8#newcode127 Line 127: nextchar = s[end:end + 1] Why not s[end]? Add comment if this is necessary. http://codereview.appspot.com/7311/diff/1/8#newcode132 Line 132: nextchar = s[end:end + 1] Likewise. There are more places where it does slicing, but also places where it does indexing, in this function. http://codereview.appspot.com/7311/diff/1/8#newcode290 Line 290: following strings: -Infinity, Infinity, NaN. This sounds like an incompatible change. http://codereview.appspot.com/7311/diff/1/8#newcode317 Line 317: def raw_decode(self, s, idx=0): That looks like an incompatible change http://codereview.appspot.com/7311/diff/1/9 File Modules/_json.c (right): http://codereview.appspot.com/7311/diff/1/9#newcode196 Line 196: output_size *= 2; You might want to check for integer overflow here. http://codereview.appspot.com/7311/diff/1/9#newcode215 Line 215: ascii_escape_str(PyObject *pystr) Please attach a comment to each function, telling what the function does. http://codereview.appspot.com/7311/diff/1/9#newcode733 Line 733: "..." Some text should probably be added here. http://codereview.appspot.com/7311/diff/1/9#newcode1320 Line 1320: if ((idx + 3 < length) && str[idx + 1] == 'u' && str[idx + 2] == 'l' && str[idx + 3] == 'l') { Is this really faster than a strncmp? http://codereview.appspot.com/7311/diff/1/9#newcode1528 Line 1528: PyTypeObject PyScannerType = { I think scanner objects should participate in cyclic gc. http://codereview.appspot.com/7311/diff/1/9#newcode2025 Line 2025: "make_encoder", /* tp_name */ That is a confusing type name. How about "Encoder"? http://codereview.appspot.com/7311 ---------- title: merge json library with simplejson 2.0.4 -> merge json library with simplejson 2.0.3 _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue4136> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
