drive-by comments on json-delay.js
http://codereview.chromium.org/93066/diff/1/12 File src/json-delay.js (right): http://codereview.chromium.org/93066/diff/1/12#newcode40 Line 40: // include them in the reg exps in all places where whitespace is allowed. Is this our comment? Since Irregexp does include \u2028 and \u2029 in \s, we shouldn't need to add it (as in [\s\u2028\u2029] below). http://codereview.chromium.org/93066/diff/1/12#newcode165 Line 165: indent = stepback; indent is a local variable here, so setting it just before returning won't do anything. This line can be removed. The intended behavior is achieved anyway, since the calling function's indent hasn't been modified. http://codereview.chromium.org/93066/diff/1/12#newcode179 Line 179: if (ObjectHasOwnProperty.call(replacer, i)) { The specification says that the properties of replacer must be iterated in ascending numeric order. (Actually, it doesn't, but it does explicitly say that the list of elements should be in ascending order, and then that something must be done for each element in the list, which suggests that it should be done in the same order). http://codereview.chromium.org/93066/diff/1/12#newcode185 Line 185: member += " "; Should be on previous line (or indented and put in squiggly brackets). http://codereview.chromium.org/93066/diff/1/12#newcode216 Line 216: indent = stepback; Again unnecessary. http://codereview.chromium.org/93066 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
