From 1b4d3baf2916c1d5d58d94241b785e15f2e76a5e Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Fri, 12 May 2017 15:54:21 +0200 Subject: [PATCH] IDEA-169025 Preserve run configurations order --- .../execution/impl/RunConfigurableTest.kt | 135 ++++++++++++------ .../execution/impl/RunConfigurable.java | 26 +++- .../intellij/execution/impl/RunManagerImpl.kt | 18 +-- 3 files changed, 124 insertions(+), 55 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt b/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt index a10c98304b95..40135a27df01 100644 --- a/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt +++ b/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt @@ -18,19 +18,24 @@ package com.intellij.execution.impl import com.intellij.execution.application.ApplicationConfigurationType import com.intellij.execution.impl.RunConfigurable.NodeKind.* import com.intellij.execution.junit.JUnitConfigurationType +import com.intellij.openapi.util.Disposer import com.intellij.openapi.util.Trinity -import com.intellij.testFramework.LightIdeaTestCase -import com.intellij.testFramework.LightPlatformTestCase +import com.intellij.testFramework.EdtRule +import com.intellij.testFramework.ProjectRule +import com.intellij.testFramework.RunsInEdt import com.intellij.testFramework.assertions.Assertions.assertThat import com.intellij.ui.RowsDnDSupport import com.intellij.ui.RowsDnDSupport.RefinedDropSupport.Position.* import com.intellij.ui.treeStructure.Tree import com.intellij.util.loadElement import org.jdom.Element +import org.junit.After +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test import java.util.* import javax.swing.tree.DefaultMutableTreeNode import javax.swing.tree.TreePath -import kotlin.properties.Delegates private val ORDER = arrayOf(CONFIGURATION_TYPE, //Application FOLDER, //1 @@ -42,41 +47,56 @@ private val ORDER = arrayOf(CONFIGURATION_TYPE, //Application CONFIGURATION, CONFIGURATION, TEMPORARY_CONFIGURATION, UNKNOWN//Defaults ) -private fun createRunManager(element: Element): RunManagerImpl { - val runManager = RunManagerImpl(LightPlatformTestCase.getProject()) - runManager.initializeConfigurationTypes(arrayOf(ApplicationConfigurationType.getInstance(), JUnitConfigurationType.getInstance())) - runManager.loadState(element) - return runManager -} +@RunsInEdt +class RunConfigurableTest { + companion object { + @JvmField + @ClassRule + val projectRule = ProjectRule() -class RunConfigurableTest : LightIdeaTestCase() { - private var configurable: MockRunConfigurable? = null - private var tree: Tree by Delegates.notNull() - private var root: DefaultMutableTreeNode? = null - private var model: RunConfigurable.MyTreeModel by Delegates.notNull() + private fun createRunManager(element: Element): RunManagerImpl { + val runManager = RunManagerImpl(projectRule.project) + runManager.initializeConfigurationTypes(arrayOf(ApplicationConfigurationType.getInstance(), JUnitConfigurationType.getInstance())) + runManager.loadState(element) + return runManager + } - override fun setUp() { - super.setUp() - - configurable = MockRunConfigurable(createRunManager(loadElement(RunConfigurableTest::class.java.getResourceAsStream("folders.xml")))) - tree = configurable!!.myTree - root = configurable!!.myRoot - model = configurable!!.myTreeModel - } - - override fun tearDown() { - try { - if (configurable != null) { - configurable!!.disposeUIResources() + private class MockRunConfigurable(private val testManager: RunManagerImpl) : RunConfigurable(projectRule.project) { + init { + createComponent() } - configurable = null - root = null - } - finally { - super.tearDown() + + internal override fun getRunManager() = testManager } } + @JvmField + @Rule + val edtRule = EdtRule() + + private val disposable = Disposer.newDisposable() + + private val configurable: RunConfigurable by lazy { + val result = MockRunConfigurable(createRunManager(loadElement(RunConfigurableTest::class.java.getResourceAsStream("folders.xml")))) + Disposer.register(disposable, result) + result + } + + private val root: DefaultMutableTreeNode + get() = configurable.myRoot + + private val tree: Tree + get() = configurable.myTree + + private val model: RunConfigurable.MyTreeModel + get() = configurable.myTreeModel + + @After + fun tearDown() { + Disposer.dispose(disposable) + } + + @Test fun testDND() { doExpand() val never = intArrayOf(-1, 0, 14, 22, 23, 999) @@ -130,7 +150,7 @@ class RunConfigurableTest : LightIdeaTestCase() { tree.expandPath(TreePath(node.path)) } - assertThat(ORDER.mapIndexed { index, nodeKind -> RunConfigurable.getKind(tree.getPathForRow(index).lastPathComponent as DefaultMutableTreeNode) }).containsExactly(*ORDER) + assertThat(ORDER.mapIndexed { index, nodeKind -> RunConfigurable.getKind(tree.getPathForRow(index).lastPathComponent as DefaultMutableTreeNode) }).containsExactly(*ORDER) } private fun assertCan(oldIndex: Int, newIndex: Int, position: RowsDnDSupport.RefinedDropSupport.Position) { @@ -155,6 +175,7 @@ class RunConfigurableTest : LightIdeaTestCase() { } } + @Test fun testMoveUpDown() { doExpand() checkPositionToMove(0, 1, null) @@ -181,14 +202,46 @@ class RunConfigurableTest : LightIdeaTestCase() { private fun checkPositionToMove(selectedRow: Int, direction: Int, expected: Trinity?) { tree.setSelectionRow(selectedRow) - assertThat(configurable!!.getAvailableDropPosition(direction)).isEqualTo(expected) - } -} - -private class MockRunConfigurable(private val testManager: RunManagerImpl) : RunConfigurable(LightPlatformTestCase.getProject()) { - init { - createComponent() + assertThat(configurable.getAvailableDropPosition(direction)).isEqualTo(expected) } - internal override fun getRunManager() = testManager + @Test + fun testSort() { + doExpand() + assertThat(configurable.isModified).isFalse() + model.drop(2, 0, ABOVE) + assertThat(configurable.isModified).isTrue() + configurable.apply() + assertThat(configurable.runManager.allSettings.map { it.name }).isEqualTo(listOf("Renamer", + "UI", + "AuTest", + "Simples", + "OutAndErr", + "C148C_TersePrincess", + "Periods", + "C148E_Porcelain", + "ErrAndOut", + "All in titled", + "All in titled2", + "All in titled3", + "All in titled4", + "All in titled5")) + assertThat(configurable.isModified).isFalse() + model.drop(4, 8, BELOW) + configurable.apply() + assertThat(configurable.runManager.allSettings.map { it.name }).isEqualTo(listOf("Renamer", + "AuTest", + "Simples", + "UI", + "OutAndErr", + "C148C_TersePrincess", + "Periods", + "C148E_Porcelain", + "ErrAndOut", + "All in titled", + "All in titled2", + "All in titled3", + "All in titled4", + "All in titled5")) + } } \ No newline at end of file diff --git a/platform/lang-impl/src/com/intellij/execution/impl/RunConfigurable.java b/platform/lang-impl/src/com/intellij/execution/impl/RunConfigurable.java index 16d8f99e6602..a62238f55cf8 100644 --- a/platform/lang-impl/src/com/intellij/execution/impl/RunConfigurable.java +++ b/platform/lang-impl/src/com/intellij/execution/impl/RunConfigurable.java @@ -20,6 +20,7 @@ import com.intellij.execution.configuration.ConfigurationFactoryEx; import com.intellij.execution.configurations.*; import com.intellij.icons.AllIcons; import com.intellij.ide.DataManager; +import com.intellij.openapi.Disposable; import com.intellij.openapi.actionSystem.*; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; @@ -29,6 +30,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.Messages; import com.intellij.openapi.ui.popup.ListPopup; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Trinity; import com.intellij.openapi.util.text.StringUtil; @@ -43,6 +45,7 @@ import com.intellij.util.containers.HashMap; import com.intellij.util.ui.*; import com.intellij.util.ui.tree.TreeUtil; import gnu.trove.THashSet; +import gnu.trove.TObjectIntHashMap; import net.miginfocom.swing.MigLayout; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -64,7 +67,7 @@ import static com.intellij.openapi.ui.LabeledComponent.create; import static com.intellij.openapi.wm.IdeFocusManager.getGlobalInstance; import static com.intellij.ui.RowsDnDSupport.RefinedDropSupport.Position.*; -class RunConfigurable extends BaseConfigurable { +class RunConfigurable extends BaseConfigurable implements Disposable { @NonNls private static final Object DEFAULTS = new Object() { @Override public String toString() { @@ -642,13 +645,19 @@ class RunConfigurable extends BaseConfigurable { try { updateActiveConfigurationFromSelected(); + TObjectIntHashMap settingsToOrder = new TObjectIntHashMap<>(); + int order = 0; Set toDeleteSettings = new THashSet<>(manager.getAllSettings()); RunnerAndConfigurationSettings selectedSettings = getSelectedSettings(); for (int i = 0; i < myRoot.getChildCount(); i++) { DefaultMutableTreeNode node = (DefaultMutableTreeNode)myRoot.getChildAt(i); Object userObject = node.getUserObject(); if (userObject instanceof ConfigurationType) { - applyByType(node, (ConfigurationType)userObject, selectedSettings, toDeleteSettings); + List beans = applyByType(node, (ConfigurationType)userObject, selectedSettings); + for (RunConfigurationBean bean : beans) { + settingsToOrder.put(bean.getSettings(), order++); + toDeleteSettings.remove(bean.getSettings()); + } } } manager.removeConfigurations(toDeleteSettings); @@ -670,7 +679,7 @@ class RunConfigurable extends BaseConfigurable { each.first.apply(); } - manager.setOrder(null); + manager.setOrder(Comparator.comparingInt(settingsToOrder::get)); } finally { manager.fireEndUpdate(); @@ -686,7 +695,7 @@ class RunConfigurable extends BaseConfigurable { } } - private void applyByType(@NotNull DefaultMutableTreeNode typeNode, @NotNull ConfigurationType type, @Nullable RunnerAndConfigurationSettings selectedSettings, @NotNull Set toDeleteSettings) throws ConfigurationException { + private List applyByType(@NotNull DefaultMutableTreeNode typeNode, @NotNull ConfigurationType type, @Nullable RunnerAndConfigurationSettings selectedSettings) throws ConfigurationException { int indexToMove = -1; final List configurationBeans = new ArrayList<>(); @@ -739,7 +748,6 @@ class RunConfigurable extends BaseConfigurable { // try to apply all for (RunConfigurationBean bean : configurationBeans) { applyConfiguration(typeNode, bean.getConfigurable()); - toDeleteSettings.remove(bean.getSettings()); } //Just saved as 'stable' configuration shouldn't stay between temporary ones (here we order model to save) @@ -750,6 +758,8 @@ class RunConfigurable extends BaseConfigurable { if (shift != 0 && indexToMove != -1) { configurationBeans.add(indexToMove-shift, configurationBeans.remove(indexToMove)); } + + return configurationBeans; } static void collectNodesRecursively(DefaultMutableTreeNode parentNode, List nodes, NodeKind... allowed) { @@ -852,6 +862,11 @@ class RunConfigurable extends BaseConfigurable { @Override public void disposeUIResources() { + Disposer.dispose(this); + } + + @Override + public void dispose() { isDisposed = true; for (Configurable configurable : myStoredComponents.values()) { configurable.disposeUIResources(); @@ -1638,6 +1653,7 @@ class RunConfigurable extends BaseConfigurable { myShared = configurable.isStoreProjectConfiguration(); } + @NotNull public RunnerAndConfigurationSettings getSettings() { return mySettings; } diff --git a/platform/lang-impl/src/com/intellij/execution/impl/RunManagerImpl.kt b/platform/lang-impl/src/com/intellij/execution/impl/RunManagerImpl.kt index 6edc1c7591ed..2246ebc6f30b 100644 --- a/platform/lang-impl/src/com/intellij/execution/impl/RunManagerImpl.kt +++ b/platform/lang-impl/src/com/intellij/execution/impl/RunManagerImpl.kt @@ -262,24 +262,23 @@ open class RunManagerImpl(internal val project: Project) : RunManagerEx(), Persi lock.write { immutableSortedSettingsList = null - existingId = findExistingConfigurationId(settings) // https://youtrack.jetbrains.com/issue/IDEA-112821 // we should check by instance, not by id (todo is it still relevant?) + existingId = if (idToSettings.get(newId) === settings) newId else findExistingConfigurationId(settings) existingId?.let { - // idToSettings is a LinkedHashMap - we must remove even if existingId equals to newId and in any case we will replace it on put - idToSettings.remove(it) + if (newId != it) { + idToSettings.remove(it) + } } + idToSettings.put(newId, settings) + if (selectedConfigurationId != null && selectedConfigurationId == existingId) { selectedConfigurationId = newId } - idToSettings.put(newId, settings) if (existingId == null) { refreshUsagesList(settings) - } - - if (existingId == null) { settings.schemeManager?.addScheme(settings as RunnerAndConfigurationSettingsImpl) } else { @@ -287,9 +286,10 @@ open class RunManagerImpl(internal val project: Project) : RunManagerEx(), Persi } } - checkRecentsLimit() - if (existingId == null) { + if (settings.isTemporary) { + checkRecentsLimit() + } eventPublisher.runConfigurationAdded(settings) } else {