Message 79054 - Python tracker (original) (raw)
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"?