Bug 446104 - Fix target removal. Also fix Config Edit page.

The launch bar manager was missing the line to actually remove the
launch target in launchTargetRemoved. Added a test to detect that.

Also fixed the Config Edit page. Name verification was failing since
it was comparing against the false configs created when the tabs
are initialized. We clean up now right after that init.

Change-Id: I9cff816040f2e1866c9454cb00f69ec4468225d4
Reviewed-on: https://git.eclipse.org/r/34477
Tested-by: Hudson CI
Reviewed-by: Doug Schaefer <dschaefer@qnx.com>

launch/org.eclipse.cdt.launchbar.core.tests/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManagerTest.java

@@ -14,6 +14,7 @@
  *******************************************************************************/
 package org.eclipse.cdt.launchbar.core.internal;
 
+import static org.junit.Assert.assertNotEquals;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -444,7 +445,8 @@ public class LaunchBarManagerTest extends TestCase {
 
 		ILaunchTargetType targetType = manager.getLaunchTargetType(DEFAULT_TARGET_TYPE_ID);
 		assertNotNull(targetType);
-		manager.launchTargetAdded(new TestLaunchTarget("testTarget", targetType));
+		ILaunchTarget testTarget = new TestLaunchTarget("testTarget", targetType);
+		manager.launchTargetAdded(testTarget);
 
 		// verify that our launch config got created and saved
 		assertNotNull(manager.getActiveLaunchMode());
@@ -457,6 +459,12 @@ public class LaunchBarManagerTest extends TestCase {
 		assertNull(manager.getActiveLaunchTarget());
 		assertNull(manager.getActiveLaunchMode());
 		verify(wc).delete();
+		
+		// remove the target and make sure it's gone.
+		manager.launchTargetRemoved(testTarget);
+		ILaunchTarget[] allTargets = manager.getAllLaunchTargets();
+		assertEquals(1, allTargets.length);
+		assertNotEquals(testTarget, allTargets[0]);
 	}
 
 	@Test

launch/org.eclipse.cdt.launchbar.core/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManager.java

@@ -895,6 +895,7 @@ public class LaunchBarManager implements ILaunchBarManager, ILaunchConfiguration
 
 	@Override
 	public void launchTargetRemoved(ILaunchTarget target) throws CoreException {
+		targets.remove(getTargetId(target));
 		for (Listener listener : listeners) {
 			try {
 				listener.launchTargetsChanged();

launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigEditPage.java

@@ -15,8 +15,11 @@ import java.lang.reflect.InvocationTargetException;
 import org.eclipse.cdt.launchbar.ui.internal.Activator;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.debug.core.DebugPlugin;
+import org.eclipse.debug.core.ILaunchConfiguration;
 import org.eclipse.debug.core.ILaunchConfigurationType;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
+import org.eclipse.debug.core.ILaunchManager;
+import org.eclipse.debug.internal.core.LaunchManager;
 import org.eclipse.debug.internal.ui.DebugUIPlugin;
 import org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationManager;
 import org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationPresentationManager;
@@ -77,15 +80,14 @@ public class NewLaunchConfigEditPage extends WizardPage {
 		nameText.addModifyListener(new ModifyListener() {
 			@Override
 			public void modifyText(ModifyEvent e) {
-				String name = nameText.getText();
-				if (!name.equals(workingCopy.getName())) {
-					String errMessage = checkName(name);
-					if (errMessage == null) {
-						workingCopy.rename(name);
-						validateFields();
-					} else {
-						setErrorMessage(errMessage);
-					}
+				String name = nameText.getText().trim();
+				workingCopy.rename(name);
+
+				String errMessage = checkName(name);
+				if (errMessage == null) {
+					validateFields();
+				} else {
+					setErrorMessage(errMessage);
 				}
 			}
 		});
@@ -96,8 +98,14 @@ public class NewLaunchConfigEditPage extends WizardPage {
 		try {
 			if (name.isEmpty()) {
 				return "Name can not be empty";
-			} else if (DebugPlugin.getDefault().getLaunchManager().isExistingLaunchConfigurationName(name)) {
-				return ("A configuration with this name already exists");
+			}
+			
+			ILaunchManager manager = DebugPlugin.getDefault().getLaunchManager();
+			if (manager.isExistingLaunchConfigurationName(name)) {
+				ILaunchConfiguration config = ((LaunchManager) manager).findLaunchConfiguration(name);
+				if (config != workingCopy.getOriginal()) {
+					return ("A configuration with this name already exists");
+				}
 			}
 		} catch (Exception e) {
 			Activator.log(e);
@@ -114,7 +122,6 @@ public class NewLaunchConfigEditPage extends WizardPage {
 			String initialMode = ((NewLaunchConfigWizard) getWizard()).modePage.selectedGroup.getMode();
 			workingCopy = type.newInstance(null, "New Configuration");
 			tabGroup = LaunchConfigurationPresentationManager.getDefault().getTabGroup(workingCopy, initialMode);
-			nameText.setText(workingCopy.getName());
 			for (CTabItem item : tabFolder.getItems())
 				item.dispose();
 			tabGroup.createTabs(launchConfigurationDialog, initialMode);
@@ -125,8 +132,6 @@ public class NewLaunchConfigEditPage extends WizardPage {
 				tab.setDefaults(workingCopy);
 				if (firstTab) {
 					firstTab = false;
-					// tab.setDefaults likely renames it
-					nameText.setText(workingCopy.getName());
 				}
 			}
 
@@ -141,7 +146,12 @@ public class NewLaunchConfigEditPage extends WizardPage {
 				tabItem.setControl(tab.getControl());
 			}
 
+			// Clean up any created configs before we set the name and trigger
+			// any validation
+			((NewLaunchConfigWizard) getWizard()).cleanUpConfigs();
+
 			tabFolder.setSelection(0);
+			nameText.setText(workingCopy.getName());
 		} catch (CoreException e) {
 			Activator.log(e);
 			return;

launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigWizard.java

@@ -53,13 +53,13 @@ public class NewLaunchConfigWizard extends Wizard implements ILaunchConfiguratio
 
 	@Override
 	public boolean performFinish() {
-		cleanUpListeners();
+		cleanUpConfigs();
 		return editPage.performFinish();
 	}
 
 	@Override
 	public boolean performCancel() {
-		cleanUpListeners();
+		cleanUpConfigs();
 		return super.performCancel();
 	}
 
@@ -68,12 +68,10 @@ public class NewLaunchConfigWizard extends Wizard implements ILaunchConfiguratio
 		// We need to make sure those saves are deleted when the dialog is finished.
 		// We also need to turn off listening in the tool bar manager so that we don't treat these
 		// as real launch configs.
-
-//		LaunchToolBarManager.getInstance().setIgnoreLaunchConfigEvents(true);
 		DebugPlugin.getDefault().getLaunchManager().addLaunchConfigurationListener(this);
 	}
 
-	private void cleanUpListeners() {
+	void cleanUpConfigs() {
 		DebugPlugin.getDefault().getLaunchManager().removeLaunchConfigurationListener(this);
 		for (ILaunchConfiguration config : configsToDelete) {
 			try {
@@ -82,7 +80,6 @@ public class NewLaunchConfigWizard extends Wizard implements ILaunchConfiguratio
 				Activator.log(e);
 			}
 		}
-//		LaunchToolBarManager.getInstance().setIgnoreLaunchConfigEvents(false);
 	}
 
 	@Override