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.

Sign up for GitHub

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

Jump to bottom

Ignore HCR Failures for current session #347 #558

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

Ignore HCR Failures for current session #347 #558

wants to merge 2 commits into from
+73 −25

Conversation

SougandhS
Copy link
Contributor

@SougandhS SougandhS commented Nov 11, 2024 •
edited
Loading

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
image

image

What it does

How to test

Author checklist

@SougandhS
Copy link
Contributor Author

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

@jukzi
Copy link
Contributor

jukzi commented Nov 11, 2024

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

@SougandhS
Copy link
Contributor Author

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

Yeah sure, no problem 👍

@SougandhS SougandhS force-pushed the HCR_Ignore_this_session branch 2 times, most recently from d780561 to 1a41681 Compare November 11, 2024 12:37
@eclipse-jdt-bot
Copy link
Contributor

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

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
Copy link
Contributor Author

Hi @jukzi , can you try committing again ?

iloveeclipse
iloveeclipse requested changes Nov 27, 2024
View reviewed changes
org.eclipse.jdt.debug/model/org/eclipse/jdt/internal/debug/core/model/JDIDebugTarget.java Outdated
/**
* Preference storage for active debug session.
*/
public static final Map<String, Object> debugSessionVariables = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will remove static then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@SougandhS SougandhS force-pushed the HCR_Ignore_this_session branch 3 times, most recently from fcb4123 to 61c6b16 Compare November 27, 2024 11:36
@SougandhS
Ignore HCR Failures for current session eclipse-jdt#347
fe1273b 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jukzi jukzi jukzi left review comments

@iloveeclipse iloveeclipse Awaiting requested review from iloveeclipse

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Please provide a "Ignore in this session" option if hot-code replace fails
4 participants
@SougandhS @jukzi @eclipse-jdt-bot @iloveeclipse