From 671fea7f494a7dcb1d796ff0a534331b6aa7a58c Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Mon, 6 Oct 2014 22:29:15 -0400 Subject: [PATCH] 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 --- .../core/internal/LaunchBarManagerTest.java | 10 ++++- .../core/internal/LaunchBarManager.java | 1 + .../dialogs/NewLaunchConfigEditPage.java | 38 ++++++++++++------- .../dialogs/NewLaunchConfigWizard.java | 9 ++--- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/launch/org.eclipse.cdt.launchbar.core.tests/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManagerTest.java b/launch/org.eclipse.cdt.launchbar.core.tests/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManagerTest.java index 4c815ae47db..1aa0f9024e7 100644 --- a/launch/org.eclipse.cdt.launchbar.core.tests/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManagerTest.java +++ b/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 diff --git a/launch/org.eclipse.cdt.launchbar.core/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManager.java b/launch/org.eclipse.cdt.launchbar.core/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManager.java index a36117e80f4..18201f7ca8c 100644 --- a/launch/org.eclipse.cdt.launchbar.core/src/org/eclipse/cdt/launchbar/core/internal/LaunchBarManager.java +++ b/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(); diff --git a/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigEditPage.java b/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigEditPage.java index ca49ffb9c28..9458276c082 100644 --- a/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigEditPage.java +++ b/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; diff --git a/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigWizard.java b/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigWizard.java index 2a7bc94a510..2d53b5142b1 100644 --- a/launch/org.eclipse.cdt.launchbar.ui/src/org/eclipse/cdt/launchbar/ui/internal/dialogs/NewLaunchConfigWizard.java +++ b/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