Looking good so far. Got a few comments. Also, test case would be great.
Again, sorry about being so slow on code reviews!
https://codereview.chromium.org/16093040/diff/1007/src/debug-debugger.js
File src/debug-debugger.js (right):
https://codereview.chromium.org/16093040/diff/1007/src/debug-debugger.js#newcode280
src/debug-debugger.js:280: other_script.id, this.line_, this.column_,
this.groupId_, this.statement_aligned_);
80 char limit
https://codereview.chromium.org/16093040/diff/1007/src/debug-debugger.js#newcode447
src/debug-debugger.js:447: var actual_position =
%SetScriptBreakPoint(script, position, this.statement_aligned_,
break_point);
80 char limit
https://codereview.chromium.org/16093040/diff/1007/src/debug-debugger.js#newcode810
src/debug-debugger.js:810: opt_condition, opt_groupId,
opt_statement_aligned) {
80 char limit
https://codereview.chromium.org/16093040/diff/1007/src/debug-debugger.js#newcode813
src/debug-debugger.js:813: opt_condition, opt_groupId,
opt_statement_aligned);
80 char limit
https://codereview.chromium.org/16093040/diff/1007/src/debug.cc
File src/debug.cc (right):
https://codereview.chromium.org/16093040/diff/1007/src/debug.cc#newcode239
src/debug.cc:239: bool statement_alighned) {
Change variable name to "statement_aligned".
Suggestion: I'd prefer introducing an enum for better redability.
Something like
enum PositionAlignment {
STATEMENT_ALIGNED = 0,
BREAK_POSITION_ALIGNED = 1
};
The advantage is that the use sites would read like
it.FindBreakLocationFromPosition(
*source_position, STATEMENT_ALIGNED);
Of course you would have to do that in javascript as well. You could add
those enum values as constants to macros.py.
https://codereview.chromium.org/16093040/diff/1007/src/debug.cc#newcode247
src/debug.cc:247: if (statement_alighned) {
You could use a ternary operator to simplify this
int next_position = statement_aligned ? this->statement_position()
: this->position();
https://codereview.chromium.org/16093040/diff/1007/src/debug.cc#newcode1199
src/debug.cc:1199: bool statement_alighned) {
Ditto.
https://codereview.chromium.org/16093040/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
