Ignore HCR Failures for current session #347 #558

SougandhS · 2024-11-11T11:38:53Z

Enhancement : #347

Adds a checkbox to the HCR error alert box that allows users to ignore HCR failures only for the current debug session, rather than applying it to all future debug sessions.

Fixes : #347

What it does

How to test

Author checklist

I have thoroughly tested my changes
The change is following the coding conventions
I have signed the Eclipse Contributor Agreement (ECA)

SougandhS · 2024-11-11T12:08:54Z

Hi @jukzi ,
Could u please check this too when you are free.

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/JavaHotCodeReplaceListener.java

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/ErrorDialogWithToggle.java

jukzi · 2024-11-11T12:14:21Z

As this seems to be new feature can it wait for M1?

SougandhS · 2024-11-11T12:17:16Z

As this seems to be new feature can it wait for M1?

Yeah sure, no problem 👍

eclipse-jdt-bot · 2024-11-26T14:05:26Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

org.eclipse.jdt.debug.ui/META-INF/MANIFEST.MF
org.eclipse.jdt.debug.ui/pom.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 0349b257fde13d14d686530066272b272ced6329 Mon Sep 17 00:00:00 2001
From: Eclipse JDT Bot <[email protected]>
Date: Wed, 27 Nov 2024 11:45:24 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/org.eclipse.jdt.debug.ui/META-INF/MANIFEST.MF b/org.eclipse.jdt.debug.ui/META-INF/MANIFEST.MF
index e651afa8a..06269ace2 100644
--- a/org.eclipse.jdt.debug.ui/META-INF/MANIFEST.MF
+++ b/org.eclipse.jdt.debug.ui/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jdt.debug.ui; singleton:=true
-Bundle-Version: 3.13.600.qualifier
+Bundle-Version: 3.13.700.qualifier
 Bundle-Activator: org.eclipse.jdt.internal.debug.ui.JDIDebugUIPlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
diff --git a/org.eclipse.jdt.debug.ui/pom.xml b/org.eclipse.jdt.debug.ui/pom.xml
index dceebf96d..597b96d81 100644
--- a/org.eclipse.jdt.debug.ui/pom.xml
+++ b/org.eclipse.jdt.debug.ui/pom.xml
@@ -18,7 +18,7 @@
   </parent>
   <groupId>org.eclipse.jdt</groupId>
   <artifactId>org.eclipse.jdt.debug.ui</artifactId>
-  <version>3.13.600-SNAPSHOT</version>
+  <version>3.13.700-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
   <properties>
   	<defaultSigning-excludeInnerJars>true</defaultSigning-excludeInnerJars>
-- 
2.47.0

Further information are available in Common Build Issues - Missing version increments.

SougandhS · 2024-11-27T06:22:14Z

Hi @jukzi , can you try committing again ?

iloveeclipse · 2024-11-27T06:26:06Z

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java

+	/**
+	 * Preference storage for active debug session.
+	 */
+	public static final Map<String, Object> debugSessionVariables = new ConcurrentHashMap<>();


Wjy is this a static map and why it is a map at all? If with the the "session" the whole Eclipse session meant, no map is needed, a Boolean flag will be enough. If with the "session" a specific debug session meant, the code is plain wrong as it would affect all other debug sessions runnijg in parallel or after the current one.

This can be used to store pref values only for current debug session. other pref values can also be added.

other pref values in the sense other features preferences value only for current debug session in future if it requires.

This can be used to store pref values only for current debug session

Then a static map is wrong. Try to start two sessions in parallel.

other pref values in the sense other features preferences value only for current debug session in future if it requires.

If future requires, the code can be refactored.

will remove static then.

other pref values in the sense other features preferences value only for current debug session in future if it requires.

If future requires, the code can be refactored.

will replace it with boolean flag as you suggested since its more appropriate.

I wasnt able to check without static but now I have modified it further to make it more debug sessions specific.

Tested with parallel debug launches

iloveeclipse · 2024-12-03T16:07:19Z

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/ErrorDialogWithToggle.java

@@ -43,25 +47,37 @@ public class ErrorDialogWithToggle extends ErrorDialog {
 	/**
 	 * The message displayed to the user, with the toggle button
 	 */
-	private String fToggleMessage= null;
+	private String fToggleMessage1 = null;


Style: please don't initialize fields to default values, java does it automatically, but during debugging you will jump from field to field just because of this meaningless instructions here.

will remove default initialization

iloveeclipse · 2024-12-03T16:11:15Z

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/ErrorDialogWithToggle.java

 		fStore.setValue(fPreferenceKey, !getToggleButton().getSelection());
+		if (fToggleButton2 != null) {
+			try {
+				JDIDebugTarget.hcrDebugSessionErrors.put(target.getName(), fToggleButton2.getSelection());


This is inappropriate place / dependency. The ErrorDialogWithToggle has no relationship to JDIDebugTarget and shouldn't have.

The HotCodeReplaceErrorDialog class that extends this dialog is the right place for HCR related code.

Yes, but I couldn't find any ways to access same from JavaHotCodeReplaceListener class, which checks the boolean value.

for every HCR failures a new instance of HotCodeReplaceErrorDialog created for the current launch, so JDIDebugTarget is leveraged for checking the boolean as it is same for the launches

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/ErrorDialogWithToggle.java

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java

SougandhS · 2025-01-13T07:09:47Z

Hi @iloveeclipse, could u please check this again when u have time ?

SougandhS · 2025-01-13T09:24:30Z

Is it possible to include this in M2 ?

Adds a checkbox to the HCR error alert box that allows users to ignore HCR failures only for the current debug session, rather than applying it to all future debug sessions. Fixes : eclipse-jdt#347

please update review if you still plan to finish review

jukzi reviewed Nov 11, 2024

View reviewed changes

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/JavaHotCodeReplaceListener.java Outdated Show resolved Hide resolved

jukzi reviewed Nov 11, 2024

View reviewed changes

org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java Outdated Show resolved Hide resolved

jukzi reviewed Nov 11, 2024

View reviewed changes

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/ErrorDialogWithToggle.java Outdated Show resolved Hide resolved

SougandhS force-pushed the HCR_Ignore_this_session branch 2 times, most recently from d780561 to 1a41681 Compare November 11, 2024 12:37

jukzi force-pushed the HCR_Ignore_this_session branch from 1a41681 to 34717d0 Compare November 26, 2024 14:03

iloveeclipse requested changes Nov 27, 2024

View reviewed changes

SougandhS force-pushed the HCR_Ignore_this_session branch 4 times, most recently from 024d81c to fe1273b Compare November 27, 2024 11:43

SougandhS requested a review from iloveeclipse November 27, 2024 12:40

SougandhS force-pushed the HCR_Ignore_this_session branch from ba0149c to 42d462f Compare December 3, 2024 06:27

iloveeclipse reviewed Dec 3, 2024

View reviewed changes

iloveeclipse previously requested changes Dec 3, 2024

View reviewed changes

SougandhS force-pushed the HCR_Ignore_this_session branch 2 times, most recently from 47e421d to c40c922 Compare December 5, 2024 10:00

SougandhS requested a review from iloveeclipse December 5, 2024 11:38

SougandhS force-pushed the HCR_Ignore_this_session branch from c40c922 to ce76980 Compare January 1, 2025 02:50

SougandhS force-pushed the HCR_Ignore_this_session branch from ce76980 to d42cd43 Compare January 13, 2025 07:08

Ignore HCR Failures for current session eclipse-jdt#347

d0cc028

Adds a checkbox to the HCR error alert box that allows users to ignore HCR failures only for the current debug session, rather than applying it to all future debug sessions. Fixes : eclipse-jdt#347

SougandhS force-pushed the HCR_Ignore_this_session branch from d42cd43 to d0cc028 Compare January 15, 2025 06:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ignore HCR Failures for current session #347 #558

Ignore HCR Failures for current session #347 #558

SougandhS commented Nov 11, 2024 •

edited

Loading

SougandhS commented Nov 11, 2024

jukzi commented Nov 11, 2024

SougandhS commented Nov 11, 2024

eclipse-jdt-bot commented Nov 26, 2024 •

edited

Loading

SougandhS commented Nov 27, 2024

iloveeclipse Nov 27, 2024

SougandhS Nov 27, 2024

SougandhS Nov 27, 2024

iloveeclipse Nov 27, 2024

iloveeclipse Nov 27, 2024

SougandhS Nov 27, 2024

SougandhS Nov 27, 2024

SougandhS Nov 27, 2024

iloveeclipse Dec 3, 2024

SougandhS Dec 5, 2024

iloveeclipse Dec 3, 2024

SougandhS Dec 5, 2024

SougandhS Jan 13, 2025

SougandhS commented Jan 13, 2025 •

edited

Loading

SougandhS commented Jan 13, 2025

Ignore HCR Failures for current session #347 #558

Are you sure you want to change the base?

Ignore HCR Failures for current session #347 #558

Conversation

SougandhS commented Nov 11, 2024 • edited Loading

What it does

How to test

Author checklist

SougandhS commented Nov 11, 2024

jukzi commented Nov 11, 2024

SougandhS commented Nov 11, 2024

eclipse-jdt-bot commented Nov 26, 2024 • edited Loading

SougandhS commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SougandhS commented Jan 13, 2025 • edited Loading

SougandhS commented Jan 13, 2025

SougandhS commented Nov 11, 2024 •

edited

Loading

eclipse-jdt-bot commented Nov 26, 2024 •

edited

Loading

SougandhS commented Jan 13, 2025 •

edited

Loading