Skip to content
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

Clean-up LaunchManager attempt 2 #1618

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link
Member

Second attempt of #1487 after it was reverted in #1616.

This PR is split into two commits, the first one only containing clean-ups that should not change any logic. The second commit defers obtaining the IResource of a delta as long as possible.

Implement the IResourceDeltaVisitor in LaunchManager as lambda's
instead of nested classes and more.
Defer obtaining a delta's resource as long as possible and instead only
on the full-path of the delta.
Copy link
Contributor

Test Results

 1 755 files  ±0   1 755 suites  ±0   1h 49m 31s ⏱️ + 10m 36s
 4 170 tests ±0   4 146 ✅  - 2   22 💤 ±0  2 ❌ +2 
13 107 runs  ±0  12 937 ✅  - 6  164 💤 ±0  6 ❌ +6 

For more details on these failures, see this check.

Results for commit 0d45dcf. ± Comparison against base commit 6dd6732.

@trancexpress
Copy link
Contributor

trancexpress commented Nov 15, 2024

The problem is still seen. This "fixes" the bug:

diff --git a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
index 80a62dfea1..35228b29ad 100644
--- a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
+++ b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
@@ -551,8 +551,8 @@ public class LaunchManager extends PlatformObject implements ILaunchManager, IRe
                        }
                        return false;
                }
-               if (LAUNCH_CONFIG_FILE_EXTENSIONS.contains(delta.getFullPath().getFileExtension())) {
-                       if (delta.getResource() instanceof IFile file) {
+               if (delta.getResource() instanceof IFile file) {
+                       if (LAUNCH_CONFIG_FILE_EXTENSIONS.contains(delta.getFullPath().getFileExtension())) {
                                ILaunchConfiguration handle = new LaunchConfiguration(file);
                                switch (delta.getKind()) {
                                        case IResourceDelta.ADDED:

I'm not exactly sure why, probably the return false statement in the outer if block now does something differently.

Here are the reproduction steps:

  1. Create a Java project.
  2. Create a folder .launch in the project.
  3. Create a snippet with a main() method.
  4. Run the snippet.
  5. Go into the launch config dialog for the new launch (of the snippet created above).
  6. Go to the Common tab, make the launch shared, the directory of the shared launch file is .launch.
  7. Delete the shared launch file on command line.
  8. Refresh the project.
  9. Observe that the launch config dialog still lists the launch for the snippet, it should not.

Once you have the project and snippet, you can repeat only 4. and onward.

Also recorded:

platform_gh1618_reproduction_steps.webm

@iloveeclipse
Copy link
Member

May be this PR should have a test case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants