Skip to content

Commit

Permalink
[VIM-3577] Replace weak reference to focused editor to needed informa…
Browse files Browse the repository at this point in the history
…tion

The `editorInFocus` used the weak reference in order to avoid editor leaks. However, if the user is unlucky, the GC may be called in between the console closing and switching focus to the new editor. In this case, the logic breaks and the Editor remains in the insert mode.
Now, we store only the required information about the last used editor.
  • Loading branch information
AlexPl292 committed Dec 17, 2024
1 parent 4614e2a commit 5995a48
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.editor.EditorKind
import com.maddyhome.idea.vim.KeyHandler
import com.maddyhome.idea.vim.LastUsedEditorInfo
import com.maddyhome.idea.vim.VimPlugin
import com.maddyhome.idea.vim.api.ExecutionContext
import com.maddyhome.idea.vim.api.VimEditor
Expand All @@ -31,10 +32,9 @@ import com.maddyhome.idea.vim.state.mode.Mode
*/
class IJEditorFocusListener : EditorListener {
override fun focusGained(editor: VimEditor) {
val oldEditor = KeyHandler.getInstance().editorInFocus
if (oldEditor != null && oldEditor.ij == editor.ij) return

KeyHandler.getInstance().editorInFocus = editor
val oldEditorInfo = KeyHandler.getInstance().lastUsedEditorInfo
val currentEditorHashCode = editor.ij.hashCode()
if (oldEditorInfo.hash == currentEditorHashCode) return

// We add Vim bindings to all opened editors, including editors used as UI controls rather than just project file
// editors. This includes editors used as part of the UI, such as the VCS commit message, or used as read-only
Expand All @@ -57,15 +57,17 @@ class IJEditorFocusListener : EditorListener {
// to know that a read-only editor that is hosting a console view with a running process can be treated as writable.

val ijEditor = editor.ij
val isCurrentEditorTerminal = isTerminal(ijEditor)

KeyHandler.getInstance().lastUsedEditorInfo = LastUsedEditorInfo(currentEditorHashCode, isCurrentEditorTerminal)

val switchToInsertMode = Runnable {
val context: ExecutionContext = injector.executionContextManager.getEditorExecutionContext(editor)
VimPlugin.getChange().insertBeforeCursor(editor, context)
}
if (isTerminal(ijEditor) && !ijEditor.inInsertMode) {
if (isCurrentEditorTerminal && !ijEditor.inInsertMode) {
switchToInsertMode.run()
} else if (ijEditor.isInsertMode && ((oldEditor != null && isTerminal(oldEditor.ij)) || !ijEditor.document.isWritable)) {
} else if (ijEditor.isInsertMode && (oldEditorInfo.isTerminal || !ijEditor.document.isWritable)) {
val context: ExecutionContext = injector.executionContextManager.getEditorExecutionContext(editor)
val mode = injector.vimState.mode
when (mode) {
Expand Down
13 changes: 6 additions & 7 deletions vim-engine/src/main/kotlin/com/maddyhome/idea/vim/KeyHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import com.maddyhome.idea.vim.state.VimStateMachine
import com.maddyhome.idea.vim.state.mode.Mode
import com.maddyhome.idea.vim.state.mode.ReturnTo
import com.maddyhome.idea.vim.state.mode.returnTo
import java.lang.ref.WeakReference
import javax.swing.KeyStroke

/**
Expand All @@ -47,7 +46,6 @@ import javax.swing.KeyStroke
// 1. avoid using handleKeyRecursionCount & shouldRecord
// 2. maybe we can live without allowKeyMappings: Boolean & mappingCompleted: Boolean
class KeyHandler {
private var editorInFocusReference: WeakReference<VimEditor>? = null
private val keyConsumers: List<KeyConsumer> = listOf(ModalInputConsumer(), MappingProcessor, CommandCountConsumer(), DeleteCommandConsumer(), EditorResetConsumer(), CharArgumentConsumer(), RegisterConsumer(), DigraphConsumer(), CommandConsumer(), SelectRegisterConsumer(), ModeInputConsumer())
private var handleKeyRecursionCount = 0

Expand All @@ -64,11 +62,7 @@ class KeyHandler {
val keyStack: KeyStack = KeyStack()
val modalEntryKeys: MutableList<KeyStroke> = ArrayList()

var editorInFocus: VimEditor?
get() = editorInFocusReference?.get()
set(value) {
editorInFocusReference = WeakReference(value)
}
var lastUsedEditorInfo: LastUsedEditorInfo = LastUsedEditorInfo(-1, false)

/**
* This is the main key handler for the Vim plugin. Every keystroke not handled directly by Idea is sent here for
Expand Down Expand Up @@ -482,3 +476,8 @@ sealed interface KeyProcessResult {
}

typealias KeyProcessing = (KeyHandlerState, VimEditor, ExecutionContext) -> Unit

data class LastUsedEditorInfo(
val hash: Int,
val isTerminal: Boolean,
)

0 comments on commit 5995a48

Please sign in to comment.