Bugfix for #518 (Setting a breakpoint inside lambda with object). #523
Conversation
|
Maybe it would be good to extend the existing tests for this case. |
|
Review should consider this: |
|
please add a junit test. see |
|
Hi, @jukzi, I would try to craft a PR with a JUnit test but first I need a hint to make my Eclipse SDK working again. My initial setup with OOMPH was fine and I used it for several months. Now I have updated the SDK with the "Perform setup tasks" feature. After this I now have zillions of compile errors in my SDK because all/many of the external dependencies like JSoup are missing. How can I restore/recreate all these dependencies to get rid of the compile errors? Thanks in advance. |
|
When i have such problems it helps to 1. pull all repositories, 2. use the "edit" button of the eclipse-sdk target platform to update it. 3. reload the active target platform. 4. rebuild all projects. Don't know why such is not done automatically @merks |
|
Help -> Perform Setup Tasks... should generally update everything after pulling all the git repositories. The *.target file should be resolved automatically by those steps, but for unknown reasons (in PDE) that doesn't always work. :-( |
|
Thanks for the quick response with valuable hints from experienced Eclipse developers. But they have produced some big question marks on my face. As you can see I need some more detailed info. Let's go through the steps.
|
|
windows/preference: |
|
Thanks. This procedure helped me out and resolved my compile errors. :) |
|
Now I added a JUnit test. |
jukzi
force-pushed
the
518-breakpoint-inside-lambda-with-object
branch
from
a309be9 to
ccedd54
Compare
| @@ -0,0 +1,42 @@ | |||
| /******************************************************************************* | |||
| * Copyright (c) 2024 IBM Corporation and others. | |||
There was a problem hiding this comment.
typically use your employer here
There was a problem hiding this comment.
There is an employer but for Eclipse I work in my free time as private person.
There was a problem hiding this comment.
Then you can use your private name here
....debug.tests/tests/org/eclipse/jdt/debug/tests/breakpoints/TestToggleBreakpointsTarget8.java
Show resolved
Hide resolved
| @@ -1066,10 +1066,16 @@ public boolean visit(LambdaExpression node) { | |||
|
|
|||
| } | |||
| } | |||
|
|
|||
| // Lambda body can be a block (which is handled above) or an (arbitrary) expression. So maybe the | |||
| // following cases are insufficient and should be replaced by handling the general | |||
There was a problem hiding this comment.
sounds odd to know what todo and not doing it ;-) could you instead of a comment suggest such an appropriate fix?
There was a problem hiding this comment.
Well, I am not 100% sure if the extended version of the fix would be OK. Maybe the gurus for Java grammar and AST can give a short advice here. @stephan-herrmann ? But what is clear according to my tests is that the fix "as is" solves the described bug.
There was a problem hiding this comment.
Right, just handling the two specified alternatives Block and Expression sounds like the most consistent approach.
Seeing other visit methods freely invoking .accept(this) on general expressions, I see no reason why this should cause problems here, but I haven't studied class ValidBreakpointLocationLocator in more detail.
| } else if (body instanceof LambdaExpression) { | ||
| body.accept(this); | ||
| } else if (body instanceof MethodInvocation) { | ||
| body.accept(this); | ||
| } else if (body instanceof ClassInstanceCreation) { |
There was a problem hiding this comment.
since the handling is the same may be use OR in the condition or even a general "instanceof Expression"
There was a problem hiding this comment.
Of course I can implement a OR in the condition and adapt the PR accordingly.
|
@nettozahler ping can you get this PR done, please? |
|
OK, let's try to get it done... I will add some comments. |
nettozahler
force-pushed
the
518-breakpoint-inside-lambda-with-object
branch
from
ccedd54 to
7cebcbd
Compare
|
Now let's try it with the revised PR. |
jukzi
force-pushed
the
518-breakpoint-inside-lambda-with-object
branch
from
7cebcbd to
5420f12
Compare
|
@nettozahler many thanks for fixing this bug, such work is much more important in my opinion than all darkmode, icon, whatever style changes even though you sadly won't get that much attention for your change :-) I can easily work with an "oldish" UI if it works and help me accomplish my tasks! |
|
@nettozahler thanks for contributing |
|
As an Eclipse user for many years (since the days of VisualAge - if anybody remembers this) it feels right to "give something back". And the competition with other IDEs is hard so I think we need all contributions: the ones for a more fancy UI as well as those for even better functionality. I would also like to say my thanks for your outstanding support especially in the performance and memory consumption areas. |
What it does
Fixes #518
How to test
Try to set a breakpoint inside a lambda expression with its body beginning with an anonymous class declaration.
See #518 as example. The new breakpoint should appear as usual and jumping to it from inside the "breakpoint view" must work.
Author checklist