From 916c5e48274843931fb3c0cc21ce270420ac9b4b Mon Sep 17 00:00:00 2001 From: NaMinhyeok Date: Tue, 26 Aug 2025 21:12:57 +0900 Subject: [PATCH 1/5] Prevent entering insert mode in read-only files Signed-off-by: NaMinhyeok --- .../change/insert/InsertExitModeActionTest.kt | 41 +++++++++++++++++++ .../change/insert/InsertNewLineBelowAction.kt | 8 ++++ .../idea/vim/api/VimChangeGroupBase.kt | 14 +++++++ 3 files changed, 63 insertions(+) diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt index c672e957b1..7e81312dce 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt @@ -11,6 +11,8 @@ package org.jetbrains.plugins.ideavim.action.change.insert import com.maddyhome.idea.vim.state.mode.Mode import org.jetbrains.plugins.ideavim.VimTestCase import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource class InsertExitModeActionTest : VimTestCase() { @Test @@ -22,4 +24,43 @@ class InsertExitModeActionTest : VimTestCase() { fun `test exit visual mode on line start`() { doTest("i", "${c}123", "${c}123", Mode.NORMAL()) } + + @ParameterizedTest + @ValueSource(strings = ["i", "a", "o", "O"]) + fun `test cannot enter insert mode in read-only file`(insertCommand: String) { + configureByText("12${c}3") + fixture.editor.document.setReadOnly(true) + + typeText(insertCommand) + + assertMode(Mode.NORMAL()) + assertState("12${c}3") + } + + @Test + fun `test cannot change text in read-only file with c motion`() { + configureByText("12${c}3") + fixture.editor.document.setReadOnly(true) + + // Try to change with 'cw' - should prevent the change + typeText("cw") + + // Should remain in normal mode + assertMode(Mode.NORMAL()) + assertState("12${c}3") + } + + @Test + fun `test cannot change text in read-only file with r motion`() { + configureByText("12${c}3") + fixture.editor.document.setReadOnly(true) + + // Try to change with 'r' - should prevent the change + typeText("rX") + + // Should remain in normal mode + assertMode(Mode.NORMAL()) + assertState("12${c}3") + + } } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt index 1c8e48e1ec..3dbdcc747b 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt @@ -107,6 +107,14 @@ private fun insertNewLineAbove(editor: VimEditor, context: ExecutionContext) { */ private fun insertNewLineBelow(editor: VimEditor, context: ExecutionContext) { if (editor.isOneLineMode()) return + + // Prevent entering insert mode in read-only files before moving the caret + if (!editor.isWritable()) { + injector.messages.showStatusBarMessage(editor, "Cannot make changes, file is read-only") + injector.messages.indicateError() + return + } + for (caret in editor.nativeCarets()) { caret.moveToOffset(injector.motion.moveCaretToCurrentLineEnd(editor, caret)) } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt index 438f241385..01621f70f2 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt @@ -404,6 +404,13 @@ abstract class VimChangeGroupBase : VimChangeGroup { * @param context The data context */ override fun insertAfterCaret(editor: VimEditor, context: ExecutionContext) { + // Prevent entering insert mode in read-only files before moving the caret + if (!editor.isWritable()) { + injector.messages.showStatusBarMessage(editor, "Cannot make changes, file is read-only") + injector.messages.indicateError() + return + } + for (caret in editor.nativeCarets()) { caret.moveToMotion(injector.motion.getHorizontalMotion(editor, caret, 1, true)) } @@ -441,6 +448,13 @@ abstract class VimChangeGroupBase : VimChangeGroup { * @param mode The mode - indicate insert or replace */ override fun initInsert(editor: VimEditor, context: ExecutionContext, mode: Mode) { + // Prevent entering insert mode in read-only files + if (!editor.isWritable()) { + injector.messages.showStatusBarMessage(editor, "Cannot make changes, file is read-only") + injector.messages.indicateError() + return + } + val state = injector.vimState injector.application.runReadAction { for (caret in editor.nativeCarets()) { From 40d26ce2b23b8dfdd9ccb3fa8495da433b2a4756 Mon Sep 17 00:00:00 2001 From: NaMinhyeok Date: Mon, 1 Sep 2025 13:11:43 +0900 Subject: [PATCH 2/5] Enhance InsertExitModeAction to handle read-only files gracefully and ensure ESC functionality Signed-off-by: NaMinhyeok --- .../change/insert/InsertExitModeActionTest.kt | 27 +++++++++++++++++++ .../change/insert/InsertExitModeAction.kt | 26 +++++++++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt index 7e81312dce..5cda3e0f2e 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt @@ -63,4 +63,31 @@ class InsertExitModeActionTest : VimTestCase() { assertState("12${c}3") } + + @Test + fun `test exit insert mode in read-only file`() { + // Test that ESC works in read-only files without hanging + configureByText("test ${c}content") + fixture.editor.document.setReadOnly(true) + + // This should complete without hanging or errors + typeText("i") + + // Reset read-only status + fixture.editor.document.setReadOnly(false) + } + + @Test + fun `test exit insert mode with repeat count in read-only file`() { + // Test that ESC works with repeat counts in read-only files + configureByText("${c}hello") + fixture.editor.document.setReadOnly(true) + + // 3i should repeat the insert 3 times when ESC is pressed + // In read-only files, this should exit cleanly without hanging + typeText("3i") + + // Reset read-only status + fixture.editor.document.setReadOnly(false) + } } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt index 1bfb0e8b57..06d2ccb105 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt @@ -11,16 +11,20 @@ import com.intellij.vim.annotations.CommandOrMotion import com.intellij.vim.annotations.Mode import com.maddyhome.idea.vim.api.ExecutionContext import com.maddyhome.idea.vim.api.VimEditor +import com.maddyhome.idea.vim.api.injector import com.maddyhome.idea.vim.command.Command import com.maddyhome.idea.vim.command.OperatorArguments import com.maddyhome.idea.vim.handler.VimActionHandler +import com.maddyhome.idea.vim.api.VimMarkService +import com.maddyhome.idea.vim.mark.VimMarkConstants.MARK_CHANGE_END +import com.maddyhome.idea.vim.state.mode.Mode as VimMode @CommandOrMotion(keys = ["", "", ""], modes = [Mode.INSERT]) class InsertExitModeAction : VimActionHandler.SingleExecution() { // Note that hitting Escape can insert text when exiting insert mode after visual block mode. - // If the editor is read-only, we'll get a "This view is read-only" tooltip. However, we should only enter insert - // mode if both editor and document are writable. - override val type: Command.Type = Command.Type.INSERT + // We use OTHER_SELF_SYNCHRONIZED to handle write locks manually, ensuring ESC always works + // even in read-only files by providing a fallback path that doesn't require write access. + override val type: Command.Type = Command.Type.OTHER_SELF_SYNCHRONIZED override fun execute( editor: VimEditor, @@ -28,7 +32,21 @@ class InsertExitModeAction : VimActionHandler.SingleExecution() { cmd: Command, operatorArguments: OperatorArguments, ): Boolean { - editor.exitInsertMode(context) + // Handle write locks manually since we use OTHER_SELF_SYNCHRONIZED + // Most of the time we don't need write access, only for repeat insert operations + try { + editor.exitInsertMode(context) + } catch (e: Exception) { + // If something fails, still try to exit insert mode without write operations + // This ensures ESC always works even in read-only files + val markGroup = injector.markService + markGroup.setMark(editor, VimMarkService.INSERT_EXIT_MARK) + markGroup.setMark(editor, MARK_CHANGE_END) + if (editor.mode is VimMode.REPLACE) { + editor.insertMode = true + } + editor.mode = VimMode.NORMAL() + } return true } } From 87e414dd46690998add26645d550b460390a9e30 Mon Sep 17 00:00:00 2001 From: NaMinhyeok Date: Mon, 1 Sep 2025 23:40:40 +0900 Subject: [PATCH 3/5] Refactor insert mode handling to check writability for new and repeat operations Signed-off-by: NaMinhyeok --- .../src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt index 3cded0b0b9..25e4abe4fa 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt @@ -50,6 +50,7 @@ sealed class Argument { constructor(motion: MotionActionHandler, argument: Argument?) : this(motion as EditorActionHandlerBase, argument) constructor(motion: TextObjectActionHandler) : this(motion as EditorActionHandlerBase) constructor(motion: ExternalActionHandler) : this(motion as EditorActionHandlerBase) + constructor(motion: EditorActionHandlerBase) : this(motion, null) fun getMotionType() = if (isLinewiseMotion()) SelectionType.LINE_WISE else SelectionType.CHARACTER_WISE @@ -57,7 +58,7 @@ sealed class Argument { is TextObjectActionHandler -> motion.visualType == TextObjectVisualType.LINE_WISE is MotionActionHandler -> motion.motionType == MotionType.LINE_WISE is ExternalActionHandler -> motion.isLinewiseMotion - else -> error("Command is not a motion: $motion") + else -> false // Default to character-wise for other actions } fun withArgument(argument: Argument) = Motion(motion, argument) From a5863ad9dfe58ba939f6d30735941387208da61b Mon Sep 17 00:00:00 2001 From: NaMinhyeok Date: Fri, 5 Sep 2025 21:24:24 +0900 Subject: [PATCH 4/5] revert CommandBuilder usage Signed-off-by: NaMinhyeok --- .../change/insert/InsertExitModeActionTest.kt | 7 +++++-- .../change/insert/InsertExitModeAction.kt | 20 ++----------------- .../change/insert/InsertNewLineBelowAction.kt | 9 +-------- .../maddyhome/idea/vim/command/Argument.kt | 3 +-- 4 files changed, 9 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt index 5cda3e0f2e..c65fe84af8 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertExitModeActionTest.kt @@ -27,11 +27,12 @@ class InsertExitModeActionTest : VimTestCase() { @ParameterizedTest @ValueSource(strings = ["i", "a", "o", "O"]) - fun `test cannot enter insert mode in read-only file`(insertCommand: String) { + fun `test read-only file allows insert entry but blocks changes`(insertCommand: String) { configureByText("12${c}3") fixture.editor.document.setReadOnly(true) - typeText(insertCommand) + // Enter insert-like commands, then immediately escape; no text should change and ESC should work + typeText("$insertCommand") assertMode(Mode.NORMAL()) assertState("12${c}3") @@ -86,6 +87,8 @@ class InsertExitModeActionTest : VimTestCase() { // 3i should repeat the insert 3 times when ESC is pressed // In read-only files, this should exit cleanly without hanging typeText("3i") + assertMode(Mode.NORMAL()) + assertState("${c}hello") // Reset read-only status fixture.editor.document.setReadOnly(false) diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt index 06d2ccb105..5fe8af5723 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertExitModeAction.kt @@ -21,9 +21,7 @@ import com.maddyhome.idea.vim.state.mode.Mode as VimMode @CommandOrMotion(keys = ["", "", ""], modes = [Mode.INSERT]) class InsertExitModeAction : VimActionHandler.SingleExecution() { - // Note that hitting Escape can insert text when exiting insert mode after visual block mode. - // We use OTHER_SELF_SYNCHRONIZED to handle write locks manually, ensuring ESC always works - // even in read-only files by providing a fallback path that doesn't require write access. + // Note: ESC should not require write access itself; any write is gated in processEscape. override val type: Command.Type = Command.Type.OTHER_SELF_SYNCHRONIZED override fun execute( @@ -32,21 +30,7 @@ class InsertExitModeAction : VimActionHandler.SingleExecution() { cmd: Command, operatorArguments: OperatorArguments, ): Boolean { - // Handle write locks manually since we use OTHER_SELF_SYNCHRONIZED - // Most of the time we don't need write access, only for repeat insert operations - try { - editor.exitInsertMode(context) - } catch (e: Exception) { - // If something fails, still try to exit insert mode without write operations - // This ensures ESC always works even in read-only files - val markGroup = injector.markService - markGroup.setMark(editor, VimMarkService.INSERT_EXIT_MARK) - markGroup.setMark(editor, MARK_CHANGE_END) - if (editor.mode is VimMode.REPLACE) { - editor.insertMode = true - } - editor.mode = VimMode.NORMAL() - } + editor.exitInsertMode(context) return true } } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt index 3dbdcc747b..5287af9bca 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/change/insert/InsertNewLineBelowAction.kt @@ -107,14 +107,7 @@ private fun insertNewLineAbove(editor: VimEditor, context: ExecutionContext) { */ private fun insertNewLineBelow(editor: VimEditor, context: ExecutionContext) { if (editor.isOneLineMode()) return - - // Prevent entering insert mode in read-only files before moving the caret - if (!editor.isWritable()) { - injector.messages.showStatusBarMessage(editor, "Cannot make changes, file is read-only") - injector.messages.indicateError() - return - } - + for (caret in editor.nativeCarets()) { caret.moveToOffset(injector.motion.moveCaretToCurrentLineEnd(editor, caret)) } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt index 25e4abe4fa..3cded0b0b9 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/command/Argument.kt @@ -50,7 +50,6 @@ sealed class Argument { constructor(motion: MotionActionHandler, argument: Argument?) : this(motion as EditorActionHandlerBase, argument) constructor(motion: TextObjectActionHandler) : this(motion as EditorActionHandlerBase) constructor(motion: ExternalActionHandler) : this(motion as EditorActionHandlerBase) - constructor(motion: EditorActionHandlerBase) : this(motion, null) fun getMotionType() = if (isLinewiseMotion()) SelectionType.LINE_WISE else SelectionType.CHARACTER_WISE @@ -58,7 +57,7 @@ sealed class Argument { is TextObjectActionHandler -> motion.visualType == TextObjectVisualType.LINE_WISE is MotionActionHandler -> motion.motionType == MotionType.LINE_WISE is ExternalActionHandler -> motion.isLinewiseMotion - else -> false // Default to character-wise for other actions + else -> error("Command is not a motion: $motion") } fun withArgument(argument: Argument) = Motion(motion, argument) From 7fc4ea0eab949ed6cca2881152c83afcdb9c7c49 Mon Sep 17 00:00:00 2001 From: NaMinhyeok Date: Thu, 30 Oct 2025 21:53:01 +0900 Subject: [PATCH 5/5] Prevent entering insert mode in read-only files Add writability check in initInsert() to prevent insert mode entry for read-only files. Shows dialog for temporarily read-only files(e.g., ~/ideavimrc) or blocks entry for non-writable files Signed-off-by: NaMinhyeok --- .../kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt index 01621f70f2..05e2c2441e 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroupBase.kt @@ -410,7 +410,7 @@ abstract class VimChangeGroupBase : VimChangeGroup { injector.messages.indicateError() return } - + for (caret in editor.nativeCarets()) { caret.moveToMotion(injector.motion.getHorizontalMotion(editor, caret, 1, true)) } @@ -454,7 +454,7 @@ abstract class VimChangeGroupBase : VimChangeGroup { injector.messages.indicateError() return } - + val state = injector.vimState injector.application.runReadAction { for (caret in editor.nativeCarets()) {