diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt index 95015dfd1..b85b3c467 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/MotionActionTest.kt @@ -1044,10 +1044,19 @@ two @TestWithoutNeovim(reason = SkipNeovimReason.UNCLEAR) @Test fun testLastWord() { - typeTextInFile(injector.parser.parseKeys("w"), "${c}one\n") + typeTextInFile(injector.parser.parseKeys("w"), "${c}one") assertOffset(2) } + @TestWithoutNeovim(reason = SkipNeovimReason.UNCLEAR) + @Test + fun testLastWord2() { + typeTextInFile(injector.parser.parseKeys("w"), "${c}one\n") + // Counter-intuitive, but correct. There is no next word, so we get to the end of the current word, skip whitespace + // and move past the end of the file. Behaviour matches Vim - `w` will move to the next line (which is empty) + assertOffset(4) + } + // |b| @Test fun testWordBackwardsAtFirstLineWithWhitespaceInFront() { diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertDeletePreviousWordActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertDeletePreviousWordActionTest.kt index 2dd56ec1d..17bfaed9e 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertDeletePreviousWordActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/change/insert/InsertDeletePreviousWordActionTest.kt @@ -128,7 +128,7 @@ class InsertDeletePreviousWordActionTest : VimTestCase() { fun `test delete starting from the last character of the file`() { // This test was originally trying to delete the previous word with the caret positioned at the end of line and // recorded different behaviour to Vim. The problem was that the caret was incorrectly positioned _passed_ the end - // of the line, and indeed passed the end of the file. + // of the line, and indeed past the end of the file. // This placement is valid, both in IdeaVim and Vim, but only when `:set virtualedit=onemore` is set. The test was // showing a bug in the implementation in this scenario. The test is now explicit in what it's trying to do, and // matches Vim's behaviour. diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterWordActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterWordActionTest.kt index 1bfceb7e7..44e032c57 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterWordActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterWordActionTest.kt @@ -788,19 +788,12 @@ class MotionOuterWordActionTest : VimTestCase() { ) } - // TODO: Fix this bug - @VimBehaviorDiffers(originalVimAfter = - "Lorem${s} ipsum --dolor${c} ${se}sit amet, consectetur adipiscing elit", - description = "First aw should select whitespace+'ipsum' " + - "second should select whitespace+'--' " + - "third should select 'dolor' and following whitespace", - ) @Test fun `test select multiple outer words starting in whitespace`() { doTest( "v3aw", "Lorem ${c} ipsum --dolor sit amet, consectetur adipiscing elit", - "Lorem${s} ipsum --dolo${c}r${se} sit amet, consectetur adipiscing elit", + "Lorem${s} ipsum --dolor${c} ${se}sit amet, consectetur adipiscing elit", Mode.VISUAL(SelectionType.CHARACTER_WISE), ) } diff --git a/src/test/java/org/jetbrains/plugins/ideavim/helper/SearchHelperTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/helper/SearchHelperTest.kt index 41ddc0b7e..72cd05096 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/helper/SearchHelperTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/helper/SearchHelperTest.kt @@ -28,8 +28,7 @@ class SearchHelperTest : VimTestCase() { fun testFindNextWord() { val text = "first second" configureByText(text) - val nextWordPosition = - injector.searchHelper.findNextWord(fixture.editor.vim, 0, 1, bigWord = true, spaceWords = false) + val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 1, bigWord = true) kotlin.test.assertEquals(nextWordPosition, text.indexOf("second")) } @@ -38,7 +37,7 @@ class SearchHelperTest : VimTestCase() { fun testFindSecondNextWord() { val text = "first second third" configureByText(text) - val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 2, bigWord = true, false) + val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 2, bigWord = true) kotlin.test.assertEquals(nextWordPosition, text.indexOf("third")) } @@ -47,7 +46,7 @@ class SearchHelperTest : VimTestCase() { fun testFindAfterLastWord() { val text = "first second" configureByText(text) - val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 3, bigWord = true, false) + val nextWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, 0, 3, bigWord = true) kotlin.test.assertEquals(nextWordPosition, text.length) } @@ -57,7 +56,7 @@ class SearchHelperTest : VimTestCase() { val text = "first second" configureByText(text) val previousWordPosition = - injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -1, bigWord = true, false) + injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -1, bigWord = true) kotlin.test.assertEquals(previousWordPosition, text.indexOf("first")) } @@ -67,13 +66,7 @@ class SearchHelperTest : VimTestCase() { val text = "first second third" configureByText(text) val previousWordPosition = - injector.searchHelper.findNextWord( - fixture.editor.vim, - text.indexOf("third"), - -2, - bigWord = true, - spaceWords = false, - ) + injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("third"), -2, bigWord = true) kotlin.test.assertEquals(previousWordPosition, text.indexOf("first")) } @@ -83,13 +76,7 @@ class SearchHelperTest : VimTestCase() { val text = "first second" configureByText(text) val previousWordPosition = - injector.searchHelper.findNextWord( - fixture.editor.vim, - text.indexOf("second"), - -3, - bigWord = true, - spaceWords = false, - ) + injector.searchHelper.findNextWord(fixture.editor.vim, text.indexOf("second"), -3, bigWord = true) kotlin.test.assertEquals(previousWordPosition, text.indexOf("first")) } @@ -98,8 +85,7 @@ class SearchHelperTest : VimTestCase() { fun testFindPreviousWordWhenCursorOutOfBound() { val text = "first second" configureByText(text) - val previousWordPosition = - injector.searchHelper.findNextWord(fixture.editor.vim, text.length, -1, bigWord = true, spaceWords = false) + val previousWordPosition = injector.searchHelper.findNextWord(fixture.editor.vim, text.length, -1, bigWord = true) kotlin.test.assertEquals(previousWordPosition, text.indexOf("second")) } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/DeletePrevWordAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/DeletePrevWordAction.kt index 6e7c767f5..13bcb58ea 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/DeletePrevWordAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/DeletePrevWordAction.kt @@ -35,7 +35,7 @@ class DeletePrevWordAction : VimActionHandler.SingleExecution() { val oldText = commandLine.actualText val motion = - injector.motion.findOffsetOfNextWord(oldText, oldText.length, commandLine.caret.offset, -1, true, editor) + injector.motion.findOffsetOfNextWord(oldText, commandLine.caret.offset, -1, true, editor) when (motion) { is Motion.AbsoluteOffset -> { val newText = oldText.substring(0, motion.offset) + oldText.substring(caretOffset) @@ -48,4 +48,4 @@ class DeletePrevWordAction : VimActionHandler.SingleExecution() { return true } -} \ No newline at end of file +} diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToNextWordAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToNextWordAction.kt index a488de226..00af9e8be 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToNextWordAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToNextWordAction.kt @@ -31,7 +31,7 @@ class MoveToNextWordAction : VimActionHandler.SingleExecution() { val commandLine = injector.commandLine.getActiveCommandLine() ?: return false val text = commandLine.actualText - val motion = injector.motion.findOffsetOfNextWord(text, text.length, commandLine.caret.offset, 1, true, editor) + val motion = injector.motion.findOffsetOfNextWord(text, commandLine.caret.offset, 1, true, editor) when (motion) { is Motion.AbsoluteOffset -> { commandLine.caret.offset = motion.offset diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToPreviousWordAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToPreviousWordAction.kt index 48778d9af..6e6a661c1 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToPreviousWordAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/ex/MoveToPreviousWordAction.kt @@ -31,7 +31,7 @@ class MoveToPreviousWordAction : VimActionHandler.SingleExecution() { val commandLine = injector.commandLine.getActiveCommandLine() ?: return false val text = commandLine.actualText - val motion = injector.motion.findOffsetOfNextWord(text, text.length, commandLine.caret.offset, -1, true, editor) + val motion = injector.motion.findOffsetOfNextWord(text, commandLine.caret.offset, -1, true, editor) when (motion) { is Motion.AbsoluteOffset -> { commandLine.caret.offset = motion.offset @@ -41,4 +41,4 @@ class MoveToPreviousWordAction : VimActionHandler.SingleExecution() { } return true } -} \ No newline at end of file +} diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroup.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroup.kt index 954b8265a..cde5a4cfc 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroup.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroup.kt @@ -102,7 +102,6 @@ interface VimMotionGroup { * Find the offset of the start of the next/previous word/WORD in some text outside the editor (e.g., command line) * * @param text The text to search in - * @param textLength The text length (there is no guarantee that calling [text.length] will be a constant time operation) * @param searchFrom The buffer offset to start searching from * @param count The number of words to skip * @param bigWord If true then find WORD, if false then find word @@ -111,7 +110,6 @@ interface VimMotionGroup { */ fun findOffsetOfNextWord( text: CharSequence, - textLength: Int = text.length, searchFrom: Int, count: Int, bigWord: Boolean, diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroupBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroupBase.kt index 5e252d2d9..b779317e4 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroupBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimMotionGroupBase.kt @@ -97,29 +97,20 @@ abstract class VimMotionGroupBase : VimMotionGroup { * @return position */ override fun findOffsetOfNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean): Motion { - return findOffsetOfNextWord(editor.text(), editor.fileSize().toInt(), searchFrom, count, bigWord, editor) + return findOffsetOfNextWord(editor.text(), searchFrom, count, bigWord, editor) } override fun findOffsetOfNextWord( text: CharSequence, - textLength: Int, searchFrom: Int, count: Int, bigWord: Boolean, editor: VimEditor, ): Motion { - if ((searchFrom == 0 && count < 0) || (searchFrom >= textLength - 1 && count > 0)) { + if ((searchFrom == 0 && count < 0) || (searchFrom >= text.length - 1 && count > 0)) { return Motion.Error } - return (injector.searchHelper.findNextWord( - text, - textLength, - editor, - searchFrom, - count, - bigWord, - false - )).toMotionOrError() + return (injector.searchHelper.findNextWord(text, editor, searchFrom, count, bigWord)).toMotionOrError() } override fun getHorizontalMotion( diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelper.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelper.kt index 307f6c4fa..ac0fe26c6 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelper.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelper.kt @@ -99,36 +99,35 @@ interface VimSearchHelper { /** * Find the next word in the editor's document, from the given starting point * + * Note that this function can return an out of bounds index when there is no next word! + * * @param editor The editor's document to search in. Editor is required because word boundaries depend on * local-to-buffer options * @param searchFrom The offset in the document to search from * @param count Return an offset to the [count] word from the starting position. Will search backwards if negative * @param bigWord Use WORD instead of word boundaries - * @param spaceWords Include whitespace as part of a word, e.g. the difference between `iw` and `aw` motions * @return The offset of the [count] next word, or `0` or the offset of the end of file if not found */ - fun findNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, spaceWords: Boolean): Int + fun findNextWord(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean): Int /** * Find the next word in some text outside the editor (e.g., command line), from the given starting point * + * Note that this function can return an out of bounds index when there is no next word! + * * @param text The text to search in - * @param textLength The text length * @param editor Required because word boundaries depend on local-to-buffer options * @param searchFrom The offset in the document to search from * @param count Return an offset to the [count] word from the starting position. Will search backwards if negative * @param bigWord Use WORD instead of word boundaries - * @param spaceWords Include whitespace as part of a word, e.g. the difference between `iw` and `aw` motions * @return The offset of the [count] next word, or `0` or the offset of the end of file if not found */ fun findNextWord( text: CharSequence, - textLength: Int, editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, - spaceWords: Boolean, ): Int /** diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelperBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelperBase.kt index a365e8778..a601fa344 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelperBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchHelperBase.kt @@ -98,25 +98,26 @@ abstract class VimSearchHelperBase : VimSearchHelper { searchFrom: Int, count: Int, bigWord: Boolean, - spaceWords: Boolean, ): Int { - return findNextWord(editor.text(), editor.fileSize().toInt(), editor, searchFrom, count, bigWord, spaceWords) + return findNextWord(editor.text(), editor, searchFrom, count, bigWord) } - // TODO: Get rid of this overload when rewriting findNextWordOne override fun findNextWord( text: CharSequence, - textLength: Int, editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, - spaceWords: Boolean, ): Int { - val step = if (count >= 0) 1 else -1 var pos = searchFrom repeat(abs(count)) { - pos = findNextWordOne(text, editor, pos, textLength, step, bigWord, spaceWords) + pos = if (count > 0) { + findNextWordOne(text, editor, pos, bigWord) + } else { + findPreviousWordOne(text, editor, pos, bigWord) + } + + if (pos >= text.length) return pos } return pos } @@ -287,7 +288,96 @@ abstract class VimSearchHelperBase : VimSearchHelper { ).map { it.range } } + /** + * Find the next word from the current starting position, skipping current word and whitespace + * + * Note that this will return an out of bound index if there is no next word! This is necessary to distinguish between + * no next word and the next word being on the last character of the file. + * + * Also remember that two different "word" types can butt up against each other - e.g. KEYWORD followed by PUNCTUATION + */ private fun findNextWordOne( + chars: CharSequence, + editor: VimEditor, + start: Int, + bigWord: Boolean, + stopAtEndOfLine: Boolean = false, + ): Int { + var pos = start + if (pos >= chars.length) return chars.length + + val startingCharType = charType(editor, chars[start], bigWord) + + // It is important to move first, to properly handle stopping at the end of a line and moving from an empty line + pos++ + + // If we're on a word, move past the end of it + if (pos < chars.length && startingCharType != CharacterHelper.CharacterType.WHITESPACE) { + pos = skipWhileCharacterType(editor, chars, pos, 1, startingCharType, bigWord) + } + + // Skip following whitespace, optionally stopping at the end of the line (on the newline char). + // An empty line is a word, so stop when the offset is at the newline char of an empty line. + while (pos < chars.length && isWhitespace(editor, chars[pos], bigWord)) { + if (isEmptyLine(chars, pos) || (chars[pos] == '\n' && stopAtEndOfLine)) return pos + pos++ + } + + // We're now on the first character of a word or just past the end of the file + return pos.coerceAtMost(chars.length) + } + + /** + * Find the start of the current word, skipping current whitespace + * + * This function will always return an in-bounds index. If there is no previous word (because we're at the start of + * the file), the offset will be `0`, the start of the current word or preceding whitespace. + */ + private fun findPreviousWordOne( + chars: CharSequence, + editor: VimEditor, + start: Int, + bigWord: Boolean, + ): Int { + var pos = start + + // Always move back one to make sure that we don't get stuck on the start of a word + pos-- + + // Skip any intermediate whitespace, stopping at an empty line (offset is the newline char of the empty line). + // This will leave us on the last character of the previous word. + while (pos >= 0 && isWhitespace(editor, chars[pos], bigWord)) { + if (isEmptyLine(chars, pos)) return pos + pos-- + } + + // We're now on a word character, or at the start of the file. Move back until we're past the start of the word, + // then move forward to the start of the word + if (pos >= 0) { + pos = skipWhileCharacterType(editor, chars, pos, -1, charType(editor, chars[pos], bigWord), bigWord) + 1 + } + + return pos.coerceAtLeast(0) + } + + @Suppress("SameParameterValue") + private fun findCharBeforeNextWord(editor: VimEditor, pos: Int, isBig: Boolean, stopAtEndOfLine: Boolean): Int { + val chars = editor.text() + + // Find the next word, and take the character before it. If there is no next word, we're at the end of the file, and + // offset will be chars.length. Subtracting one gives us an in-bounds index again + var offset = findNextWordOne(chars, editor, pos, isBig, stopAtEndOfLine) - 1 + + // Don't back up to the end of a non-empty line + if (chars[offset] == '\n' && !isEmptyLine(chars, offset)) { + offset-- + } + + return offset + } + + // TODO: Remove this once findWordUnderCursor has been properly rewritten + private fun oldFindNextWordOne( chars: CharSequence, editor: VimEditor, pos: Int, @@ -358,7 +448,7 @@ abstract class VimSearchHelperBase : VimSearchHelper { return res } - @Suppress("GrazieInspection", "StructuralWrap") + @Suppress("GrazieInspection") private fun findNextWordEndOne( chars: CharSequence, editor: VimEditor, @@ -1440,10 +1530,10 @@ abstract class VimSearchHelperBase : VimSearchHelper { // TODO: This could be simplified to move backwards until char type changes if ((!onWordStart && !(onSpace && isOuter)) || hasSelection || (count > 1 && dir == -1)) { start = if (dir == 1) { - findNextWord(editor, pos, -1, isBig, !isOuter) + oldFindNextWordOne(editor.text(), editor, pos, editor.text().length, -1, isBig, spaceWords = !isOuter) } else { val c = -(count - if (onWordStart && !hasSelection) 1 else 0) - findNextWord(editor, pos, c, isBig, !isOuter) + oldFindNextWordOne(editor.text(), editor, pos, editor.text().length, c, isBig, spaceWords = !isOuter) } start = editor.normalizeOffset(start, false) } @@ -1460,6 +1550,7 @@ abstract class VimSearchHelperBase : VimSearchHelper { // TODO: Figure out the logic of this going backwards if (dir == 1) { var count = count + var shouldEndOnWhitespace = false // Selecting word/WORDs (forwards): // If there's no selection, we need to calculate the first range: @@ -1508,16 +1599,18 @@ abstract class VimSearchHelperBase : VimSearchHelper { // We're on a word, move to the end, and include following whitespace by moving to the character before the // next word - isOuter && !onSpace -> // "${s}word ${se}word" - findNextWord(chars, chars.length, editor, start, 1, isBig, spaceWords = true) - 1 + isOuter && !onSpace -> { // "${s}word ${se}word" + shouldEndOnWhitespace = true + findCharBeforeNextWord(editor, start, isBig, stopAtEndOfLine = true) + } // We're on a word, move to the end, not including trailing whitespace !isOuter && !onSpace -> // "${s}word${se} word" findNextWordEnd(editor, start, 1, isBig, stopOnEmptyLine = true, allowMoveFromWordEnd = false) // We're on preceding whitespace, move to the character before the next word - else /* !isOuter && onSpace */ -> // "${s} ${se}word" - findNextWord(chars, chars.length, editor, start, 1, isBig, spaceWords = true) - 1 + else /* !isOuter && onSpace */ -> // "${s} ${se}word" + findCharBeforeNextWord(editor, start, isBig, stopAtEndOfLine = true) } count-- @@ -1585,7 +1678,7 @@ abstract class VimSearchHelperBase : VimSearchHelper { } else { // Move to one before start of next word (skips following whitespace) - findNextWord(chars, chars.length, editor, end, 1, isBig, spaceWords = true) - 1 + findCharBeforeNextWord(editor, end, isBig, stopAtEndOfLine = true) } } else { @@ -1657,6 +1750,23 @@ abstract class VimSearchHelperBase : VimSearchHelper { } } } + + if (isOuter && shouldEndOnWhitespace && start > 0 + && !isWhitespace(editor, chars[end], isBig) + && !isWhitespace(editor, chars[start], isBig)) { + + // Outer word objects normally include following whitespace. But if there's no following whitespace to include, + // we should extend the range to include preceding whitespace. However, Vim doesn't select whitespace at the + // start of a line + var offset = start - 1 + while (offset >= 0 && chars[offset] != '\n' && isWhitespace(editor, chars[offset], isBig)) { + offset-- + } + if (offset > 0 && chars[offset] != '\n') start = offset + 1 + } + + if (start == end && chars[start] == '\n') end++ + return TextRange(start, end + 1) } else if (!onWordEnd || hasSelection || (count > 1 && dir == 1) || (onSpace && isOuter)) { end = if (dir == 1) {