From def7238ea84b1f918cc79cca2d57057b4ccd2354 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Wed, 28 Nov 2018 18:39:17 +0100 Subject: [PATCH] simplify ChooseRunConfigurationPopup sort logic and fix .ArrayIndexOutOfBoundsException: -1 (if result is empty) add test --- .../execution/impl/RunConfigurableTest.kt | 71 ++++++++--- .../actions/ChooseRunConfigurationPopup.java | 119 +++++++++++------- 2 files changed, 127 insertions(+), 63 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 7b2c455c715c..dfb5be16d077 100644 --- a/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt +++ b/java/java-tests/testSrc/com/intellij/execution/impl/RunConfigurableTest.kt @@ -1,6 +1,8 @@ // Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.execution.impl +import com.intellij.execution.actions.ChooseRunConfigurationPopup +import com.intellij.execution.actions.ExecutorProvider import com.intellij.execution.application.ApplicationConfigurationType import com.intellij.execution.impl.RunConfigurableNodeKind.* import com.intellij.execution.junit.JUnitConfigurationType @@ -195,25 +197,26 @@ internal class RunConfigurableTest { model.drop(2, 14, ABOVE) assertThat(configurable.isModified).isTrue() configurable.apply() - assertThat(configurable.runManager.allSettings.map { it.name }).containsExactly("Renamer", - "UI", - "AuTest", - "Simples", - "OutAndErr", - "C148C_TersePrincess", - "Periods", - "C148E_Porcelain", - "ErrAndOut", - "CodeGenerator", - "All in titled", - "All in titled2", - "All in titled3", - "All in titled4", - "All in titled5") + val runManager = configurable.runManager + assertThat(runManager.allSettings.map { it.name }).containsExactly("Renamer", + "UI", + "AuTest", + "Simples", + "OutAndErr", + "C148C_TersePrincess", + "Periods", + "C148E_Porcelain", + "ErrAndOut", + "CodeGenerator", + "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.joinToString("\n") { "[${it.type.displayName}] [${it.folderName ?: ""}] ${it.name}" }).isEqualTo(""" + assertThat(runManager.allSettings.joinToString("\n") { "[${it.type.displayName}] [${it.folderName ?: ""}] ${it.name}" }).isEqualTo(""" [Application] [1] Renamer [Application] [1] UI [Application] [1] Simples @@ -230,5 +233,41 @@ internal class RunConfigurableTest { [JUnit] [5] All in titled4 [JUnit] [] All in titled5 """.trimIndent()) + + val executorProvider = ExecutorProvider { throw UnsupportedOperationException() } + assertThat(ChooseRunConfigurationPopup.createSettingsList(runManager, executorProvider, false, false).joinToString("\n") { + val value = it.value + if (value is String) { + "[$value]" + } + else { + it.value!!.toString() + } + }).isEqualTo(""" + [1] + [2 (mnemonic is to "AuTest")] + [3] + Application: CodeGenerator (level: WORKSPACE) + [4] + [5] + JUnit: All in titled5 (level: TEMPORARY) + """.trimIndent()) + assertThat(ChooseRunConfigurationPopup.createSettingsList(runManager, executorProvider, false, true).joinToString("\n") { + val value = it.value + if (value is String) { + "[$value]" + } + else { + it.value!!.toString() + } + }).isEqualTo(""" + [1] + [2 (mnemonic is to "AuTest")] + [3] + [4] + [5] + Application: CodeGenerator (level: WORKSPACE) + JUnit: All in titled5 (level: TEMPORARY) + """.trimIndent()) } } \ No newline at end of file diff --git a/platform/lang-impl/src/com/intellij/execution/actions/ChooseRunConfigurationPopup.java b/platform/lang-impl/src/com/intellij/execution/actions/ChooseRunConfigurationPopup.java index 3ce104929627..2c0cfab6b0bc 100644 --- a/platform/lang-impl/src/com/intellij/execution/actions/ChooseRunConfigurationPopup.java +++ b/platform/lang-impl/src/com/intellij/execution/actions/ChooseRunConfigurationPopup.java @@ -4,7 +4,6 @@ package com.intellij.execution.actions; import com.intellij.execution.*; import com.intellij.execution.configurations.ConfigurationType; -import com.intellij.execution.configurations.UnknownConfigurationType; import com.intellij.execution.impl.EditConfigurationsDialog; import com.intellij.execution.impl.RunDialog; import com.intellij.execution.impl.RunManagerImpl; @@ -33,11 +32,12 @@ import com.intellij.ui.popup.WizardPopup; import com.intellij.ui.popup.list.ListPopupImpl; import com.intellij.ui.popup.list.PopupListElementRenderer; import com.intellij.ui.speedSearch.SpeedSearch; -import com.intellij.util.ObjectUtils; import com.intellij.util.SmartList; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.ui.UIUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; import javax.swing.*; import java.awt.*; @@ -47,8 +47,6 @@ import java.awt.event.MouseEvent; import java.util.List; import java.util.*; -import static com.intellij.execution.impl.RunConfigurationListManagerHelperKt.compareTypesForUi; - public class ChooseRunConfigurationPopup implements ExecutorProvider { private final Project myProject; @@ -917,14 +915,26 @@ public class ChooseRunConfigurationPopup implements ExecutorProvider { } @NotNull - public static List createSettingsList(@NotNull Project project, @NotNull ExecutorProvider executorProvider, boolean isCreateEditAction) { + public static List createSettingsList(@NotNull Project project, + @NotNull ExecutorProvider executorProvider, + boolean isCreateEditAction) { + //noinspection TestOnlyProblems + return createSettingsList(RunManagerImpl.getInstanceImpl(project), executorProvider, isCreateEditAction, Registry.is("run.popup.move.folders.to.top", false)); + } + + @TestOnly + @NotNull + public static List createSettingsList(@NotNull RunManagerImpl runManager, + @NotNull ExecutorProvider executorProvider, + boolean isCreateEditAction, + boolean isMoveFoldersToTop) { List result = new ArrayList<>(); if (isCreateEditAction) { result.add(createEditAction()); } - RunManagerImpl runManager = RunManagerImpl.getInstanceImpl(project); + Project project = runManager.getProject(); final RunnerAndConfigurationSettings selectedConfiguration = runManager.getSelectedConfiguration(); if (selectedConfiguration != null) { addActionsForSelected(selectedConfiguration, project, result); @@ -933,65 +943,80 @@ public class ChooseRunConfigurationPopup implements ExecutorProvider { Map wrappedExisting = new LinkedHashMap<>(); List folderWrappers = new SmartList<>(); for (Map> folderToConfigurations : runManager.getConfigurationsGroupedByTypeAndFolder(false).values()) { - for (Map.Entry> entry : folderToConfigurations.entrySet()) { - final String folderName = entry.getKey(); - List configurations = entry.getValue(); - if (folderName != null) { - boolean isSelected = configurations.contains(selectedConfiguration); - if (isSelected) { - assert selectedConfiguration != null; + if (isMoveFoldersToTop) { + for (Map.Entry> entry : folderToConfigurations.entrySet()) { + final String folderName = entry.getKey(); + List configurations = entry.getValue(); + if (folderName != null) { + folderWrappers.add(createFolderItem(project, executorProvider, selectedConfiguration, folderName, configurations)); } - FolderWrapper folderWrapper = new FolderWrapper(project, executorProvider, - folderName + (isSelected ? " (mnemonic is to \"" + selectedConfiguration.getName() + "\")" : ""), - configurations); - if (isSelected) { - folderWrapper.setMnemonic(1); - } - folderWrappers.add(folderWrapper); - } - else { - for (RunnerAndConfigurationSettings configuration : configurations) { - final ItemWrapper wrapped = ItemWrapper.wrap(project, configuration); - if (configuration == selectedConfiguration) { - wrapped.setMnemonic(1); + else { + for (RunnerAndConfigurationSettings configuration : configurations) { + wrapAndAdd(project, configuration, selectedConfiguration, wrappedExisting); } - wrappedExisting.put(configuration, wrapped); + } + } + } + else { + // add only folders + for (Map.Entry> entry : folderToConfigurations.entrySet()) { + final String folderName = entry.getKey(); + if (folderName != null) { + result.add(createFolderItem(project, executorProvider, selectedConfiguration, folderName, entry.getValue())); + } + } + + // add configurations + List configurations = folderToConfigurations.get(null); + if (!ContainerUtil.isEmpty(configurations)) { + for (RunnerAndConfigurationSettings configuration : configurations) { + result.add(wrapAndAdd(project, configuration, selectedConfiguration, wrappedExisting)); } } } } - boolean isMoveFoldersToTop = Registry.is("run.popup.move.folders.to.top", false); if (isMoveFoldersToTop) { result.addAll(folderWrappers); } if (!DumbService.isDumb(project)) { populateWithDynamicRunners(result, wrappedExisting, project, RunManagerEx.getInstanceEx(project), selectedConfiguration); } - result.addAll(wrappedExisting.values()); - if (!isMoveFoldersToTop) { - addFolders(result, folderWrappers); + if (isMoveFoldersToTop) { + result.addAll(wrappedExisting.values()); } return result; } - private static void addFolders(@NotNull List result, @NotNull List folderWrappers) { - final int topIndex = result.size() - 1; - for (FolderWrapper folderWrapper : folderWrappers) { - int bestIndex = topIndex; - for (int index = topIndex; index < result.size(); index++) { - bestIndex = index; - ItemWrapper item = result.get(index); - ConfigurationType currentType = item.getType(); - int m = currentType == null - ? 1 - : compareTypesForUi(ObjectUtils.notNull(folderWrapper.getType(), UnknownConfigurationType.getInstance()), currentType); - if (m < 0 || (m == 0 && !(item instanceof FolderWrapper))) { - break; - } - } - result.add(bestIndex, folderWrapper); + @NotNull + private static ItemWrapper wrapAndAdd(@NotNull Project project, + @NotNull RunnerAndConfigurationSettings configuration, + @Nullable RunnerAndConfigurationSettings selectedConfiguration, + @NotNull Map wrappedExisting) { + ItemWrapper wrapped = ItemWrapper.wrap(project, configuration); + if (configuration == selectedConfiguration) { + wrapped.setMnemonic(1); } + wrappedExisting.put(configuration, wrapped); + return wrapped; + } + + @NotNull + private static FolderWrapper createFolderItem(@NotNull Project project, + @NotNull ExecutorProvider executorProvider, + @Nullable RunnerAndConfigurationSettings selectedConfiguration, + @NotNull String folderName, + @NotNull List configurations) { + boolean isSelected = selectedConfiguration != null && configurations.contains(selectedConfiguration); + String value = folderName; + if (isSelected) { + value += " (mnemonic is to \"" + selectedConfiguration.getName() + "\")"; + } + FolderWrapper result = new FolderWrapper(project, executorProvider, value, configurations); + if (isSelected) { + result.setMnemonic(1); + } + return result; } private static void addActionsForSelected(@NotNull RunnerAndConfigurationSettings selectedConfiguration, @NotNull Project project, @NotNull List result) {