From 13626ab74c72346f5361e9168f8a9e46dedc2425 Mon Sep 17 00:00:00 2001 From: Teodor Madan Date: Tue, 29 Apr 2014 18:07:11 +0300 Subject: [PATCH] Bug 433761 - Breakpoint Actions Dialog should not allow creating unnamed actions - Add validator and error decorator for breakpoint and tracepoint action name editor - Generics Warning Cleanup from BreakpointActionManager; avoid spawning bp action execute jobs in lack of any actions - MIBreakpointsManager - avoid spawning needless job when there's no breakpoint to execute actions for Change-Id: Ibe29b97e4f3a86d5429e3f71aae728dd7e31b97b Signed-off-by: Teodor Madan Reviewed-on: https://git.eclipse.org/r/25768 Tested-by: Hudson CI --- .../BreakpointActionManager.java | 48 +++++++++---------- .../ui/breakpointactions/ActionDialog.java | 38 ++++++++++++++- .../ui/breakpointactions/messages.properties | 1 + .../TracepointActionDialog.java | 39 ++++++++++++++- .../dsf/mi/service/MIBreakpointsManager.java | 20 ++++---- 5 files changed, 109 insertions(+), 37 deletions(-) diff --git a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/breakpointactions/BreakpointActionManager.java b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/breakpointactions/BreakpointActionManager.java index a21157dcaff..3abb2c09924 100644 --- a/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/breakpointactions/BreakpointActionManager.java +++ b/debug/org.eclipse.cdt.debug.core/src/org/eclipse/cdt/debug/core/breakpointactions/BreakpointActionManager.java @@ -13,8 +13,6 @@ package org.eclipse.cdt.debug.core.breakpointactions; import java.io.ByteArrayOutputStream; import java.io.StringReader; import java.util.ArrayList; -import java.util.Iterator; - import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; @@ -49,7 +47,7 @@ public class BreakpointActionManager { private static final String BREAKPOINT_ACTION_DATA = "BreakpointActionManager.actionData"; //$NON-NLS-1$ private IExtension[] breakpointActionExtensions = null; - private ArrayList breakpointActions = null; + private ArrayList breakpointActions = null; public BreakpointActionManager() { } @@ -103,21 +101,23 @@ public class BreakpointActionManager { if (breakpoint != null) { IMarker marker = breakpoint.getMarker(); String actionNames = marker.getAttribute(BREAKPOINT_ACTION_ATTRIBUTE, ""); //$NON-NLS-1$ - final String[] actions = actionNames.split(","); //$NON-NLS-1$ - if (actions.length > 0){ - Job job = new Job("Execute breakpoint actions") { - @Override - public IStatus run(final IProgressMonitor monitor) { - return doExecuteActions(breakpoint, context, actions, monitor); + if (actionNames.length() > 0 ) { + final String[] actions = actionNames.split(","); //$NON-NLS-1$ + if (actions.length > 0){ + Job job = new Job("Execute breakpoint actions") { //$NON-NLS-1$ + @Override + public IStatus run(final IProgressMonitor monitor) { + return doExecuteActions(breakpoint, context, actions, monitor); + } + }; + job.schedule(); + try { + // wait for actions to execute + job.join(); + }catch (InterruptedException e) + { + e.printStackTrace(); } - }; - job.schedule(); - try { - // wait for actions to execute - job.join(); - }catch (InterruptedException e) - { - e.printStackTrace(); } } } @@ -141,14 +141,13 @@ public class BreakpointActionManager { monitor.worked(1); } } catch (Exception e) { - return new Status( IStatus.ERROR, CDebugCorePlugin.getUniqueIdentifier(), CDebugCorePlugin.INTERNAL_ERROR, "Internal Error", e ); + return new Status( IStatus.ERROR, CDebugCorePlugin.getUniqueIdentifier(), CDebugCorePlugin.INTERNAL_ERROR, "Internal Error", e ); //$NON-NLS-1$ } return monitor.isCanceled() ? Status.CANCEL_STATUS : Status.OK_STATUS; } public IBreakpointAction findBreakpointAction(String name) { - for (Iterator iter = getBreakpointActions().iterator(); iter.hasNext();) { - IBreakpointAction action = (IBreakpointAction) iter.next(); + for (IBreakpointAction action : getBreakpointActions()) { if (action.getName().equals(name)) return action; } @@ -168,9 +167,9 @@ public class BreakpointActionManager { return breakpointActionExtensions; } - public ArrayList getBreakpointActions() { + public ArrayList getBreakpointActions() { if (breakpointActions == null) { - breakpointActions = new ArrayList(); + breakpointActions = new ArrayList(); CDebugCorePlugin.getDefault().getBreakpointActionManager().loadActionData(); } return breakpointActions; @@ -249,15 +248,12 @@ public class BreakpointActionManager { Element rootElement = doc.createElement("breakpointActionData"); //$NON-NLS-1$ doc.appendChild(rootElement); - for (Iterator iter = getBreakpointActions().iterator(); iter.hasNext();) { - IBreakpointAction action = (IBreakpointAction) iter.next(); - + for (IBreakpointAction action : getBreakpointActions()) { Element element = doc.createElement("actionEntry"); //$NON-NLS-1$ element.setAttribute("name", action.getName()); //$NON-NLS-1$ element.setAttribute("class", action.getClass().getName()); //$NON-NLS-1$ element.setAttribute("value", action.getMemento()); //$NON-NLS-1$ rootElement.appendChild(element); - } ByteArrayOutputStream s = new ByteArrayOutputStream(); diff --git a/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/ActionDialog.java b/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/ActionDialog.java index 0c317785ea2..81b6975e721 100644 --- a/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/ActionDialog.java +++ b/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/ActionDialog.java @@ -20,13 +20,18 @@ import org.eclipse.core.runtime.IExtensionPoint; import org.eclipse.core.runtime.Platform; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.IDialogConstants; +import org.eclipse.jface.fieldassist.ControlDecoration; +import org.eclipse.jface.fieldassist.FieldDecorationRegistry; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.StackLayout; +import org.eclipse.swt.events.ModifyEvent; +import org.eclipse.swt.events.ModifyListener; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Combo; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; @@ -112,7 +117,7 @@ public class ActionDialog extends Dialog { actionNameLabel.setText(Messages.getString("ActionDialog.1")); //$NON-NLS-1$ actionNameTextWidget = new Text(dialogArea, SWT.BORDER); - actionNameTextWidget.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + addDecorator(dialogArea, actionNameTextWidget); final Label breakpointActionTypeLabel = new Label(dialogArea, SWT.NONE); breakpointActionTypeLabel.setText(Messages.getString("ActionDialog.2")); //$NON-NLS-1$ @@ -290,4 +295,35 @@ public class ActionDialog extends Dialog { return actionPageResult; } + private void addDecorator(Composite parent, final Text control) { + GridData gd = new GridData(SWT.FILL, SWT.CENTER, true, false); + gd.horizontalIndent = FieldDecorationRegistry.getDefault().getMaximumDecorationWidth(); + control.setLayoutData(gd); + + final ControlDecoration decoration = new ControlDecoration(control, SWT.TOP | SWT.LEFT, parent ); + decoration.hide(); + control.addModifyListener(new ModifyListener() { + @Override + public void modifyText(ModifyEvent e) { + String name = control.getText(); + if (name.trim().isEmpty()) { + decoration.setImage( + FieldDecorationRegistry.getDefault().getFieldDecoration( + FieldDecorationRegistry.DEC_ERROR).getImage()); + decoration.setDescriptionText(Messages.getString("ActionDialog.ErrEmptyName")); //$NON-NLS-1$ + decoration.show(); + } else { + decoration.hide(); + } + validate(); + } + }); + } + + private void validate() { + Button okButton = getButton(IDialogConstants.OK_ID); + if (okButton != null) + okButton.setEnabled(!actionNameTextWidget.getText().trim().isEmpty()); + } + } diff --git a/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/messages.properties b/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/messages.properties index e8472f47454..7457b01b466 100644 --- a/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/messages.properties +++ b/debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/ui/breakpointactions/messages.properties @@ -28,6 +28,7 @@ ActionsList.5=Down ActionDialog.0=New Breakpoint Action ActionDialog.1=Action name: ActionDialog.2=Action type: +ActionDialog.ErrEmptyName=Action name cannot be empty SoundActionComposite.4=Select a sound to play when the breakpoint is hit: SoundActionComposite.5=Choose a sound file: SoundActionComposite.6=Browse... diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/tracepointactions/TracepointActionDialog.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/tracepointactions/TracepointActionDialog.java index d04cd81733f..978d2feb1de 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/tracepointactions/TracepointActionDialog.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/tracepointactions/TracepointActionDialog.java @@ -15,6 +15,7 @@ import java.util.Vector; import org.eclipse.cdt.debug.core.breakpointactions.IBreakpointAction; import org.eclipse.cdt.debug.ui.CDebugUIPlugin; import org.eclipse.cdt.debug.ui.breakpointactions.IBreakpointActionPage; +import org.eclipse.cdt.debug.ui.breakpointactions.Messages; import org.eclipse.cdt.dsf.gdb.internal.tracepointactions.CollectAction; import org.eclipse.cdt.dsf.gdb.internal.tracepointactions.EvaluateAction; import org.eclipse.cdt.dsf.gdb.internal.tracepointactions.ITracepointAction; @@ -28,13 +29,18 @@ import org.eclipse.core.runtime.IExtensionPoint; import org.eclipse.core.runtime.Platform; import org.eclipse.jface.dialogs.Dialog; import org.eclipse.jface.dialogs.IDialogConstants; +import org.eclipse.jface.fieldassist.ControlDecoration; +import org.eclipse.jface.fieldassist.FieldDecorationRegistry; import org.eclipse.swt.SWT; import org.eclipse.swt.custom.StackLayout; +import org.eclipse.swt.events.ModifyEvent; +import org.eclipse.swt.events.ModifyListener; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.widgets.Button; import org.eclipse.swt.widgets.Combo; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; @@ -123,7 +129,7 @@ public class TracepointActionDialog extends Dialog { actionNameLabel.setText(MessagesForTracepointActions.TracepointActions_ActionDialog_Name); actionNameTextWidget = new Text(dialogArea, SWT.BORDER); - actionNameTextWidget.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false)); + addDecorator(dialogArea, actionNameTextWidget); final Label breakpointActionTypeLabel = new Label(dialogArea, SWT.NONE); breakpointActionTypeLabel.setText(MessagesForTracepointActions.TracepointActions_ActionDialog_Type); @@ -278,4 +284,35 @@ public class TracepointActionDialog extends Dialog { return actionPageResult; } + + private void addDecorator(Composite parent, final Text control) { + GridData gd = new GridData(SWT.FILL, SWT.CENTER, true, false); + gd.horizontalIndent = FieldDecorationRegistry.getDefault().getMaximumDecorationWidth(); + control.setLayoutData(gd); + + final ControlDecoration decoration = new ControlDecoration(control, SWT.TOP | SWT.LEFT, parent ); + decoration.hide(); + control.addModifyListener(new ModifyListener() { + @Override + public void modifyText(ModifyEvent e) { + String name = control.getText(); + if (name.trim().isEmpty()) { + decoration.setImage( + FieldDecorationRegistry.getDefault().getFieldDecoration( + FieldDecorationRegistry.DEC_ERROR).getImage()); + decoration.setDescriptionText(Messages.getString("ActionDialog.ErrEmptyName")); //$NON-NLS-1$ + decoration.show(); + } else { + decoration.hide(); + } + validate(); + } + }); + } + + private void validate() { + Button okButton = getButton(IDialogConstants.OK_ID); + if (okButton != null) + okButton.setEnabled(!actionNameTextWidget.getText().trim().isEmpty()); + } } diff --git a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java index 14763ba07b0..d7b4bb1f368 100644 --- a/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java +++ b/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java @@ -1420,15 +1420,17 @@ public class MIBreakpointsManager extends AbstractDsfService implements IBreakpo // Identify the platform breakpoint final ICBreakpoint breakpoint = findPlatformBreakpoint(number); - // Perform the actions asynchronously (otherwise we can have a deadlock...) - new Job("Breakpoint action") { //$NON-NLS-1$ - { setSystem(true); } - @Override - protected IStatus run(IProgressMonitor monitor) { - fBreakpointActionManager.executeActions(breakpoint, new BreakpointActionAdapter(getExecutor(), getServicesTracker(), context)); - return Status.OK_STATUS; - }; - }.schedule(); + if (breakpoint != null ) { + // Perform the actions asynchronously (otherwise we can have a deadlock...) + new Job("Breakpoint action") { //$NON-NLS-1$ + { setSystem(true); } + @Override + protected IStatus run(IProgressMonitor monitor) { + fBreakpointActionManager.executeActions(breakpoint, new BreakpointActionAdapter(getExecutor(), getServicesTracker(), context)); + return Status.OK_STATUS; + }; + }.schedule(); + } } // Helper function to locate the platform breakpoint corresponding