-
Notifications
You must be signed in to change notification settings - Fork 686
Multiple nexts with one command #2207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple nexts with one command #2207
Conversation
| """ Next breakpoint in the same context """ | ||
| self.exec_command(args, JERRY_DEBUGGER_NEXT) | ||
| if args == "": | ||
| args = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preferred way to check that a string is empty (if you know that your variable is a string) is this: if not args:
If your variable could also be some other type then you should use this form if args == "":, but as I saw in the following code, it always waiting for string and then you cast that into an integer.
| args = min(args, len(self.debugger.last_breakpoint_hit.function.lines) - | ||
| self.debugger.last_breakpoint_hit.function.line) - 1 | ||
| self.debugger.nexts_remain = args | ||
| except ValueError as val_errno: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it's necessary to put it all in a try block, because you only use cast in the first command.
jerry-debugger/jerry-client-ws.py
Outdated
| try: | ||
| args = int(args) | ||
| args = min(args, len(self.debugger.last_breakpoint_hit.function.lines) - | ||
| self.debugger.last_breakpoint_hit.function.line) - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition can not be negative here?
| self.debugger.last_breakpoint_hit.function.line) - 1 | ||
| self.debugger.nexts_remain = args | ||
| except ValueError as val_errno: | ||
| print("Error: expected a positive integer: %s" % val_errno) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about those cases when the args value is a zero or negative number?
06de0f3 to
40ee022
Compare
|
@robertsipka I've updated the PR, thank you for the review. |
jerry-debugger/jerry-client-ws.py
Outdated
| self.src_offset_diff = 0 | ||
| self.client_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
| self.client_socket.connect((self.host, self.port)) | ||
| self.nexts_remain = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this as repeat, so we could extend this to step etc. later.
40ee022 to
23db6f5
Compare
Make a next command more gdb like. If an argument is given `next 10`, it does 10 nexts. JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu
23db6f5 to
e292e2a
Compare
zherczeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LaszloLango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Make a next command more gdb like.
If an argument is given
next 10, it does 10 nexts.JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu