Manually track last selected editor

This is important for initialising options. We can't rely on FileEditorManager.selectedTextEditor, as 2024.2 changed behaviour to reset to null during creation of a new editor. This fixes tests (and option initialisation) for 242.

src/main/java/com/maddyhome/idea/vim/listener/VimListenerManager.kt

@@ -34,6 +34,7 @@ import com.intellij.openapi.editor.ex.DocumentEx
 import com.intellij.openapi.editor.ex.EditorEx
 import com.intellij.openapi.editor.ex.FocusChangeListener
 import com.intellij.openapi.editor.impl.EditorComponentImpl
+import com.intellij.openapi.fileEditor.FileEditor
 import com.intellij.openapi.fileEditor.FileEditorManager
 import com.intellij.openapi.fileEditor.FileEditorManagerEvent
 import com.intellij.openapi.fileEditor.FileEditorManagerListener
@@ -44,6 +45,7 @@ import com.intellij.openapi.fileEditor.ex.FileEditorWithProvider
 import com.intellij.openapi.fileEditor.impl.EditorComposite
 import com.intellij.openapi.fileEditor.impl.EditorWindow
 import com.intellij.openapi.observable.util.addKeyListener
+import com.intellij.openapi.project.Project
 import com.intellij.openapi.project.ProjectManager
 import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.util.Key
@@ -103,8 +105,11 @@ import com.maddyhome.idea.vim.ui.widgets.macro.MacroWidgetListener
 import com.maddyhome.idea.vim.ui.widgets.macro.macroWidgetOptionListener
 import com.maddyhome.idea.vim.ui.widgets.mode.listeners.ModeWidgetListener
 import com.maddyhome.idea.vim.ui.widgets.mode.modeWidgetOptionListener
+import org.jetbrains.annotations.TestOnly
 import java.awt.event.MouseAdapter
 import java.awt.event.MouseEvent
+import java.lang.ref.WeakReference
+import java.util.WeakHashMap
 import javax.swing.SwingUtilities
 
 /**
@@ -126,23 +131,15 @@ import javax.swing.SwingUtilities
  * Make sure the selected editor isn't the new editor, which can happen if there are no other editors open.
  */
 private fun getOpeningEditor(newEditor: Editor) = newEditor.project?.let { project ->
-  // Some TextEditor implementations create a dummy Editor instance on demand, e.g., while downloading a file to edit
-  // (see BaseRemoteFileEditor). This can cause recursion if the newly opened/created TextEditor is also the currently
-  // selected TextEditor, because we will be notified of the new dummy Editor before it has finished initialisation, and
-  // try to get its opening editor, causing a new dummy Editor to be created and notifications sent, and so on.
-  // This was reported for 232 and 233 (see VIM-3066), but I can't recreate in 241. The callstack looks different, now
-  // using coroutines, so it's possible the deadlock has been broken. However, it's sensible to leave the recursion
-  // guard in.
-  if (openingEditorRecursionGuard) return null
-  openingEditorRecursionGuard = true
-  try {
-    FileEditorManager.getInstance(project).selectedTextEditor?.takeUnless { it == newEditor }
-  }
-  finally {
-    openingEditorRecursionGuard = false
-  }
+  // We can't rely on FileEditorManager.selectedTextEditor because we're trying to retrieve the selected text editor
+  // while creating a text editor that is about to become the selected text editor.
+  // This worked fine for 2024.2, but internal changes for 2024.3 broke things. It appears that the currently selected
+  // text editor is reset to null while the soon-to-be-selected text editor is being created. We therefore track the
+  // last selected editor manually.
+  // Note that if we ever switch back to FileEditorManager.selectedTextEditor, be careful of recursion, because the
+  // actual editor might be created on-demand, which would notify our initialisation method, which would call us...
+  VimListenerManager.VimLastSelectedEditorTracker.getLastSelectedEditor(project)?.takeUnless { it == newEditor }
 }
-private var openingEditorRecursionGuard = false
 
 internal object VimListenerManager {
 
@@ -373,6 +370,26 @@ internal object VimListenerManager {
     }
   }
 
+  internal object VimLastSelectedEditorTracker {
+    // This stores a weak reference to an editor against a weak reference to a project, which means there is nothing
+    // keeping the project or editor from being garbage collected at any time. Stale keys are automatically expunged
+    // whenever the map is used.
+    private val selectedEditors = WeakHashMap<Project, WeakReference<Editor>>()
+
+    fun getLastSelectedEditor(project: Project): Editor? = selectedEditors[project]?.get()
+
+    internal fun setLastSelectedEditor(fileEditor: FileEditor?) {
+      (fileEditor as? TextEditor)?.editor?.let { editor ->
+        editor.project?.let { project -> selectedEditors[project] = WeakReference(editor) }
+      }
+    }
+
+    @TestOnly
+    internal fun resetLastSelectedEditor(project: Project) {
+      selectedEditors.remove(project)
+    }
+  }
+
   /**
    * Called when the selected file editor changes. In other words, when the user selects a new tab. Used to remember the
    * last selected file, update search highlights in the new tab, etc. This will be called with non-local Code With Me
@@ -387,6 +404,7 @@ internal object VimListenerManager {
       FileGroup.fileEditorManagerSelectionChangedCallback(event)
       VimPlugin.getSearch().fileEditorManagerSelectionChangedCallback(event)
       IjVimRedrawService.fileEditorManagerSelectionChangedCallback(event)
+      VimLastSelectedEditorTracker.setLastSelectedEditor(event.newEditor)
     }
   }
 

src/test/java/org/jetbrains/plugins/ideavim/option/overrides/WrapOptionMapperTest.kt

@@ -21,7 +21,6 @@ import org.jetbrains.plugins.ideavim.SkipNeovimReason
 import org.jetbrains.plugins.ideavim.TestWithoutNeovim
 import org.jetbrains.plugins.ideavim.VimTestCase
 import org.junit.jupiter.api.BeforeEach
-import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.TestInfo
 import kotlin.test.assertFalse
@@ -248,7 +247,6 @@ class WrapOptionMapperTest : VimTestCase() {
   }
 
   @Test
-  @Disabled("Doesn't work in 242")
   fun `test open new window after setting option copies value as explicitly set`() {
     enterCommand("set nowrap")
     assertCommandOutput("set wrap?", "nowrap")
@@ -278,7 +276,6 @@ class WrapOptionMapperTest : VimTestCase() {
   }
 
   @Test
-  @Disabled("Doesn't work in 242")
   fun `test setlocal 'wrap' then open new window uses value from setglobal`() {
     enterCommand("setlocal nowrap")
     assertCommandOutput("setglobal wrap?", "  wrap")
@@ -294,7 +291,6 @@ class WrapOptionMapperTest : VimTestCase() {
   }
 
   @Test
-  @Disabled("Doesn't work in 242")
   fun `test setting global IDE value will update effective Vim value in new window initialised from value set during startup`() {
     try {
       injector.optionGroup.startInitVimRc()

src/testFixtures/kotlin/org/jetbrains/plugins/ideavim/VimTestCase.kt

@@ -73,6 +73,7 @@ import com.maddyhome.idea.vim.helper.getGuiCursorMode
 import com.maddyhome.idea.vim.key.MappingOwner
 import com.maddyhome.idea.vim.key.ToKeysMappingInfo
 import com.maddyhome.idea.vim.listener.SelectionVimListenerSuppressor
+import com.maddyhome.idea.vim.listener.VimListenerManager
 import com.maddyhome.idea.vim.newapi.IjVimEditor
 import com.maddyhome.idea.vim.newapi.globalIjOptions
 import com.maddyhome.idea.vim.newapi.ijOptions
@@ -242,6 +243,7 @@ abstract class VimNoWriteActionTestCase {
     bookmarksManager?.bookmarks?.forEach { bookmark ->
       bookmarksManager.remove(bookmark)
     }
+    VimListenerManager.VimLastSelectedEditorTracker.resetLastSelectedEditor(fixture.project)
     SelectionVimListenerSuppressor.lock().use { fixture.tearDown() }
     ExEntryPanel.getInstance().deactivate(false)
     VimPlugin.getVariableService().clear()