Disable all conditions in breakpoints #549 #550

SougandhS · 2024-10-29T06:14:30Z

Provides additional right click menu option for removing all the conditions set in breakpoints. Will be useful for clearing all the conditions in breakpoints at once.

Enhancement #549

What it does

Provides additional option to remove all conditions

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)

iloveeclipse

As commented on the ticket, this only adds more complexity to the menu.
I don't see any reason why anyone would need an extra item for removing conditional breakpoints and why it is so important to get it into the menu.

In the opposite, removing al conditional breakpoints is more dangerous as usual ones because often conditions are non trivial and if lost, need much more time to restore compared to simple breakpoints that can be togled by a single click.

So definitely no.

SougandhS · 2024-10-29T06:35:18Z

As commented on the ticket, this only adds more complexity to the menu. I don't see any reason why anyone would need an extra item for removing conditional breakpoints and why it is so important to get it into the menu.

In the opposite, removing al conditional breakpoints is more dangerous as usual ones because often conditions are non trivial and if lost, need much more time to restore compared to simple breakpoints that can be togled by a single click.

So definitely no.

Okay @iloveeclipse will cancel this then. Thank you

SougandhS · 2024-10-29T06:54:11Z

As commented on the ticket, this only adds more complexity to the menu. I don't see any reason why anyone would need an extra item for removing conditional breakpoints and why it is so important to get it into the menu.

In the opposite, removing al conditional breakpoints is more dangerous as usual ones because often conditions are non trivial and if lost, need much more time to restore compared to simple breakpoints that can be togled by a single click.

So definitely no.

@iloveeclipse By the way the conditions are not removed they are only disabled. if user wants that condition again the condition will still be available in the text area so it can enabled by the tick option

SougandhS · 2024-10-29T06:54:41Z

As commented on the ticket, this only adds more complexity to the menu. I don't see any reason why anyone would need an extra item for removing conditional breakpoints and why it is so important to get it into the menu.
In the opposite, removing al conditional breakpoints is more dangerous as usual ones because often conditions are non trivial and if lost, need much more time to restore compared to simple breakpoints that can be togled by a single click.
So definitely no.

@iloveeclipse By the way the conditions are not removed they are only disabled. if user wants that condition again the condition will still be available in the text area so it can enabled by the tick option

@iloveeclipse shall I reopen this ?

iloveeclipse · 2024-10-29T07:05:05Z

By the way the conditions are not removed they are only disabled.

Ticket title is "Option for Remove all conditional"
PR says "Clear all conditions in breakpoints"
Screenshot says "Remove all conditions"
You say "only disabled"

This is extremely confusing.
Please make sure that you properly explain the problem & solution, as all points mean completely different actions!

SougandhS · 2024-10-29T07:06:56Z

By the way the conditions are not removed they are only disabled.

Ticket title is "Option for Remove all conditional"

PR says "Clear all conditions in breakpoints"

Screenshot says "Remove all conditions"

You say "only disabled"

This is extremely confusing. Please make sure that you properly explain the problem & solution, as all points mean completely different actions!

Will correct it now.

SougandhS · 2024-10-29T07:08:50Z

By the way the conditions are not removed they are only disabled.

Ticket title is "Option for Remove all conditional"

PR says "Clear all conditions in breakpoints"

Screenshot says "Remove all conditions"

You say "only disabled"

This is extremely confusing. Please make sure that you properly explain the problem & solution, as all points mean completely different actions!

Will update the code now.

iloveeclipse · 2024-10-29T07:17:56Z

Will update the code now.

Now you are pushed an older commit? It is not on the master branch head as before.

SougandhS · 2024-10-29T08:27:01Z

Will update the code now.

Now you are pushed an older commit? It is not on the master branch head as before.

I have force pushed a new one now.

iloveeclipse · 2024-10-29T08:27:21Z

Still not on latest master state

SougandhS · 2024-10-29T08:28:18Z

Still not on latest master state

let me check again

SougandhS · 2024-10-29T10:01:11Z

Hi @iloveeclipse , had some issues with my git earlier, now its fixed pls check latest commit

org.eclipse.jdt.debug.ui/plugin.properties

org.eclipse.jdt.debug.ui/plugin.xml

org.eclipse.jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/ActionMessages.properties

...dt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/RemoveAllCondtionalBreakpoints.java

iloveeclipse · 2024-10-29T16:11:26Z

...dt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/RemoveAllCondtionalBreakpoints.java

+						for (IBreakpoint breakpoint : DebugPlugin.getDefault().getBreakpointManager().getBreakpoints()) {
+							if (breakpoint instanceof JavaLineBreakpoint javaBp) {
+								if (javaBp.isConditionEnabled()) {
+									javaBp.setConditionEnabled(false);


I'm not sure this is the right thing to do. I would expect the breakpoint to be disabled, not the condition on the breakpoint

Intention of the PR is to provide functionality similar to "Remove All Triggers" in which all triggers in breakpoints will be disabled by preserving the breakpoint enabled/disabled status.

so for this all conditional check will be disabled. keeping the condition and enabled/disabled status as it is.

...dt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/RemoveAllCondtionalBreakpoints.java

eclipse-jdt-bot · 2024-11-27T03:56:03Z

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 e2a30e58100aaed483f81549fdae4b5583719da4 Mon Sep 17 00:00:00 2001
From: Eclipse JDT Bot <jdt-bot@eclipse.org>
Date: Mon, 2 Dec 2024 02:28:18 +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.

iloveeclipse · 2024-11-27T06:30:27Z

org.eclipse.jdt.debug.ui/plugin.xml

+               helpContextId="remove_all_breakpoints_action_context"
+               icon="$nl$/icons/full/elcl16/removeAllConditions.png"
+               class="org.eclipse.jdt.internal.debug.ui.actions.RemoveAllCondtionalBreakpoints"
+               menubarPath="breakpointGroupJava"


The right solution would be to add additional separator or group at the platform and contribute new menu to that, so it is not placed in a wrong location. Please try that.

iloveeclipse · 2024-11-27T06:34:59Z

....jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/DisableCondtionalBreakpoints.java

+		IWorkspaceRunnable runnable = monitor -> {
+			for (IBreakpoint breakpoint : DebugPlugin.getDefault().getBreakpointManager().getBreakpoints()) {
+				try {
+					breakpoint.getMarker().setAttribute(IBreakpoint.ENABLED, breakpoint.isEnabled());


looks like here you change all breakpoints, while above only JavaLimeBreakpoints. This is not OK.

Will change it

iloveeclipse · 2024-11-27T06:36:18Z

....jdt.debug.ui/ui/org/eclipse/jdt/internal/debug/ui/actions/DisableCondtionalBreakpoints.java

+	}
+
+	@Override
+	protected void initialize() {


Why do you need listener? Isn't action state updated on every right click?

I have to check that.

Listener is required then only it will update, other right click options have the same too.

Provides additional Action in breakpoints actions for disabling all the conditions set in breakpoints. Will be useful for disabling every conditions in breakpoints by preserving the conditions in the text editor. Enhancement eclipse-jdt#549

This PR provides additional separation and arrangements between Enable/Disable Actions in breakpoints view. New actions can be contributed here. Part of Enhancement #eclipse-jdt/eclipse.jdt.debug#550

iloveeclipse requested changes Oct 29, 2024

View reviewed changes

SougandhS closed this Oct 29, 2024

SougandhS changed the title ~~Clear all conditions in breakpoints #549~~ Disable all conditions in breakpoints #549 Oct 29, 2024

SougandhS reopened this Oct 29, 2024

SougandhS force-pushed the RemoveAll_Condition_Options branch 2 times, most recently from bacf750 to 394e594 Compare October 29, 2024 08:13

SougandhS force-pushed the RemoveAll_Condition_Options branch 2 times, most recently from c20f1f4 to 2d7ded5 Compare October 29, 2024 08:35

SougandhS requested a review from iloveeclipse October 29, 2024 08:48

SougandhS force-pushed the RemoveAll_Condition_Options branch 2 times, most recently from 75f08dd to afa36d2 Compare October 29, 2024 10:00

iloveeclipse requested changes Oct 29, 2024

View reviewed changes

SougandhS force-pushed the RemoveAll_Condition_Options branch from afa36d2 to 821e5d4 Compare November 7, 2024 09:00

SougandhS force-pushed the RemoveAll_Condition_Options branch from 821e5d4 to 426110f Compare November 27, 2024 03:54

SougandhS requested a review from iloveeclipse November 27, 2024 03:57

iloveeclipse requested changes Nov 27, 2024

View reviewed changes

SougandhS force-pushed the RemoveAll_Condition_Options branch from 50fdd56 to f2133cf Compare December 2, 2024 02:26

Version bump(s) for 4.35 stream

02609fc

SougandhS mentioned this pull request Dec 2, 2024

Additional Separator #549 eclipse-platform/eclipse.platform#1648

Open

SougandhS requested a review from iloveeclipse December 2, 2024 02:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable all conditions in breakpoints #549 #550

Disable all conditions in breakpoints #549 #550

SougandhS commented Oct 29, 2024 •

edited

Loading

iloveeclipse left a comment

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024 •

edited

Loading

iloveeclipse Oct 29, 2024

SougandhS Oct 30, 2024 •

edited

Loading

SougandhS Oct 30, 2024

eclipse-jdt-bot commented Nov 27, 2024 •

edited

Loading

iloveeclipse Nov 27, 2024

iloveeclipse Nov 27, 2024

SougandhS Nov 27, 2024

iloveeclipse Nov 27, 2024

SougandhS Nov 27, 2024

SougandhS Nov 27, 2024 •

edited

Loading

Disable all conditions in breakpoints #549 #550

Are you sure you want to change the base?

Disable all conditions in breakpoints #549 #550

Conversation

SougandhS commented Oct 29, 2024 • edited Loading

What it does

How to test

Author checklist

iloveeclipse left a comment

Choose a reason for hiding this comment

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

iloveeclipse commented Oct 29, 2024

SougandhS commented Oct 29, 2024

SougandhS commented Oct 29, 2024 • edited Loading

iloveeclipse Oct 29, 2024

Choose a reason for hiding this comment

SougandhS Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

SougandhS Oct 30, 2024

Choose a reason for hiding this comment

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

iloveeclipse Nov 27, 2024

Choose a reason for hiding this comment

iloveeclipse Nov 27, 2024

Choose a reason for hiding this comment

SougandhS Nov 27, 2024

Choose a reason for hiding this comment

iloveeclipse Nov 27, 2024

Choose a reason for hiding this comment

SougandhS Nov 27, 2024

Choose a reason for hiding this comment

SougandhS Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

SougandhS commented Oct 29, 2024 •

edited

Loading

SougandhS commented Oct 29, 2024 •

edited

Loading

SougandhS Oct 30, 2024 •

edited

Loading

eclipse-jdt-bot commented Nov 27, 2024 •

edited

Loading

SougandhS Nov 27, 2024 •

edited

Loading