LGTM. My main comment is that I think switch statements would make some of the javascript code more readable.
http://codereview.chromium.org/14509/diff/1/4 File src/d8-debug.cc (right): http://codereview.chromium.org/14509/diff/1/4#newcode76 Line 76: Handle<Value> request = Shell::DebugCommandToJSONRequest(String::New(command)); Too long line? http://codereview.chromium.org/14509/diff/1/4#newcode90 Line 90: // All the functions used below takes one argument. takes -> take http://codereview.chromium.org/14509/diff/1/7 File src/d8.cc (right): http://codereview.chromium.org/14509/diff/1/7#newcode103 Line 103: // When debugging make exceptions appear to by uncaught. by -> be http://codereview.chromium.org/14509/diff/1/5 File src/d8.js (right): http://codereview.chromium.org/14509/diff/1/5#newcode203 Line 203: return SourceUnderline(location.sourceText(), location.position - location.start); Is this line too long? http://codereview.chromium.org/14509/diff/1/5#newcode237 Line 237: // Switch on command. This might be more readable if you use an actual switch statement? http://codereview.chromium.org/14509/diff/1/5#newcode337 Line 337: if (args[0] == 'in' || args[0] == 'i') { Use a switch on args[0]? http://codereview.chromium.org/14509/diff/1/5#newcode456 Line 456: if (args[0] == 'natives') { switch? http://codereview.chromium.org/14509/diff/1/5#newcode555 Line 555: if (response.command == 'setbreakpoint') { switch? http://codereview.chromium.org/14509/diff/1/9 File src/debug.cc (right): http://codereview.chromium.org/14509/diff/1/9#newcode1379 Line 1379: if (Debug::InDebugger()) return; This looks like an accidental edit? http://codereview.chromium.org/14509 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
