Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add support for lambda breakpoints #427
Add support for lambda breakpoints #427
Changes from 3 commits
438078d
9b001d7
ec133c3
6dcb88d
aec3abe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 only works for expression-style lambda with 2 lines.
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 tested for
and
Its working, can you give a example snippet ?
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.
Case 1:
This will hit lambda entry on line 21.
Case 2:
This will hit lambda entry on line 22.
It is not accurate to compare the column number for multi-line lambda. It might be better to compare the offset.
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.
True. But I couldn’t find a method to calculate offset by providing the line and column. But will check more to see if i have missed any such method. JDT should have such methods, not sure if they are part of the API.
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.
You can use
this.compilationUnit#getPosition(line, column)
to get the offset.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.
Thanks. I think i missed it. The problem with current condition is I didn’t intend to support blocks. But still if we only do boundry check and ignore middle lines it will work when we have multi lines. But position is much simpler. I will switch to that.
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.
For the scenario 1 as user what you would expect ?
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 expect adding a breakpoint on (Line 22, Column 24), and if Column 24 breakpoint is not supported, then fallback to a line breakpoint on Line 22.
If it's inside a block body, I'll treat it as a normal function body and not fall back to the Lambda entry breakpoint.
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.
LambdaExpression.getBody() will return either a Block or an Expression, which we can use to determine which lambda it is.
If it's an expression-style lambda, it's ok to enable Lambda breakpoint if the offset is within the whole LambdaExpression range.
If it's a block-style lambda, I would enable Lambda breakpoint only if the offset is in [LambdaStart, LambdaBodyStart).
Just my two cents.
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.
Now keeping mind the future expansion of inline breakpoints for method invocations, I decided to restrict the lambda breakpoints by
This way we will not break any functionality nor complicate breakpoint logic in future when we introduce method invocation inline breakpoints. And for lambda blocks we can always use line breakpoints which is much clear and straight forward.
WDYT ?