diff --git a/src/main/java/com/maddyhome/idea/vim/extension/highlightedyank/VimHighlightedYank.kt b/src/main/java/com/maddyhome/idea/vim/extension/highlightedyank/VimHighlightedYank.kt index bdb47cac3d..af0c7bc420 100644 --- a/src/main/java/com/maddyhome/idea/vim/extension/highlightedyank/VimHighlightedYank.kt +++ b/src/main/java/com/maddyhome/idea/vim/extension/highlightedyank/VimHighlightedYank.kt @@ -10,7 +10,7 @@ package com.maddyhome.idea.vim.extension.highlightedyank import com.intellij.ide.ui.LafManager import com.intellij.ide.ui.LafManagerListener -import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.ModalityState import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.colors.EditorColors import com.intellij.openapi.editor.markup.EffectType @@ -19,25 +19,24 @@ import com.intellij.openapi.editor.markup.HighlighterTargetArea import com.intellij.openapi.editor.markup.RangeHighlighter import com.intellij.openapi.editor.markup.TextAttributes import com.intellij.openapi.util.Disposer +import com.intellij.util.Alarm +import com.intellij.util.Alarm.ThreadToUse import com.maddyhome.idea.vim.VimPlugin -import com.maddyhome.idea.vim.VimProjectService import com.maddyhome.idea.vim.api.VimEditor import com.maddyhome.idea.vim.common.TextRange import com.maddyhome.idea.vim.extension.VimExtension import com.maddyhome.idea.vim.helper.MessageHelper -import com.maddyhome.idea.vim.helper.StrictMode import com.maddyhome.idea.vim.helper.VimNlsSafe import com.maddyhome.idea.vim.listener.VimInsertListener import com.maddyhome.idea.vim.listener.VimYankListener import com.maddyhome.idea.vim.newapi.ij +import com.maddyhome.idea.vim.vimscript.model.datatypes.VimDataType import com.maddyhome.idea.vim.vimscript.model.datatypes.VimString import org.jetbrains.annotations.NonNls import java.awt.Color import java.awt.Font -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit -internal const val DEFAULT_HIGHLIGHT_DURATION: Long = 300 +internal const val DEFAULT_HIGHLIGHT_DURATION: Int = 300 @NonNls private val HIGHLIGHT_DURATION_VARIABLE_NAME = "highlightedyank_highlight_duration" @@ -78,111 +77,128 @@ internal class HighlightColorResetter : LafManagerListener { */ internal class VimHighlightedYank : VimExtension, VimYankListener, VimInsertListener { private val highlightHandler = HighlightHandler() + private var initialised = false override fun getName() = "highlightedyank" override fun init() { + // Note that these listeners will still be registered when IdeaVim is disabled. However, they'll never get called VimPlugin.getYank().addListener(this) VimPlugin.getChange().addInsertListener(this) + + // Register our own disposable to remove highlights when IdeaVim is disabled. Somehow, we need to re-register when + // IdeaVim is re-enabled. We don't get a call back for that, but because the listeners are active until the + // _extension_ is disabled, make sure we're properly initialised each time we're called. + registerIdeaVimDisabledCallback() + initialised = true + } + + private fun registerIdeaVimDisabledCallback() { + // TODO: IdeaVim should help with lifecycle management here - VIM-3419 + // IdeaVim should possibly unregister extensions, but it would also need to re-register them. We have to do this + // state management manually for now + Disposer.register(VimPlugin.getInstance().onOffDisposable) { + highlightHandler.clearYankHighlighters() + initialised = false + } } override fun dispose() { + // Called when the extension is disabled with `:set no{extension}` or if the plugin owning the extension unloads VimPlugin.getYank().removeListener(this) VimPlugin.getChange().removeInsertListener(this) + + highlightHandler.clearYankHighlighters() + initialised = false } override fun yankPerformed(editor: VimEditor, range: TextRange) { + ensureInitialised() highlightHandler.highlightYankRange(editor.ij, range) } override fun insertModeStarted(editor: Editor) { - highlightHandler.clearAllYankHighlighters() + ensureInitialised() + highlightHandler.clearYankHighlighters() + } + + private fun ensureInitialised() { + if (!initialised) { + registerIdeaVimDisabledCallback() + initialised = true + } } private class HighlightHandler { - private var editor: Editor? = null - private val yankHighlighters: MutableSet = mutableSetOf() + private val alarm = Alarm(ThreadToUse.SWING_THREAD) + private var lastEditor: Editor? = null + private val highlighters = mutableSetOf() fun highlightYankRange(editor: Editor, range: TextRange) { // from vim-highlightedyank docs: When a new text is yanked or user starts editing, the old highlighting would be deleted - clearAllYankHighlighters() - - this.editor = editor - val project = editor.project - if (project != null) { - Disposer.register( - VimProjectService.getInstance(project), - ) { - this.editor = null - yankHighlighters.clear() - } - } - - if (range.isMultiple) { - for (i in 0 until range.size()) { - highlightSingleRange(editor, range.startOffsets[i]..range.endOffsets[i]) - } - } else { - highlightSingleRange(editor, range.startOffset..range.endOffset) - } - } - - fun clearAllYankHighlighters() { - yankHighlighters.forEach { highlighter -> - editor?.markupModel?.removeHighlighter(highlighter) ?: StrictMode.fail("Highlighters without an editor") + clearYankHighlighters() + + lastEditor = editor + + val attributes = getHighlightTextAttributes(editor) + for (i in 0 until range.size()) { + val highlighter = editor.markupModel.addRangeHighlighter( + range.startOffsets[i], + range.endOffsets[i], + HighlighterLayer.SELECTION, + attributes, + HighlighterTargetArea.EXACT_RANGE, + ) + highlighters.add(highlighter) } - yankHighlighters.clear() - } - - private fun highlightSingleRange(editor: Editor, range: ClosedRange) { - val highlighter = editor.markupModel.addRangeHighlighter( - range.start, - range.endInclusive, - HighlighterLayer.SELECTION, - getHighlightTextAttributes(), - HighlighterTargetArea.EXACT_RANGE, - ) - - yankHighlighters.add(highlighter) - - setClearHighlightRangeTimer(highlighter) - } - - private fun setClearHighlightRangeTimer(highlighter: RangeHighlighter) { - val timeout = extractUsersHighlightDuration() - // from vim-highlightedyank docs: A negative number makes the highlight persistent. + val timeout = extractUsersHighlightDuration() if (timeout >= 0) { - Executors.newSingleThreadScheduledExecutor().schedule( - { - ApplicationManager.getApplication().invokeLater { - editor?.markupModel?.removeHighlighter(highlighter) ?: StrictMode.fail("Highlighters without an editor") - } - }, + // Note modality. This is important when highlighting an editor when a modal dialog is open, such as the resolve + // conflict diff view + alarm.addRequest( + { clearYankHighlighters() }, timeout, - TimeUnit.MILLISECONDS, + ModalityState.any() ) } } - private fun getHighlightTextAttributes() = TextAttributes( + fun clearYankHighlighters() { + alarm.cancelAllRequests() + // Make sure the last editor we saved is still alive before we use it. We can't just use the list of open editors + // because this list is empty when IdeaVim is disabled, so we're unable to clean up + lastEditor?.let { editor -> + if (!editor.isDisposed) { + highlighters.forEach { highlighter -> editor.markupModel.removeHighlighter(highlighter) } + } + } + lastEditor = null + highlighters.clear() + } + + private fun getHighlightTextAttributes(editor: Editor) = TextAttributes( null, extractUsersHighlightColor(), - editor?.colorsScheme?.getColor(EditorColors.CARET_COLOR), + editor.colorsScheme.getColor(EditorColors.CARET_COLOR), EffectType.SEARCH_MATCH, Font.PLAIN, ) - private fun extractUsersHighlightDuration(): Long { + private fun extractUsersHighlightDuration(): Int { return extractVariable(HIGHLIGHT_DURATION_VARIABLE_NAME, DEFAULT_HIGHLIGHT_DURATION) { - it.toLong() + // toVimNumber will return 0 for an invalid string. Let's force it to throw + when (it) { + is VimString -> it.value.toInt() + else -> it.toVimNumber().value + } } } private fun extractUsersHighlightColor(): Color { return extractVariable(HIGHLIGHT_COLOR_VARIABLE_NAME, getDefaultHighlightTextColor()) { value -> - val rgba = value + val rgba = value.asString() .substring(4) .filter { it != '(' && it != ')' && !it.isWhitespace() } .split(',') @@ -192,12 +208,11 @@ internal class VimHighlightedYank : VimExtension, VimYankListener, VimInsertList } } - private fun extractVariable(variable: String, default: T, extractFun: (value: String) -> T): T { + private fun extractVariable(variable: String, default: T, extractFun: (value: VimDataType) -> T): T { val value = VimPlugin.getVariableService().getGlobalVariableValue(variable) - - if (value is VimString) { + if (value != null) { return try { - extractFun(value.value) + extractFun(value) } catch (e: Exception) { @VimNlsSafe val message = MessageHelper.message( "highlightedyank.invalid.value.of.0.1", diff --git a/src/test/java/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankTest.kt index 91ce2e7081..0fb898878c 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankTest.kt @@ -9,6 +9,8 @@ package org.jetbrains.plugins.ideavim.extension.highlightedyank import com.intellij.openapi.editor.markup.RangeHighlighter +import com.intellij.openapi.util.TextRange +import com.maddyhome.idea.vim.VimPlugin import com.maddyhome.idea.vim.extension.highlightedyank.DEFAULT_HIGHLIGHT_DURATION import com.maddyhome.idea.vim.state.mode.Mode import org.jetbrains.plugins.ideavim.VimTestCase @@ -16,6 +18,9 @@ import org.jetbrains.plugins.ideavim.assertHappened import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInfo +import java.awt.Color +import kotlin.test.assertEquals +import kotlin.test.assertTrue class VimHighlightedYankTest : VimTestCase() { @BeforeEach @@ -26,20 +31,37 @@ class VimHighlightedYankTest : VimTestCase() { @Test fun `test highlighting whole line when whole line is yanked`() { - doTest("yy", code, code, Mode.NORMAL()) + doTest("yy", code, code) assertAllHighlightersCount(1) assertHighlighterRange(1, 40, getFirstHighlighter()) } + @Test + fun `test highlighting multiple lines`() { + doTest("2yy", code, code) + + assertAllHighlightersCount(1) + assertHighlighterRange(1, 59, getFirstHighlighter()) + } + @Test fun `test highlighting single word when single word is yanked`() { - doTest("yiw", code, code, Mode.NORMAL()) + doTest("yiw", code, code) assertAllHighlightersCount(1) assertHighlighterRange(5, 8, getFirstHighlighter()) } + @Test + fun `test removing previous highlight when yanking again`() { + // Move the caret back to original position :) + doTest(listOf("yy", "j", "yy", "k"), code, code) + + assertAllHighlightersCount(1) + assertHighlighterRange(40, 59, getFirstHighlighter()) + } + @Test fun `test removing previous highlight when entering insert mode`() { doTest("yyi", code, code, Mode.INSERT) @@ -51,9 +73,8 @@ class VimHighlightedYankTest : VimTestCase() { fun `test highlighting with multiple cursors`() { doTest( "yiw", - codeWithMultipleCurors, - codeWithMultipleCurors, - Mode.NORMAL(), + codeWithMultipleCursors, + codeWithMultipleCursors, ) val highlighters = fixture.editor.markupModel.allHighlighters @@ -67,9 +88,9 @@ class VimHighlightedYankTest : VimTestCase() { fun `test clearing all highlighters with multiple cursors`() { doTest( "yiwi", - codeWithMultipleCurors, - codeWithMultipleCurors, -Mode.INSERT, + codeWithMultipleCursors, + codeWithMultipleCursors, + Mode.INSERT, ) assertAllHighlightersCount(0) @@ -77,37 +98,110 @@ Mode.INSERT, @Test fun `test highlighting for a correct default amount of time`() { - doTest("yiw", code, code, Mode.NORMAL()) + doTest("yiw", code, code) - assertHappened(DEFAULT_HIGHLIGHT_DURATION.toInt(), 200) { + assertHappened(DEFAULT_HIGHLIGHT_DURATION, 200) { getAllHighlightersCount() == 0 } } + @Test + fun `test highlighting for a correct user provided amount of time`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_duration = 1000") + typeText("yiw") + + assertHappened(1000, 200) { + getAllHighlightersCount() == 0 + } + } + + @Test + fun `test highlighting for a correct user provided amount of time when value passed as string`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_duration = \"1000\"") + typeText("yiw") + + assertHappened(1000, 200) { + getAllHighlightersCount() == 0 + } + } + + @Test + fun `test indicating error when incorrect highlight duration was provided by user`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_duration = \"500.15\"") + typeText("yy") + + assertEquals( + "highlightedyank: Invalid value of g:highlightedyank_highlight_duration -- For input string: \"500.15\"", + VimPlugin.getMessage(), + ) + } + + @Test + fun `test not indicating error when correct highlight duration was provided by user`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_duration = \"-1\"") + typeText("yy") + + assertEquals(null, VimPlugin.getMessage()) + } + + @Test + fun `test not indicating error when correct highlight duration was provided by user as string value`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_duration = \"-1\"") + typeText("yy") + + assertEquals(null, VimPlugin.getMessage()) + } + + @Test + fun `test custom colour used when provided by user`() { + configureByText(code) + enterCommand("let g:highlightedyank_highlight_color=\"rgba(100,10,20,30)\"") + typeText("yy") + + val highlighter = getFirstHighlighter() + assertEquals(Color(100, 10, 20, 30), highlighter.getTextAttributes(null)?.backgroundColor) + } + + @Test + fun `test indicating error when incorrect highlight color was provided by user`() { + configureByText(code) + + listOf("rgba(1,2,3)", "rgba(1, 2, 3, 0.1)", "rgb(1,2,3)", "rgba(260, 2, 5, 6)").forEach { color -> + enterCommand("let g:highlightedyank_highlight_color = \"$color\"") + typeText("yy") + + assertTrue( + VimPlugin.getMessage().contains("highlightedyank: Invalid value of g:highlightedyank_highlight_color"), + color, + ) + } + } + private val code = """ fun ${c}sum(x: Int, y: Int, z: Int): Int { return x + y + z } """ - private val codeWithMultipleCurors = """ + private val codeWithMultipleCursors = """ fun sum(x: ${c}Int, y: ${c}Int, z: ${c}Int): Int { return x + y + z } """ private fun assertHighlighterRange(start: Int, end: Int, highlighter: RangeHighlighter) { - kotlin.test.assertEquals(start, highlighter.startOffset) - kotlin.test.assertEquals(end, highlighter.endOffset) + assertEquals(TextRange(start, end), highlighter.textRange) } private fun assertAllHighlightersCount(count: Int) { - kotlin.test.assertEquals(count, getAllHighlightersCount()) + assertEquals(count, getAllHighlightersCount()) } private fun getAllHighlightersCount() = fixture.editor.markupModel.allHighlighters.size - - private fun getFirstHighlighter(): RangeHighlighter { - return fixture.editor.markupModel.allHighlighters.first() - } + private fun getFirstHighlighter(): RangeHighlighter = fixture.editor.markupModel.allHighlighters.first() } diff --git a/src/testFixtures/kotlin/org/jetbrains/plugins/ideavim/VimTestCase.kt b/src/testFixtures/kotlin/org/jetbrains/plugins/ideavim/VimTestCase.kt index 52b35d22f1..1dbddf17ca 100644 --- a/src/testFixtures/kotlin/org/jetbrains/plugins/ideavim/VimTestCase.kt +++ b/src/testFixtures/kotlin/org/jetbrains/plugins/ideavim/VimTestCase.kt @@ -159,7 +159,7 @@ abstract class VimTestCase { bookmarksManager?.bookmarks?.forEach { bookmark -> bookmarksManager.remove(bookmark) } - fixture.editor?.let { injector.messages.showStatusBarMessage(it.vim, "") } + injector.messages.showStatusBarMessage(null, null) SelectionVimListenerSuppressor.lock().use { fixture.tearDown() } ExEntryPanel.getInstance().deactivate(false) VimPlugin.getVariableService().clear() diff --git a/tests/java-tests/src/test/kotlin/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankJavaTest.kt b/tests/java-tests/src/test/kotlin/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankJavaTest.kt deleted file mode 100644 index 53c01e0fd1..0000000000 --- a/tests/java-tests/src/test/kotlin/org/jetbrains/plugins/ideavim/extension/highlightedyank/VimHighlightedYankJavaTest.kt +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright 2003-2024 The IdeaVim authors - * - * Use of this source code is governed by an MIT-style - * license that can be found in the LICENSE.txt file or at - * https://opensource.org/licenses/MIT. - */ - -package org.jetbrains.plugins.ideavim.extension.highlightedyank - -import com.intellij.openapi.editor.markup.RangeHighlighter -import com.maddyhome.idea.vim.VimPlugin -import com.maddyhome.idea.vim.api.injector -import org.jetbrains.plugins.ideavim.VimJavaTestCase -import org.jetbrains.plugins.ideavim.assertHappened -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test -import org.junit.jupiter.api.TestInfo - -class VimHighlightedYankJavaTest : VimJavaTestCase() { - @BeforeEach - override fun setUp(testInfo: TestInfo) { - super.setUp(testInfo) - enableExtensions("highlightedyank") - } - - @Test - fun `test removing previous highlight when new range is yanked`() { - configureByJavaText(code) - typeText(injector.parser.parseKeys("yyjyy")) - - assertAllHighlightersCount(1) - assertHighlighterRange(40, 59, getFirstHighlighter()) - } - - @Test - fun `test indicating error when incorrect highlight duration was provided by user`() { - configureByJavaText(code) - typeText(injector.parser.parseKeys(":let g:highlightedyank_highlight_duration = \"500.15\"")) - typeText(injector.parser.parseKeys("yy")) - - kotlin.test.assertEquals( - "highlightedyank: Invalid value of g:highlightedyank_highlight_duration -- For input string: \"500.15\"", - VimPlugin.getMessage(), - ) - } - - @Test - fun `test not indicating error when correct highlight duration was provided by user`() { - configureByJavaText(code) - typeText(injector.parser.parseKeys(":let g:highlightedyank_highlight_duration = \"-1\"")) - typeText(injector.parser.parseKeys("yy")) - - kotlin.test.assertEquals(VimPlugin.getMessage(), "") - } - - @Test - fun `test indicating error when incorrect highlight color was provided by user`() { - configureByJavaText(code) - - listOf("rgba(1,2,3)", "rgba(1, 2, 3, 0.1)", "rgb(1,2,3)", "rgba(260, 2, 5, 6)").forEach { color -> - typeText(injector.parser.parseKeys(":let g:highlightedyank_highlight_color = \"$color\"")) - typeText(injector.parser.parseKeys("yy")) - - kotlin.test.assertTrue( - VimPlugin.getMessage().contains("highlightedyank: Invalid value of g:highlightedyank_highlight_color"), - color, - ) - } - } - - @Test - fun `test indicating error when correct highlight color was provided by user`() { - configureByJavaText(code) - - listOf("rgba(1,2,3,5)", "rgba1, 2, 3, 1", "rgba(1, 2, 3, 4").forEach { color -> - typeText(injector.parser.parseKeys(":let g:highlightedyank_highlight_color = \"$color\"")) - typeText(injector.parser.parseKeys("yy")) - - kotlin.test.assertEquals("", VimPlugin.getMessage()) - } - } - - @Test - fun `test highlighting for a correct user provided amount of time`() { - configureByJavaText(code) - typeText(injector.parser.parseKeys(":let g:highlightedyank_highlight_duration = \"1000\"")) - typeText(injector.parser.parseKeys("yiw")) - - assertHappened(1000, 200) { - getAllHighlightersCount() == 0 - } - } - - private val code = """ -fun ${c}sum(x: Int, y: Int, z: Int): Int { - return x + y + z -} -""" - - private fun assertAllHighlightersCount(count: Int) { - kotlin.test.assertEquals(count, getAllHighlightersCount()) - } - - private fun getAllHighlightersCount() = fixture.editor.markupModel.allHighlighters.size - - private fun assertHighlighterRange(start: Int, end: Int, highlighter: RangeHighlighter) { - kotlin.test.assertEquals(start, highlighter.startOffset) - kotlin.test.assertEquals(end, highlighter.endOffset) - } - - private fun getFirstHighlighter(): RangeHighlighter { - return fixture.editor.markupModel.allHighlighters.first() - } -} \ No newline at end of file