Fix options initialisation in 2024.2+

src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt

@@ -12,15 +12,14 @@ import com.intellij.application.options.CodeStyle
 import com.intellij.codeStyle.AbstractConvertLineSeparatorsAction
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.editor.Editor
 import com.intellij.openapi.editor.EditorKind
 import com.intellij.openapi.editor.EditorSettings.LineNumerationType
 import com.intellij.openapi.editor.ScrollPositionCalculator
 import com.intellij.openapi.editor.ex.EditorEx
 import com.intellij.openapi.editor.ex.EditorSettingsExternalizable
 import com.intellij.openapi.editor.impl.softwrap.SoftWrapAppliancePlaces
 import com.intellij.openapi.fileEditor.FileDocumentManager
-import com.intellij.openapi.fileEditor.FileEditorManagerEvent
-import com.intellij.openapi.fileEditor.TextEditor
 import com.intellij.openapi.fileEditor.impl.LoadTextUtil
 import com.intellij.openapi.fileEditor.impl.text.TextEditorImpl
 import com.intellij.openapi.project.Project
@@ -158,25 +157,24 @@ internal class OptionGroup : VimOptionGroupBase(), IjVimOptionGroup, InternalOpt
   }
 
   companion object {
-    fun fileEditorManagerSelectionChangedCallback(event: FileEditorManagerEvent) {
-      // Vim only has one window, and it's not possible to close it. This means that editing a new file will always
-      // reuse an existing window (opening a new window will always open from an existing window). More importantly,
-      // this means that any newly edited file will always get up-to-date local-to-window options. A new window is based
-      // on the opening window (treated as split then edit, so copy local + per-window "global" window values, then
-      // apply the per-window "global" values) and an edit reapplies the per-window "global" values.
-      // If we close all windows, and open a new one, we can only use the per-window "global" values from the fallback
-      // window, but this is only initialised when we first read `~/.ideavimrc` during startup. Vim would use the values
-      // from the current window, so to simulate this, we should update the fallback window with the values from the
-      // window that was selected at the time that the last window was closed.
-      // Unfortunately, we can't reliably know if a closing editor is the selected editor. Instead, we rely on selection
-      // change events. If an editor is losing selection and there is no new selection, we can assume this means that
-      // the last editor has been closed, and use the closed editor to update the fallback window
-      //
-      // XXX: event.oldEditor will must probably return a disposed editor. So, it should be treated with care
-      if (event.newEditor == null) {
-        (event.oldEditor as? TextEditor)?.editor?.let {
-          (VimPlugin.getOptionGroup() as OptionGroup).updateFallbackWindow(injector.fallbackWindow, it.vim)
-        }
+    fun editorReleased(editor: Editor) {
+      // Vim always has at least one window; it's not possible to close it. Editing a new file will open a new buffer in
+      // the current window, or it's possible to split the current buffer into a new window, or open a new buffer in a
+      // new window. This is important for us because when Vim opens a new window, the new window's local options are
+      // copied from the current window.
+      // In detail: splitting the current window gets a complete copy of local and per-window global option values.
+      // Editing a new file will split the current window and then edit the new buffer in-place.
+      // IntelliJ does not always have an open window. It would be weird to close the last editor tab, and then open
+      // the next tab with different options - the user would expect the editor to look like the last one did.
+      // Therefore, we have a dummy "fallback" window that captures the options of the last closed editor. When opening
+      // an editor and there are no currently open editors, we use the fallback window to initialise the new window.
+      // This callback tracks when editors are closed, and if the last editor in a project is being closed, updates the
+      // fallback window's options.
+      val project = editor.project ?: return
+      if (!injector.editorGroup.getEditorsRaw()
+          .any { it.ij != editor && it.ij.project === project && it.ij.editorKind == EditorKind.MAIN_EDITOR }
+      ) {
+        (VimPlugin.getOptionGroup() as OptionGroup).updateFallbackWindow(injector.fallbackWindow, editor.vim)
       }
     }
   }

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
@@ -386,10 +403,8 @@ internal object VimListenerManager {
       MotionGroup.fileEditorManagerSelectionChangedCallback(event)
       FileGroup.fileEditorManagerSelectionChangedCallback(event)
       VimPlugin.getSearch().fileEditorManagerSelectionChangedCallback(event)
-      SlowOperations.knownIssue("VIM-3658").use {
-        OptionGroup.fileEditorManagerSelectionChangedCallback(event)
-      }
       IjVimRedrawService.fileEditorManagerSelectionChangedCallback(event)
+      VimLastSelectedEditorTracker.setLastSelectedEditor(event.newEditor)
     }
   }
 
@@ -466,6 +481,18 @@ internal object VimListenerManager {
       EditorListeners.remove(event.editor)
       injector.listenersNotifier.notifyEditorReleased(vimEditor)
       injector.markService.editorReleased(vimEditor)
+
+      // This ticket will have a different stack trace, but it's the same problem. Originally, we tracked the last
+      // editor closing based on file selection (closing an editor would select the next editor - so a null selection
+      // was taken to mean that there were no more editors to select). This assumption broke in 242, so it's changed to
+      // check when the editor is released.
+      // However, the actions taken when the last editor closes can still be expensive/slow because we copy options, and
+      // some options are backed by PSI options. E.g. 'textwidth' is mapped to
+      // CodeStyle.getSettings(ijEditor).isWrapOnTyping(language)), and getting the document's PSI language is a slow
+      // operation. This underlying issue still needs to be addressed, even though the method has moved
+      SlowOperations.knownIssue("VIM-3658").use {
+        OptionGroup.editorReleased(event.editor)
+      }
     }
 
     override fun fileOpenedSync(