From 601fdf1c003538396000f1fde52223c528560058 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Mon, 31 Jan 2022 15:11:40 +0000 Subject: [PATCH 1/2] fix(VIM-2470): Fix incorrect reset of cursor shape --- .../idea/vim/option/GuiCursorOption.kt | 26 ++++---- .../options/helpers/GuiCursorOptionHelper.kt | 36 ++++++----- .../jetbrains/plugins/ideavim/VimTestCase.kt | 2 +- .../helper/CaretVisualAttributesHelperTest.kt | 17 ++++++ .../ideavim/option/GuiCursorOptionTest.kt | 61 ++++++++++--------- 5 files changed, 86 insertions(+), 56 deletions(-) diff --git a/src/main/java/com/maddyhome/idea/vim/option/GuiCursorOption.kt b/src/main/java/com/maddyhome/idea/vim/option/GuiCursorOption.kt index 2e250b9c30..d2abc21411 100644 --- a/src/main/java/com/maddyhome/idea/vim/option/GuiCursorOption.kt +++ b/src/main/java/com/maddyhome/idea/vim/option/GuiCursorOption.kt @@ -88,7 +88,7 @@ class GuiCursorOption(name: String, abbrev: String, defaultValue: String) : } } - return GuiCursorEntry(token, modes, GuiCursorAttributes(type, thickness, highlightGroup, lmapHighlightGroup, blinkModes)) + return GuiCursorEntry(token, modes, type, thickness, highlightGroup, lmapHighlightGroup, blinkModes) } override fun onChanged(oldValue: String?, newValue: String?) { @@ -103,18 +103,22 @@ class GuiCursorOption(name: String, abbrev: String, defaultValue: String) : var highlightGroup = "" var lmapHighlightGroup = "" var blinkModes = emptyList() - values().forEach { state -> - if (state.modes.contains(mode) || state.modes.contains(GuiCursorMode.ALL)) { - type = state.attributes.type - thickness = state.attributes.thickness - if (state.attributes.highlightGroup.isNotEmpty()) { - highlightGroup = state.attributes.highlightGroup + values().forEach { data -> + if (data.modes.contains(mode) || data.modes.contains(GuiCursorMode.ALL)) { + if (data.type != null) { + type = data.type } - if (state.attributes.lmapHighlightGroup.isNotEmpty()) { - lmapHighlightGroup = state.attributes.lmapHighlightGroup + if (data.thickness != null) { + thickness = data.thickness } - if (state.attributes.blinkModes.isNotEmpty()) { - blinkModes = state.attributes.blinkModes + if (data.highlightGroup.isNotEmpty()) { + highlightGroup = data.highlightGroup + } + if (data.lmapHighlightGroup.isNotEmpty()) { + lmapHighlightGroup = data.lmapHighlightGroup + } + if (data.blinkModes.isNotEmpty()) { + blinkModes = data.blinkModes } } } diff --git a/src/main/java/com/maddyhome/idea/vim/vimscript/model/options/helpers/GuiCursorOptionHelper.kt b/src/main/java/com/maddyhome/idea/vim/vimscript/model/options/helpers/GuiCursorOptionHelper.kt index 04ca33793c..eb5589f603 100644 --- a/src/main/java/com/maddyhome/idea/vim/vimscript/model/options/helpers/GuiCursorOptionHelper.kt +++ b/src/main/java/com/maddyhome/idea/vim/vimscript/model/options/helpers/GuiCursorOptionHelper.kt @@ -48,8 +48,8 @@ object GuiCursorOptionHelper { } ) - var type = GuiCursorType.BLOCK - var thickness = 0 + var type: GuiCursorType? = null + var thickness: Int? = null var highlightGroup = "" var lmapHighlightGroup = "" val blinkModes = mutableListOf() @@ -83,7 +83,7 @@ object GuiCursorOptionHelper { } } - return GuiCursorEntry(token, modes, GuiCursorAttributes(type, thickness, highlightGroup, lmapHighlightGroup, blinkModes)) + return GuiCursorEntry(token, modes, type, thickness, highlightGroup, lmapHighlightGroup, blinkModes) } fun getAttributes(mode: GuiCursorMode): GuiCursorAttributes { @@ -96,18 +96,22 @@ object GuiCursorOptionHelper { (VimPlugin.getOptionService().getOptionValue(OptionService.Scope.GLOBAL, OptionConstants.guicursorName) as VimString).value .split(",") .map { convertToken(it) } - .forEach { state -> - if (state.modes.contains(mode) || state.modes.contains(GuiCursorMode.ALL)) { - type = state.attributes.type - thickness = state.attributes.thickness - if (state.attributes.highlightGroup.isNotEmpty()) { - highlightGroup = state.attributes.highlightGroup + .forEach { data -> + if (data.modes.contains(mode) || data.modes.contains(GuiCursorMode.ALL)) { + if (data.type != null) { + type = data.type } - if (state.attributes.lmapHighlightGroup.isNotEmpty()) { - lmapHighlightGroup = state.attributes.lmapHighlightGroup + if (data.thickness != null) { + thickness = data.thickness } - if (state.attributes.blinkModes.isNotEmpty()) { - blinkModes = state.attributes.blinkModes + if (data.highlightGroup.isNotEmpty()) { + highlightGroup = data.highlightGroup + } + if (data.lmapHighlightGroup.isNotEmpty()) { + lmapHighlightGroup = data.lmapHighlightGroup + } + if (data.blinkModes.isNotEmpty()) { + blinkModes = data.blinkModes } } } @@ -149,7 +153,11 @@ enum class GuiCursorType(val token: String) { class GuiCursorEntry( private val originalString: String, val modes: EnumSet, - val attributes: GuiCursorAttributes + val type: GuiCursorType?, + val thickness: Int?, + val highlightGroup: String, + val lmapHighlightGroup: String, + val blinkModes: List ) { override fun toString(): String { // We need to match the original string for output and remove purposes diff --git a/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt b/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt index a0cf7a80fc..1828578d0c 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt @@ -99,7 +99,7 @@ abstract class VimTestCase : UsefulTestCase() { super.setUp() val factory = IdeaTestFixtureFactory.getFixtureFactory() val projectDescriptor = LightProjectDescriptor.EMPTY_PROJECT_DESCRIPTOR - val fixtureBuilder = factory.createLightFixtureBuilder(projectDescriptor) + val fixtureBuilder = factory.createLightFixtureBuilder(projectDescriptor, "ideavim-test") val fixture = fixtureBuilder.fixture myFixture = IdeaTestFixtureFactory.getFixtureFactory().createCodeInsightFixture( fixture, diff --git a/src/test/java/org/jetbrains/plugins/ideavim/helper/CaretVisualAttributesHelperTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/helper/CaretVisualAttributesHelperTest.kt index e600856245..e1f50d4a0a 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/helper/CaretVisualAttributesHelperTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/helper/CaretVisualAttributesHelperTest.kt @@ -216,6 +216,15 @@ class CaretVisualAttributesHelperTest : VimTestCase() { assertCaretVisualAttributes("UNDERSCORE", 0.75F) } + @TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING) + fun `test block used when caret shape is unspecified`() { + configureByText("I found it in a legendary land") + enterCommand("set guicursor=c:ver25") + assertCaretVisualAttributes("BLOCK", 0.0F) + typeText(parseKeys("i")) + assertCaretVisualAttributes("BLOCK", 0.0F) + } + @TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING) fun `test 'all' guicursor option`() { configureByText("I found it in a legendary land") @@ -223,6 +232,14 @@ class CaretVisualAttributesHelperTest : VimTestCase() { assertCaretVisualAttributes("BAR", 0.25F) } + @TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING) + fun `test 'all' guicursor option without cursor shape does not affect existing shapes`() { + configureByText("I found it in a legendary land") + enterCommand("set guicursor+=a:blinkwait200-blinkoff125-blinkon150-Cursor/lCursor") + typeText(parseKeys("i")) + assertCaretVisualAttributes("BAR", 0.25F) + } + @TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING) fun `test 'all' guicursor option can be overridden`() { configureByText("I found it in a legendary land") diff --git a/src/test/java/org/jetbrains/plugins/ideavim/option/GuiCursorOptionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/option/GuiCursorOptionTest.kt index 2f2753f151..a6cf27527a 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/option/GuiCursorOptionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/option/GuiCursorOptionTest.kt @@ -45,42 +45,42 @@ class GuiCursorOptionTest : VimTestCase() { assertEquals(6, values.size) assertEquals(enumSetOf(GuiCursorMode.NORMAL, GuiCursorMode.VISUAL, GuiCursorMode.CMD_LINE), values[0].modes) - assertEquals(GuiCursorType.BLOCK, values[0].attributes.type) - assertEquals("Cursor", values[0].attributes.highlightGroup) - assertEquals("lCursor", values[0].attributes.lmapHighlightGroup) + assertEquals(GuiCursorType.BLOCK, values[0].type) + assertEquals("Cursor", values[0].highlightGroup) + assertEquals("lCursor", values[0].lmapHighlightGroup) assertEquals(enumSetOf(GuiCursorMode.VISUAL_EXCLUSIVE), values[1].modes) - assertEquals(GuiCursorType.VER, values[1].attributes.type) - assertEquals(35, values[1].attributes.thickness) - assertEquals("Cursor", values[1].attributes.highlightGroup) - assertEquals("", values[1].attributes.lmapHighlightGroup) + assertEquals(GuiCursorType.VER, values[1].type) + assertEquals(35, values[1].thickness) + assertEquals("Cursor", values[1].highlightGroup) + assertEquals("", values[1].lmapHighlightGroup) assertEquals(enumSetOf(GuiCursorMode.OP_PENDING), values[2].modes) - assertEquals(GuiCursorType.HOR, values[2].attributes.type) - assertEquals(50, values[2].attributes.thickness) - assertEquals("Cursor", values[2].attributes.highlightGroup) - assertEquals("", values[2].attributes.lmapHighlightGroup) + assertEquals(GuiCursorType.HOR, values[2].type) + assertEquals(50, values[2].thickness) + assertEquals("Cursor", values[2].highlightGroup) + assertEquals("", values[2].lmapHighlightGroup) assertEquals(enumSetOf(GuiCursorMode.INSERT, GuiCursorMode.CMD_LINE_INSERT), values[3].modes) - assertEquals(GuiCursorType.VER, values[3].attributes.type) - assertEquals(25, values[3].attributes.thickness) - assertEquals("Cursor", values[3].attributes.highlightGroup) - assertEquals("lCursor", values[3].attributes.lmapHighlightGroup) + assertEquals(GuiCursorType.VER, values[3].type) + assertEquals(25, values[3].thickness) + assertEquals("Cursor", values[3].highlightGroup) + assertEquals("lCursor", values[3].lmapHighlightGroup) assertEquals(enumSetOf(GuiCursorMode.REPLACE, GuiCursorMode.CMD_LINE_REPLACE), values[4].modes) - assertEquals(GuiCursorType.HOR, values[4].attributes.type) - assertEquals(20, values[4].attributes.thickness) - assertEquals("Cursor", values[4].attributes.highlightGroup) - assertEquals("lCursor", values[4].attributes.lmapHighlightGroup) + assertEquals(GuiCursorType.HOR, values[4].type) + assertEquals(20, values[4].thickness) + assertEquals("Cursor", values[4].highlightGroup) + assertEquals("lCursor", values[4].lmapHighlightGroup) assertEquals(enumSetOf(GuiCursorMode.SHOW_MATCH), values[5].modes) - assertEquals(GuiCursorType.BLOCK, values[5].attributes.type) - assertEquals("Cursor", values[5].attributes.highlightGroup) - assertEquals("", values[5].attributes.lmapHighlightGroup) - assertEquals(3, values[5].attributes.blinkModes.size) - assertEquals("blinkwait175", values[5].attributes.blinkModes[0]) - assertEquals("blinkoff150", values[5].attributes.blinkModes[1]) - assertEquals("blinkon175", values[5].attributes.blinkModes[2]) + assertEquals(GuiCursorType.BLOCK, values[5].type) + assertEquals("Cursor", values[5].highlightGroup) + assertEquals("", values[5].lmapHighlightGroup) + assertEquals(3, values[5].blinkModes.size) + assertEquals("blinkwait175", values[5].blinkModes[0]) + assertEquals("blinkoff150", values[5].blinkModes[1]) + assertEquals("blinkon175", values[5].blinkModes[2]) } fun `test ignores set with missing colon`() { @@ -118,15 +118,16 @@ class GuiCursorOptionTest : VimTestCase() { assertTrue(VimPlugin.getOptionService().isDefault(OptionService.Scope.GLOBAL, OptionConstants.guicursorName)) } - fun `test simple string means block caret and highlight group`() { + fun `test simple string means default caret and highlight group`() { VimPlugin.getOptionService().resetDefault(OptionService.Scope.GLOBAL, OptionConstants.guicursorName) setValue("n:MyHighlightGroup") val values = getOptionValue().split(",").map { GuiCursorOptionHelper.convertToken(it) } assertEquals(1, values.size) assertEquals(enumSetOf(GuiCursorMode.NORMAL), values[0].modes) - assertEquals(GuiCursorType.BLOCK, values[0].attributes.type) - assertEquals("MyHighlightGroup", values[0].attributes.highlightGroup) - assertEquals("", values[0].attributes.lmapHighlightGroup) + // null from convertToken and we'll give it a default value in getAttributes + assertEquals(null, values[0].type) + assertEquals("MyHighlightGroup", values[0].highlightGroup) + assertEquals("", values[0].lmapHighlightGroup) } fun `test get effective values`() { From 41ded9fcf0c839e5aa63c16a827a59caba573c0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Pl=C3=A1te?= Date: Tue, 8 Feb 2022 14:13:35 +0300 Subject: [PATCH 2/2] Remove unnecessary code update --- src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt b/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt index 1828578d0c..a0cf7a80fc 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt @@ -99,7 +99,7 @@ abstract class VimTestCase : UsefulTestCase() { super.setUp() val factory = IdeaTestFixtureFactory.getFixtureFactory() val projectDescriptor = LightProjectDescriptor.EMPTY_PROJECT_DESCRIPTOR - val fixtureBuilder = factory.createLightFixtureBuilder(projectDescriptor, "ideavim-test") + val fixtureBuilder = factory.createLightFixtureBuilder(projectDescriptor) val fixture = fixtureBuilder.fixture myFixture = IdeaTestFixtureFactory.getFixtureFactory().createCodeInsightFixture( fixture,