From d02f0e17cafb3ec3c30237aff03706c96909b906 Mon Sep 17 00:00:00 2001 From: Alex Plate Date: Wed, 31 Jul 2024 13:08:08 +0300 Subject: [PATCH] Revert "Fix(VIM-3376): Refactor the way IdeaVim executes actions" This reverts commit 24514039 --- .teamcity/_Self/Constants.kt | 16 +-- gradle.properties | 2 +- .../idea/vim/handler/VimEnterHandler.kt | 2 - .../idea/vim/helper/IjActionExecutor.kt | 98 ++++++++----------- .../vim/newapi/IjEditorExecutionContext.kt | 5 - .../ideavim/action/MotionActionTest.kt | 2 + .../motion/select/SelectKeyHandlerTest.kt | 3 + 7 files changed, 56 insertions(+), 72 deletions(-) diff --git a/.teamcity/_Self/Constants.kt b/.teamcity/_Self/Constants.kt index 3fd8c35393..4b5b96a30f 100644 --- a/.teamcity/_Self/Constants.kt +++ b/.teamcity/_Self/Constants.kt @@ -5,13 +5,13 @@ object Constants { const val EAP_CHANNEL = "eap" const val DEV_CHANNEL = "Dev" - const val GITHUB_TESTS = "2024.1.2" - const val NVIM_TESTS = "2024.1.2" - const val PROPERTY_TESTS = "2024.1.2" - const val LONG_RUNNING_TESTS = "2024.1.2" - const val QODANA_TESTS = "2024.1.2" - const val RELEASE = "2024.1.2" + const val GITHUB_TESTS = "2024.1.1" + const val NVIM_TESTS = "2024.1.1" + const val PROPERTY_TESTS = "2024.1.1" + const val LONG_RUNNING_TESTS = "2024.1.1" + const val QODANA_TESTS = "2024.1.1" + const val RELEASE = "2024.1.1" - const val RELEASE_DEV = "2024.1.2" - const val RELEASE_EAP = "2024.1.2" + const val RELEASE_DEV = "2024.1.1" + const val RELEASE_EAP = "2024.1.1" } diff --git a/gradle.properties b/gradle.properties index 9455cd7586..862d6084dc 100644 --- a/gradle.properties +++ b/gradle.properties @@ -16,7 +16,7 @@ # https://data.services.jetbrains.com/products?code=IC # Maven releases are here: https://www.jetbrains.com/intellij-repository/releases # And snapshots: https://www.jetbrains.com/intellij-repository/snapshots -ideaVersion=2024.1.2 +ideaVersion=2024.1.1 # Values for type: https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html#intellij-extension-type ideaType=IC instrumentPluginCode=true diff --git a/src/main/java/com/maddyhome/idea/vim/handler/VimEnterHandler.kt b/src/main/java/com/maddyhome/idea/vim/handler/VimEnterHandler.kt index 5bf81c08d1..04dc9c6dcb 100644 --- a/src/main/java/com/maddyhome/idea/vim/handler/VimEnterHandler.kt +++ b/src/main/java/com/maddyhome/idea/vim/handler/VimEnterHandler.kt @@ -36,7 +36,6 @@ import com.maddyhome.idea.vim.helper.isPrimaryEditor import com.maddyhome.idea.vim.helper.updateCaretsVisualAttributes import com.maddyhome.idea.vim.newapi.actionStartedFromVim import com.maddyhome.idea.vim.newapi.globalIjOptions -import com.maddyhome.idea.vim.newapi.runningIJAction import com.maddyhome.idea.vim.newapi.vim import com.maddyhome.idea.vim.state.mode.Mode import java.awt.event.KeyEvent @@ -165,7 +164,6 @@ internal abstract class OctopusHandler(private val nextHandler: EditorActionHand } if (dataContext?.actionStartedFromVim == true) return true - if (runningIJAction) return true return false } diff --git a/src/main/java/com/maddyhome/idea/vim/helper/IjActionExecutor.kt b/src/main/java/com/maddyhome/idea/vim/helper/IjActionExecutor.kt index c07fe02243..0719a08eac 100644 --- a/src/main/java/com/maddyhome/idea/vim/helper/IjActionExecutor.kt +++ b/src/main/java/com/maddyhome/idea/vim/helper/IjActionExecutor.kt @@ -8,25 +8,24 @@ package com.maddyhome.idea.vim.helper +import com.intellij.openapi.actionSystem.ActionGroup import com.intellij.openapi.actionSystem.ActionManager import com.intellij.openapi.actionSystem.ActionPlaces import com.intellij.openapi.actionSystem.AnAction +import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.actionSystem.DataContextWrapper import com.intellij.openapi.actionSystem.EmptyAction import com.intellij.openapi.actionSystem.IdeActions -import com.intellij.openapi.actionSystem.PlatformCoreDataKeys import com.intellij.openapi.actionSystem.PlatformDataKeys import com.intellij.openapi.actionSystem.ex.ActionUtil +import com.intellij.openapi.actionSystem.ex.ActionUtil.performDumbAwareWithCallbacks import com.intellij.openapi.actionSystem.impl.ProxyShortcutSet import com.intellij.openapi.command.CommandProcessor import com.intellij.openapi.command.UndoConfirmationPolicy import com.intellij.openapi.components.Service -import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.editor.actionSystem.DocCommandGroupId -import com.intellij.openapi.util.ActionCallback +import com.intellij.openapi.ui.popup.JBPopupFactory import com.intellij.openapi.util.NlsContexts -import com.intellij.openapi.util.await -import com.intellij.openapi.util.registry.Registry import com.maddyhome.idea.vim.RegisterActions import com.maddyhome.idea.vim.api.ExecutionContext import com.maddyhome.idea.vim.api.NativeAction @@ -37,11 +36,10 @@ import com.maddyhome.idea.vim.handler.EditorActionHandlerBase import com.maddyhome.idea.vim.newapi.IjNativeAction import com.maddyhome.idea.vim.newapi.ij import com.maddyhome.idea.vim.newapi.runFromVimKey -import com.maddyhome.idea.vim.newapi.runningIJAction -import kotlinx.coroutines.runBlocking import org.jetbrains.annotations.NonNls import java.awt.Component import javax.swing.JComponent +import javax.swing.SwingUtilities @Service internal class IjActionExecutor : VimActionExecutor { @@ -76,53 +74,45 @@ internal class IjActionExecutor : VimActionExecutor { val dataContext = DataContextWrapper(context.ij) dataContext.putUserData(runFromVimKey, true) - val contextComponent = PlatformCoreDataKeys.CONTEXT_COMPONENT.getData(dataContext) ?: editor?.ij?.component ?: run { - LOG.error("Cannot get context component") - return false + val actionId = ActionManager.getInstance().getId(ijAction) + val event = AnActionEvent( + null, + dataContext, + ActionPlaces.KEYBOARD_SHORTCUT, + ijAction.templatePresentation.clone(), + ActionManager.getInstance(), + 0, + ) + // beforeActionPerformedUpdate should be called to update the action. It fixes some rider-specific problems. + // because rider use async update method. See VIM-1819. + // This method executes inside of lastUpdateAndCheckDumb + // Another related issue: VIM-2604 + + // This is a hack to fix the tests and fix VIM-3332 + // We should get rid of it in VIM-3376 + if (actionId == "RunClass" || actionId == IdeActions.ACTION_COMMENT_LINE || actionId == IdeActions.ACTION_COMMENT_BLOCK) { + ijAction.beforeActionPerformedUpdate(event) + if (!event.presentation.isEnabled) return false + } else { + if (!ActionUtil.lastUpdateAndCheckDumb(ijAction, event, false)) return false } - - val result = withRunningAction { - val result = withStringRegistryOption { - ActionManager.getInstance() - .tryToExecute(ijAction, null, contextComponent, ActionPlaces.KEYBOARD_SHORTCUT, true) - } - result.wait() - } - return result.isDone - } - - private fun withRunningAction(block: () -> T): T { - runningIJAction = true - try { - return block() - } finally { - runningIJAction = false - } - } - - private fun ActionCallback.wait(): ActionCallback { - runBlocking { - try { - await() - } catch (_: RuntimeException) { - // Nothing - // The exception happens when the action is rejected - // and the exception message explains the reason for rejection - // At the moment, we don't process this information + if (ijAction is ActionGroup && !event.presentation.isPerformGroup) { + // Some ActionGroups should not be performed, but shown as a popup + val popup = JBPopupFactory.getInstance() + .createActionGroupPopup(event.presentation.text, ijAction, dataContext, false, null, -1) + val component = dataContext.getData(PlatformDataKeys.CONTEXT_COMPONENT) + if (component != null) { + val window = SwingUtilities.getWindowAncestor(component) + if (window != null) { + popup.showInCenterOf(window) + } + return true } - } - return this - } - - @Suppress("SameParameterValue") - private fun withStringRegistryOption(block: () -> T): T { - val registry = Registry.get("actionSystem.update.beforeActionPerformedUpdate") - val oldValue = registry.asString() - registry.setValue("on") - try { - return block() - } finally { - registry.setValue(oldValue) + popup.showInFocusCenter() + return true + } else { + performDumbAwareWithCallbacks(ijAction, event) { ijAction.actionPerformed(event) } + return true } } @@ -216,8 +206,4 @@ internal class IjActionExecutor : VimActionExecutor { override fun getActionIdList(idPrefix: String): List { return ActionManager.getInstance().getActionIdList(idPrefix) } - - companion object { - private val LOG = logger() - } } diff --git a/src/main/java/com/maddyhome/idea/vim/newapi/IjEditorExecutionContext.kt b/src/main/java/com/maddyhome/idea/vim/newapi/IjEditorExecutionContext.kt index 076eb7400c..2f0a0a0755 100644 --- a/src/main/java/com/maddyhome/idea/vim/newapi/IjEditorExecutionContext.kt +++ b/src/main/java/com/maddyhome/idea/vim/newapi/IjEditorExecutionContext.kt @@ -18,11 +18,6 @@ internal open class IjEditorExecutionContext(override val context: DataContext) // This key is stored in data context when the action is started from vim internal val runFromVimKey = Key.create("RunFromVim") -/** - * Sometimes we can't rely on [runFromVimKey] because the data context is created after the execution - */ -internal var runningIJAction: Boolean = false - /** * Check if the action with this data context was started from Vim */ diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt index e8cd96fe3b..b7ac887968 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt @@ -16,6 +16,7 @@ import org.jetbrains.plugins.ideavim.SkipNeovimReason import org.jetbrains.plugins.ideavim.TestWithoutNeovim import org.jetbrains.plugins.ideavim.VimBehaviorDiffers import org.jetbrains.plugins.ideavim.VimTestCase +import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test /** @@ -50,6 +51,7 @@ class MotionActionTest : VimTestCase() { } @Test + @Disabled("VIM-3376") fun testEscapeInCommand() { val content = """ on${c}e two diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/select/SelectKeyHandlerTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/select/SelectKeyHandlerTest.kt index dabc5c3c46..60ad033deb 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/select/SelectKeyHandlerTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/select/SelectKeyHandlerTest.kt @@ -13,6 +13,7 @@ import org.jetbrains.plugins.ideavim.SkipNeovimReason import org.jetbrains.plugins.ideavim.TestWithoutNeovim import org.jetbrains.plugins.ideavim.VimBehaviorDiffers import org.jetbrains.plugins.ideavim.VimTestCase +import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test /** @@ -73,6 +74,7 @@ class SelectKeyHandlerTest : VimTestCase() { @TestWithoutNeovim(SkipNeovimReason.SELECT_MODE) @Test + @Disabled("VIM-3376") fun `test char mode backspace`() { this.doTest( listOf("gh", ""), @@ -98,6 +100,7 @@ class SelectKeyHandlerTest : VimTestCase() { @TestWithoutNeovim(SkipNeovimReason.SELECT_MODE) @Test + @Disabled("VIM-3376") fun `test char mode delete`() { this.doTest( listOf("gh", ""),