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 <teodor.madan@freescale.com>
Reviewed-on: https://git.eclipse.org/r/25768
Tested-by: Hudson CI

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<IBreakpointAction> 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<IBreakpointAction> getBreakpointActions() {
 		if (breakpointActions == null) {
-			breakpointActions = new ArrayList();
+			breakpointActions = new ArrayList<IBreakpointAction>();
 			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();

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());
+	}
+
 }

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...

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());
+	}
 }

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