diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerBigWordActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerBigWordActionTest.kt index 579d386e5..cc76d8466 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerBigWordActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerBigWordActionTest.kt @@ -377,24 +377,11 @@ class MotionInnerBigWordActionTest : VimTestCase() { ) } - @VimBehaviorDiffers(originalVimAfter = - """ - |Lorem ipsum dolor sit amet, - | - |${s} - | - | - | - |${c}${se} - | - |consectetur adipiscing elit - """, - description = "I don't understand Vim's logic for selecting mulitple empty lines with 'iw'." + - "E.g. `v3iw` will select 5 lines, `v4iw` will select 7, `v5iw` selects 9. " + - "Fix only when/if Vim's behaviour is clarified" - ) @Test fun `test select multiple empty lines`() { + // I don't understand the logic of this. I would expect v3iw to select 3 lines, but instead it selects 5. + // And v4iw selects 7, v5iw selects 9. But this is how Vim behaves, and it's working and the other tests are working + // too. Let's not question it too hard. doTest( "v3iW", """ @@ -414,8 +401,8 @@ class MotionInnerBigWordActionTest : VimTestCase() { |${s} | | - |${c}${se} | + |${c}${se} | |consectetur adipiscing elit """.trimMargin(), @@ -709,22 +696,12 @@ class MotionInnerBigWordActionTest : VimTestCase() { ) } - @VimBehaviorDiffers(originalVimAfter = - """ - |Lorem ${s}Ipsum - | - |${c}L${se}orem ipsum dolor sit amet, - |consectetur adipiscing elit - |Sed in orci mauris. - |Cras id tellus in ex imperdiet egestas. - """, - description = "Vim's behaviour is weird. Makes no sense that it selects the first character of the word. " + - "Possibly a bug in Vim: https://github.com/vim/vim/issues/16514 " + - "Unclear what the correct behaviour should be", - shouldBeFixed = false - ) @Test fun `test repeated text object expands to empty line`() { + // Well. This behaviour is weird, and looks like a bug, but it matches Vim's behaviour. + // I'm not entirely sure why this happens, but it's a vote of confidence in IdeaVim's implementation that we're + // matching bugs! 😁 + // See https://github.com/vim/vim/issues/16514 doTest( listOf("viW", "iW"), """ @@ -737,8 +714,8 @@ class MotionInnerBigWordActionTest : VimTestCase() { """.trimMargin(), """ |Lorem ${s}Ipsum - |${c}${se} - |Lorem ipsum dolor sit amet, + | + |${c}L${se}orem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. @@ -768,8 +745,8 @@ class MotionInnerBigWordActionTest : VimTestCase() { """.trimMargin(), """ |Lorem ${s}Ipsum - |${c}${se} | + |${c}${se} |Lorem ipsum dolor sit amet, """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerWordActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerWordActionTest.kt index e2f6b999f..0ab1f5cac 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerWordActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionInnerWordActionTest.kt @@ -182,6 +182,16 @@ class MotionInnerWordActionTest : VimTestCase() { ) } + @Test + fun `test select word selects up to punctuation 2`() { + doTest( + "viw", + "Lorem ipsum dolor sit ame${c}t, consectetur adipiscing elit", + "Lorem ipsum dolor sit ${s}ame${c}t${se}, consectetur adipiscing elit", + Mode.VISUAL(SelectionType.CHARACTER_WISE), + ) + } + @Test fun `test select word with existing left-to-right selection selects rest of word`() { doTest( @@ -377,24 +387,11 @@ class MotionInnerWordActionTest : VimTestCase() { ) } - @VimBehaviorDiffers(originalVimAfter = - """ - |Lorem ipsum dolor sit amet, - | - |${s} - | - | - | - |${c}${se} - | - |consectetur adipiscing elit - """, - description = "I don't understand Vim's logic for selecting mulitple empty lines with 'iw'." + - "E.g. `v3iw` will select 5 lines, `v4iw` will select 7, `v5iw` selects 9. " + - "Fix only when/if Vim's behaviour is clarified" - ) @Test fun `test select multiple empty lines`() { + // I don't understand the logic of this. I would expect v3iw to select 3 lines, but instead it selects 5. + // And v4iw selects 7, v5iw selects 9. But this is how Vim behaves, and it's working and the other tests are working + // too. Let's not question it too hard. doTest( "v3iw", """ @@ -414,8 +411,8 @@ class MotionInnerWordActionTest : VimTestCase() { |${s} | | - |${c}${se} | + |${c}${se} | |consectetur adipiscing elit """.trimMargin(), @@ -708,22 +705,11 @@ class MotionInnerWordActionTest : VimTestCase() { ) } - @VimBehaviorDiffers(originalVimAfter = - """ - |Lorem ${s}Ipsum - | - |${c}L${se}orem ipsum dolor sit amet, - |consectetur adipiscing elit - |Sed in orci mauris. - |Cras id tellus in ex imperdiet egestas. - """, - description = "Vim's behaviour is weird. Makes no sense that it selects the first character of the word. " + - "Possibly a bug in Vim: https://github.com/vim/vim/issues/16514 " + - "Unclear what the correct behaviour should be", - shouldBeFixed = false - ) @Test fun `test repeated text object expands to empty line`() { + // Well. This behaviour is weird, and looks like a bug, but it matches Vim's behaviour. + // I'm not entirely sure why this happens, but it's a vote of confidence in IdeaVim's implementation that we're + // matching bugs! 😁 doTest( listOf("viw", "iw"), """ @@ -736,8 +722,8 @@ class MotionInnerWordActionTest : VimTestCase() { """.trimMargin(), """ |Lorem ${s}Ipsum - |${c}${se} - |Lorem ipsum dolor sit amet, + | + |${c}L${se}orem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. @@ -767,8 +753,8 @@ class MotionInnerWordActionTest : VimTestCase() { """.trimMargin(), """ |Lorem ${s}Ipsum - |${c}${se} | + |${c}${se} |Lorem ipsum dolor sit amet, """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), diff --git a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterBigWordActionTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterBigWordActionTest.kt index c7ef7c5e1..c67fd91db 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterBigWordActionTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/action/motion/object/MotionOuterBigWordActionTest.kt @@ -69,10 +69,9 @@ class MotionOuterBigWordActionTest : VimTestCase() { | |consectetur adipiscing elit """, -// description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + -// "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + -// "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" - description = "Not yet implemented" + description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + + "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + + "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" ) @Test fun `test select empty line`() { @@ -92,11 +91,11 @@ class MotionOuterBigWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s} + |${c}${se} | | | - | - |consectetu${c}r${se} adipiscing elit + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -144,14 +143,37 @@ class MotionOuterBigWordActionTest : VimTestCase() { ) } + @Test + fun `test select outer word skips following empty line`() { + doTest( + listOf("vaW", "aW"), + """ + |Lorem ipsum dolor sit amet, + | + |con${c}sectetur + | + |adipiscing elit + """.trimMargin(), + """ + |Lorem ipsum dolor sit amet, + | + |${s}consectetur + | + |adipiscin${c}g${se} elit + """.trimMargin(), + Mode.VISUAL(SelectionType.CHARACTER_WISE), + ) + } + @VimBehaviorDiffers(originalVimAfter = """ |Lorem ipsum dolor sit amet, | |${s} - | - |${c}${se}consectetur adipiscing elit + |${c} + |${se}consectetur adipiscing elit """, + description = "Off by one because IdeaVim does not allow selecting a newline char" ) @Test fun `test select empty line wraps to next line but does not wrap to following line`() { @@ -168,8 +190,8 @@ class MotionOuterBigWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s} - | - |consectetu${c}r${se} adipiscing elit + |${c}${se} + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -187,7 +209,8 @@ class MotionOuterBigWordActionTest : VimTestCase() { |${c} |${se} |consectetur adipiscing elit - """ + """, + description = "Off by one because IdeaVim does not allow selecting a newline char" ) @Test fun `test select multiple empty lines`() { @@ -213,9 +236,9 @@ class MotionOuterBigWordActionTest : VimTestCase() { | | | + |${c}${se} | - | - |consectetur adipiscing eli${c}t${se} + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -231,10 +254,9 @@ class MotionOuterBigWordActionTest : VimTestCase() { | |consectetur adipiscing elit """, -// description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + -// "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + -// "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" - description = "Not yet implemented" + description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + + "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + + "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" ) @Test fun `test select blank line`() { @@ -253,10 +275,10 @@ class MotionOuterBigWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s}.... + |${c}${se} | | - | - |consectetu${c}r${se} adipiscing elit + |consectetur adipiscing elit """.trimMargin().dotToSpace(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -472,13 +494,14 @@ class MotionOuterBigWordActionTest : VimTestCase() { """ |Lorem${s} Ipsum | - |${c}${se} - | + |${c} + |${se} |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. - """ + """, + description = "Off by one because IdeaVim does not currently support selecting newline char" ) @Test fun `test repeated text object expands to multiple empty lines`() { @@ -497,9 +520,9 @@ class MotionOuterBigWordActionTest : VimTestCase() { """ |Lorem${s} Ipsum | + |${c}${se} | - | - |Lore${c}m${se} ipsum dolor sit amet, + |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. @@ -532,7 +555,6 @@ class MotionOuterBigWordActionTest : VimTestCase() { ) } - // TODO: Fix this bug @VimBehaviorDiffers(originalVimAfter = """ |Lorem${s} Ipsum @@ -543,7 +565,8 @@ class MotionOuterBigWordActionTest : VimTestCase() { |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. - """ + """, + description = "Off by one because IdeaVim does not currently support selecting newline char" ) @Test fun `test repeated text object expands to cover whitespace on following blank lines`() { @@ -563,8 +586,8 @@ class MotionOuterBigWordActionTest : VimTestCase() { |Lorem${s} Ipsum | |........ - | - |Lore${c}m${se} ipsum dolor sit amet, + |${c}${se} + |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. 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 c7db17dbf..1bfceb7e7 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 @@ -67,10 +67,9 @@ class MotionOuterWordActionTest : VimTestCase() { | |consectetur adipiscing elit """, -// description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + -// "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + -// "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" - description = "Not yet implemented" + description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + + "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + + "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" ) @Test fun `test select empty line`() { @@ -90,11 +89,11 @@ class MotionOuterWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s} + |${c}${se} | | | - | - |consectetu${c}r${se} adipiscing elit + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -147,9 +146,10 @@ class MotionOuterWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s} - | - |${c}${se}consectetur adipiscing elit + |${c} + |${se}consectetur adipiscing elit """, + description = "Off by one because IdeaVim does not allow selecting a newline char" ) @Test fun `test select empty line wraps to next line but does not wrap to following line`() { @@ -166,8 +166,8 @@ class MotionOuterWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s} - | - |consectetu${c}r${se} adipiscing elit + |${c}${se} + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -185,7 +185,8 @@ class MotionOuterWordActionTest : VimTestCase() { |${c} |${se} |consectetur adipiscing elit - """ + """, + description = "Off by one because IdeaVim does not allow selecting a newline char" ) @Test fun `test select multiple empty lines`() { @@ -211,9 +212,9 @@ class MotionOuterWordActionTest : VimTestCase() { | | | + |${c}${se} | - | - |consectetur adipiscing eli${c}t${se} + |consectetur adipiscing elit """.trimMargin(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -229,10 +230,9 @@ class MotionOuterWordActionTest : VimTestCase() { | |consectetur adipiscing elit """, -// description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + -// "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + -// "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" - description = "Not yet implemented" + description = "The caret at the same offset as the selection end is an indication that there is an off-by-one error." + + "IdeaVim doesn't (currently) allow selecting the end of line char, so this inclusive range does not include the" + + "(exclusive) end of line char. Once IdeaVim handles this, we might have to fix things" ) @Test fun `test select blank line`() { @@ -251,10 +251,10 @@ class MotionOuterWordActionTest : VimTestCase() { |Lorem ipsum dolor sit amet, | |${s}.... + |${c}${se} | | - | - |consectetu${c}r${se} adipiscing elit + |consectetur adipiscing elit """.trimMargin().dotToSpace(), Mode.VISUAL(SelectionType.CHARACTER_WISE), ) @@ -494,13 +494,14 @@ class MotionOuterWordActionTest : VimTestCase() { """ |Lorem${s} Ipsum | - |${c}${se} - | + |${c} + |${se} |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. - """ + """, + description = "Off by one because IdeaVim does not currently support selecting newline char" ) @Test fun `test repeated text object expands to multiple empty lines`() { @@ -519,9 +520,9 @@ class MotionOuterWordActionTest : VimTestCase() { """ |Lorem${s} Ipsum | + |${c}${se} | - | - |Lore${c}m${se} ipsum dolor sit amet, + |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. @@ -554,7 +555,6 @@ class MotionOuterWordActionTest : VimTestCase() { ) } - // TODO: Fix this bug @VimBehaviorDiffers(originalVimAfter = """ |Lorem${s} Ipsum @@ -565,7 +565,8 @@ class MotionOuterWordActionTest : VimTestCase() { |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. - """ + """, + description = "Off by one because IdeaVim does not currently support selecting newline char" ) @Test fun `test repeated text object expands to cover whitespace on following blank lines`() { @@ -585,8 +586,8 @@ class MotionOuterWordActionTest : VimTestCase() { |Lorem${s} Ipsum | |........ - | - |Lore${c}m${se} ipsum dolor sit amet, + |${c}${se} + |Lorem ipsum dolor sit amet, |consectetur adipiscing elit |Sed in orci mauris. |Cras id tellus in ex imperdiet egestas. @@ -787,6 +788,7 @@ 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' " + diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/motion/text/MotionWordEndAction.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/motion/text/MotionWordEndAction.kt index 23c98489e..8aa6c40c6 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/motion/text/MotionWordEndAction.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/action/motion/text/MotionWordEndAction.kt @@ -21,19 +21,25 @@ import com.maddyhome.idea.vim.handler.Motion import com.maddyhome.idea.vim.handler.Motion.AbsoluteOffset import com.maddyhome.idea.vim.handler.MotionActionHandler +// Vim considers an empty line as a word/WORD, but for vi compatibility, `e` and `E` do not stop at empty lines (`ge`, +// `gE` and `w`/`W` and `b`/`B` do) @CommandOrMotion(keys = ["gE"], modes = [Mode.NORMAL, Mode.VISUAL, Mode.OP_PENDING]) -class MotionBigWordEndLeftAction : WordEndAction(Direction.BACKWARDS, true) +class MotionBigWordEndLeftAction : WordEndAction(Direction.BACKWARDS, bigWord = true, stopAtEmptyLine = true) @CommandOrMotion(keys = ["E"], modes = [Mode.NORMAL, Mode.VISUAL, Mode.OP_PENDING]) -class MotionBigWordEndRightAction : WordEndAction(Direction.FORWARDS, true) +class MotionBigWordEndRightAction : WordEndAction(Direction.FORWARDS, bigWord = true, stopAtEmptyLine = false) @CommandOrMotion(keys = ["ge"], modes = [Mode.NORMAL, Mode.VISUAL, Mode.OP_PENDING]) -class MotionWordEndLeftAction : WordEndAction(Direction.BACKWARDS, false) +class MotionWordEndLeftAction : WordEndAction(Direction.BACKWARDS, bigWord = false, stopAtEmptyLine = false) @CommandOrMotion(keys = ["e"], modes = [Mode.NORMAL, Mode.VISUAL, Mode.OP_PENDING]) -class MotionWordEndRightAction : WordEndAction(Direction.FORWARDS, false) +class MotionWordEndRightAction : WordEndAction(Direction.FORWARDS, bigWord = false, stopAtEmptyLine = false) -sealed class WordEndAction(val direction: Direction, val bigWord: Boolean) : MotionActionHandler.ForEachCaret() { +sealed class WordEndAction( + private val direction: Direction, + private val bigWord: Boolean, + private val stopAtEmptyLine: Boolean, +) : MotionActionHandler.ForEachCaret() { override fun getOffset( editor: VimEditor, caret: ImmutableVimCaret, @@ -41,13 +47,19 @@ sealed class WordEndAction(val direction: Direction, val bigWord: Boolean) : Mot argument: Argument?, operatorArguments: OperatorArguments, ): Motion { - return moveCaretToNextWordEnd(editor, caret, direction.toInt() * operatorArguments.count1, bigWord) + return moveCaretToNextWordEnd(editor, caret, direction.toInt() * operatorArguments.count1, bigWord, stopAtEmptyLine) } override val motionType: MotionType = MotionType.INCLUSIVE } -private fun moveCaretToNextWordEnd(editor: VimEditor, caret: ImmutableVimCaret, count: Int, bigWord: Boolean): Motion { +private fun moveCaretToNextWordEnd( + editor: VimEditor, + caret: ImmutableVimCaret, + count: Int, + bigWord: Boolean, + stopAtEmptyLine: Boolean, +): Motion { if (caret.offset == 0 && count < 0 || caret.offset >= editor.fileSize() - 1 && count > 0) { return Motion.Error } @@ -55,7 +67,7 @@ private fun moveCaretToNextWordEnd(editor: VimEditor, caret: ImmutableVimCaret, // If we are doing this move as part of a change command (e.q. cw), we need to count the current end of // word if the cursor happens to be on the end of a word already. If this is a normal move, we don't count // the current word. - val pos = injector.searchHelper.findNextWordEnd(editor, caret.offset, count, bigWord, false) + val pos = injector.searchHelper.findNextWordEnd(editor, caret.offset, count, bigWord, stopAtEmptyLine) return if (pos == -1) { if (count < 0) { AbsoluteOffset(injector.motion.moveCaretToLineStart(editor, 0)) diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchGroupBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchGroupBase.kt index 777c51607..29aa9e1d5 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchGroupBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimSearchGroupBase.kt @@ -622,7 +622,7 @@ abstract class VimSearchGroupBase : VimSearchGroup { ) { start + 1 } else { - injector.searchHelper.findNextWordEnd(editor, start, 1, bigWord = false, spaceWords = false) + 1 + injector.searchHelper.findNextWordEnd(editor, start, 1, bigWord = false) + 1 } return TextRange(start, end) } 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 9e87ccf33..307f6c4fa 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 @@ -139,31 +139,20 @@ interface VimSearchHelper { * @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 findNextWordEnd(editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, spaceWords: Boolean): Int - - /** - * Find the end offset in some text outside the editor (e.g., command line), from the given starting point - * - * @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 + * @param stopOnEmptyLine Vim considers an empty line to be a word/WORD, but `e` and `E` don't respect this for vi + * compatibility reasons. Callers other than `e` and `E` should pass `true` + * @param allowMoveFromWordEnd If we're already at the word end, should we be able to move to the next word end? This + * is true for word/WORD motions `e`/`E`, but false for word text objects, which do not + * extend the selection/range forwards when at the end of a current word. + * @return The offset of the [count] next word/WORD. Will return document bounds if not found */ fun findNextWordEnd( - text: CharSequence, - textLength: Int, editor: VimEditor, searchFrom: Int, count: Int, bigWord: Boolean, - spaceWords: Boolean, + stopOnEmptyLine: Boolean = true, + allowMoveFromWordEnd: Boolean = true, ): 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 937c6682b..a89957e52 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 @@ -13,6 +13,7 @@ import com.maddyhome.idea.vim.common.TextRange import com.maddyhome.idea.vim.diagnostic.vimLogger import com.maddyhome.idea.vim.helper.CharacterHelper import com.maddyhome.idea.vim.helper.CharacterHelper.charType +import com.maddyhome.idea.vim.helper.CharacterHelper.isWhitespace import com.maddyhome.idea.vim.helper.Msg import com.maddyhome.idea.vim.helper.SearchOptions import com.maddyhome.idea.vim.helper.enumSetOf @@ -102,6 +103,7 @@ abstract class VimSearchHelperBase : VimSearchHelper { return findNextWord(editor.text(), editor.fileSize().toInt(), editor, searchFrom, count, bigWord, spaceWords) } + // TODO: Get rid of this overload when rewriting findNextWordOne override fun findNextWord( text: CharSequence, textLength: Int, @@ -111,7 +113,12 @@ abstract class VimSearchHelperBase : VimSearchHelper { bigWord: Boolean, spaceWords: Boolean, ): Int { - return doFindNext(text, textLength, editor, searchFrom, count, bigWord, spaceWords, ::findNextWordOne) + val step = if (count >= 0) 1 else -1 + var pos = searchFrom + repeat(abs(count)) { + pos = findNextWordOne(text, editor, pos, textLength, step, bigWord, spaceWords) + } + return pos } override fun findNextWordEnd( @@ -119,21 +126,19 @@ abstract class VimSearchHelperBase : VimSearchHelper { searchFrom: Int, count: Int, bigWord: Boolean, - spaceWords: Boolean, + stopOnEmptyLine: Boolean, + allowMoveFromWordEnd: Boolean ): Int { - return findNextWordEnd(editor.text(), editor.fileSize().toInt(), editor, searchFrom, count, bigWord, spaceWords) - } - - override fun findNextWordEnd( - text: CharSequence, - textLength: Int, - editor: VimEditor, - searchFrom: Int, - count: Int, - bigWord: Boolean, - spaceWords: Boolean, - ): Int { - return doFindNext(text, textLength, editor, searchFrom, count, bigWord, spaceWords, ::findNextWordEndOne) + val text = editor.text() + var pos = searchFrom + repeat(abs(count)) { + pos = if (count > 0) { + findNextWordEndOne(text, editor, pos, bigWord, stopOnEmptyLine, allowMoveFromWordEnd) + } else { + findPreviousWordEndOne(text, editor, pos, bigWord) + } + } + return pos } override fun findPattern( @@ -282,29 +287,6 @@ abstract class VimSearchHelperBase : VimSearchHelper { ).map { it.range } } - private fun doFindNext( - text: CharSequence, - textLength: Int, - editor: VimEditor, - searchFrom: Int, - countDirection: Int, - bigWord: Boolean, - spaceWords: Boolean, - action: (text: CharSequence, editor: VimEditor, pos: Int, size: Int, step: Int, bigWord: Boolean, spaceWords: Boolean) -> Int, - ): Int { - var count = countDirection - val step = if (count >= 0) 1 else -1 - count = abs(count) - var pos = searchFrom - for (i in 0 until count) { - pos = action(text, editor, pos, textLength, step, bigWord, spaceWords) - if (pos == searchFrom || pos == 0 || pos == textLength - 1) { - break - } - } - return pos - } - private fun findNextWordOne( chars: CharSequence, editor: VimEditor, @@ -376,81 +358,91 @@ abstract class VimSearchHelperBase : VimSearchHelper { return res } + @Suppress("GrazieInspection", "StructuralWrap") private fun findNextWordEndOne( chars: CharSequence, editor: VimEditor, start: Int, - size: Int, - step: Int, bigWord: Boolean, - spaceWords: Boolean, + stopOnEmptyLine: Boolean, + allowMoveFromWordEnd: Boolean, ): Int { + // Scenarios: + // * Cursor on word/WORD: move to end of the current word/WORD group ("wo${c}rd word" -> "wor${c}d word") + // * Cursor on end of word/WORD: move to end of next word/WORD group ("wor${c}d word" -> "word wor${c}d") + // * Cursor on whitespace: move to end of next word/WORD group (" ${c} word" -> " wor${c}d") + // * Visual on single char: + // For `e`/`E`: move to end of next word/WORD group (" ${s}d${se} word" -> " ${s}d word${se}") + // For `iw` and no selection: DO NOT MOVE! (" ${s}d${se} word" -> " ${s}d${se} word") + // + // Note that newline characters are considered to be whitespace. This is USUALLY treated like whitespace, so + // moving through a new line to get to next word end is usually just the same as moving through other whitespace. + // Empty lines are an exception to this rule. Vim considers an empty line to be a word/WORD, but for vi + // compatibility reasons, `e` and `E` do not stop on empty lines, but text objects do. + var pos = start - var found = false - // For forward searches, skip any current whitespace so we start at the start of a word - if (step > 0 && pos < size - 1) { - if (chars[pos + 1] == '\n' - && spaceWords - && charType(editor, chars[pos], bigWord) !== charType(editor, chars[pos + 1], bigWord)) { + val startingCharType = charType(editor, chars[pos], bigWord) + + // If we process the loops below, we end up one character past the target offset. But we don't always process the + // loops. Move one character forward now so everything works cleanly + pos++ + + // When called by `e`/`E` and we're originally at the end of the current word, we want to move to end of the *next* + // word, so skip whitespace at the current pos and get to the start of the next word. + // When called by `iw` and we're at the end of the current word, we don't want to move, so if there's whitespace at + // the current position, don't skip it. We then won't be at the start of the next word, so we won't move further. + // We'll decrement before we return, so the cursor doesn't move. + // But if we start on whitespace, we always want to skip it, for both scenarios. + if (allowMoveFromWordEnd || startingCharType == CharacterHelper.CharacterType.WHITESPACE) { + while (pos < chars.length && isWhitespace(editor, chars[pos], bigWord)) { + if (stopOnEmptyLine && isEmptyLine(chars, pos)) return pos pos++ } - if (charType(editor, chars[pos + 1], bigWord) === CharacterHelper.CharacterType.WHITESPACE && !spaceWords) { - pos = skipSpace(editor, chars, pos + 1, step, size, false) - 1 - } - if (pos < size - 1 && charType(editor, chars[pos], bigWord) !== charType(editor, chars[pos + 1], bigWord)) { - pos += step - } } - var res = pos - if (pos < 0 || pos >= size) { - return pos + + // If we're on whitespace now, it's because we started on the end of a word and then immediately advanced to + // whitespace. We don't want to skip the current whitespace (as above) and we don't want to + if (pos < chars.length && !isWhitespace(editor, chars[pos], bigWord)) { + // We're currently at the start of, or inside, a word/WORD. Move to the start of the next charType segment, and + // return the character *before* it, so we get the end of the current word/WORD + val wordCharType = charType(editor, chars[pos], bigWord) + pos = skipWhileCharacterType(editor, chars, pos, 1, wordCharType, bigWord) } - var type = charType(editor, chars[pos], bigWord) - if (type === CharacterHelper.CharacterType.WHITESPACE && step >= 0 && pos < size - 1 && !spaceWords) { - type = charType(editor, chars[pos + 1], bigWord) + + // The loops above have put us one char past the next word, or one past whitespace. Or just advanced one character + pos-- + + return pos.coerceIn(0, chars.length - 1) + } + + private fun findPreviousWordEndOne( + chars: CharSequence, + editor: VimEditor, + start: Int, + bigWord: Boolean, + ): Int { + var pos = start + val startingCharType = charType(editor, chars[pos], bigWord) + + // We always have to move backwards + pos-- + + // Find the first preceding character with a different charType. Note that we use the starting charType here, + // because we might have already moved across a word/WORD/non-word boundary. + // We don't need to +/-1 here. This is returning the offset of the character at the end of the previous word/WORD + if (pos >= 0 && startingCharType != CharacterHelper.CharacterType.WHITESPACE) { + pos = skipWhileCharacterType(editor, chars, pos, -1, startingCharType, bigWord) } - var lastChar = 0.toChar() - pos += step - while (pos >= 0 && pos < size && !found) { - var newChar = chars[pos] - val newType = charType(editor, newChar, bigWord) - if (newType !== type) { - if (step >= 0) { - res = pos - 1 - } else if (newType === CharacterHelper.CharacterType.WHITESPACE && step < 0 && !spaceWords) { - // `e` does not match empty lines, but `ge` does. - // This change in behaviour might be surprising to callers! - pos = skipSpace(editor, chars, pos, step, size, matchEmptyLine = step < 0) - res = if (pos >= 0 && chars[pos] == '\n') pos + 1 else pos - } else { - res = pos - } - found = true - } else if (newChar == '\n' - && (newChar == lastChar - || (spaceWords && charType(editor, lastChar, bigWord) === CharacterHelper.CharacterType.WHITESPACE)) - ) { - // Add/subtract one so we don't take the current new line char as our end of word char. - // However, if we're matching empty lines and we start on an empty line, then we'll get two consecutive new line - // chars. Subtracting will put us back at the start character - res = if (step < 0) pos + 1 else (pos - 1).coerceAtLeast(start + 1) - found = true - } - lastChar = newChar - pos += step + + // If we ended up on whitespace, skip backwards until we find either the last character of the previous word/WORD, + // or we have to stop at an empty line, which is considered a WORD + while (pos in 0 until chars.length && isWhitespace(editor, chars[pos], bigWord)) { + // Always check empty line when moving backwards + if (isEmptyLine(chars, pos)) return pos + pos-- } - if (found) { - if (res < 0) { - res = 0 - } else if (res >= size) { - res = size - 1 - } - } else if (pos == size) { - res = size - 1 - } else if (pos <= 0) { - res = 0 - } - return res + + return pos.coerceAtLeast(0) } private fun skipSpace( @@ -473,6 +465,34 @@ abstract class VimSearchHelperBase : VimSearchHelper { return if (_offset < size) _offset else size - 1 } + private fun isEmptyLine(chars: CharSequence, offset: Int): Boolean { + // The new line char belongs to the current line, so if the previous character is also a new line char, then the + // line for the current offset is empty + return (offset == 0 && chars[offset] == '\n') + || (chars[offset] == '\n' && chars[offset - 1] == '\n') + } + + /** + * Skips characters in a given direction until reaching a different character type + * + * Returns the offset of the character with the different character type. Will return indexes out of bounds at the + * start and end of the file. Specifically, will return `-1` at the start of the file, `chars.length` at the end. + */ + private fun skipWhileCharacterType( + editor: VimEditor, + chars: CharSequence, + start: Int, + step: Int, + type: CharacterHelper.CharacterType, + isBig: Boolean, + ): Int { + var offset = start + while (offset in 0 until chars.length && charType(editor, chars[offset], isBig) === type) { + offset += step + } + return offset + } + override fun findNextCamelStart(chars: CharSequence, startIndex: Int, count: Int): Int? { return findCamelStart(chars, startIndex, count, Direction.FORWARDS) } @@ -1417,6 +1437,7 @@ abstract class VimSearchHelperBase : VimSearchHelper { logger.debug("pos=$pos") logger.debug("onWordStart=$onWordStart") + // 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) @@ -1435,12 +1456,199 @@ abstract class VimSearchHelperBase : VimSearchHelper { logger.debug("onWordEnd=$onWordEnd") var end = pos - if (!onWordEnd || hasSelection || (count > 1 && dir == 1) || (onSpace && isOuter)) { + + // TODO: Figure out the logic of this going backwards + if (dir == 1) { + var count = count + + // Selecting word/WORDs (forwards): + // If there's no selection, we need to calculate the first range: + // -> Move back to the first character _on the current line_ of the current character type + // If we're on whitespace, this is the start of the preceding whitespace + // If we're on a word/WORD char, it's the start of the word/WORD + // -> Move forward to the end of the next word/WORD or whitespace block + // (Remember that `${se}` in the following examples is at `end+1`) + // For outer objects and currently on whitespace, move to end of the next word/WORD ("${s} word${se}") + // New lines are treated as whitespace, so this will wrap and move to the end of the next word/WORD. + // Empty lines will be treated as a word. + // For outer objects on a word/WORD char, move to one character _before_ the next word/WORD ("${s}word ${se}word") + // New lines should be treated as a stop character, and we stop before the new line. + // For inner objects on a word/WORD char, move to the end of this word/WORD ("${s}word${se} word") + // This will never encounter a new line character. + // For inner objects on whitespace, move to one character _before_ the next word/WORD ("${s} ${se}word") + // New lines should be treated as a stop character, and we stop before the new line. + // -> Subtract 1 from count + // Once we have a range, or if there's an initial selection: + // -> Loop over count + // -> For inner objects, move to the end of the next character type block. Whitespace counts in the loop + // -> For outer objects, move to the character _before_ the next word/WORD. Therefore, whitespace does not count + // For all of these operations, remember that an empty line is a word. + + if (!hasSelection) { + // Move back to the first character of the current character type on the current line. + // This will be the start of the word/WORD or the start of whitespace. + val startingCharacterType = charType(editor, chars[pos], isBig) + start = pos + if (!isEmptyLine(chars, start)) { + while (start >= 0 && chars[start] != '\n' && charType(editor, chars[start], isBig) == startingCharacterType) { + start-- + } + start++ + } + + // Move forward, including or skipping whitespace as necessary. Move from the start of the current + // word/whitespace rather than the original position, so that it's easier to handle moving to the end of a word + // when the original position is already at the end of the word. + // Note that `onSpace` is the character type of the original position, but this is also the character type of + // the current start position + end = when { + // We're on preceding whitespace. Include it, and move to the end of the next word/WORD + isOuter && onSpace -> // "${s} word${se}" + findNextWordEnd(editor, start, 1, isBig, stopOnEmptyLine = true, allowMoveFromWordEnd = false) + + // 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 + + // 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 + } + + count-- + } else { + end = pos + } + + // We cannot rely on the current location of the cursor/"end". If there was no initial selection, then it will be + // at the end of a character type block, either word/WORD or whitespace. But if there was an initial selection, + // it could be anywhere. + repeat (count) { + if (isOuter) { + // Outer object. Include whitespace. + // + // Selection ends on whitespace: Move to end of next word (skips whitespace, including newline) + // "${s} ${se} word " -> "${s} word${se} " + // "${s} ${se} \n word " -> "${s} \n word${se} " + // Selection ends on end of whitespace: Move to one character before next word (or end of file) + // "${s}word ${se}word " -> "${s}word word ${se}" + // Selection ends on word: Move to one character before next word + // "${s}wo${se}rd word" -> "${s}word ${se}word" + // "${s}wo${se}rd, word" -> "${s}word${se}, word" + // Selection ends on end of word: Move to end of next word (skips whitespace) + // "${s}word${se} word " -> "${s}word word${se} " + // Selection ends on end of word with following word: Move to one character before next word + // "${s}word${se}, word " -> "${s}word, ${se}word " + // Selection ends on word char at end of line: + // If next non-newline char is word, move past newline, move to one char before next word + // Else, move to end of next word + // "${s}word${se}\nword " -> "${s}word\nword ${se}" + // "${s} word${se}\nword " -> "${s} word\nword ${se}" + // "${s}word${se}\n word " -> "${s}word\n word${se} " + // Selection ends on whitespace at end of line: + // If next non-newline char is word, move past newline, move to one char before next word + // Else, move to end of next word + // "${s} ${se}\nword " -> "${s} \nword ${se}" + // "${s} ${se}\n word " -> "${s} \n word${se} " + // + // This can be generalised to move forward one char, skip again if it's a newline, then either move to the end + // of the next word (which skips preceding whitespace), or to one character before the next word (which + // includes following whitespace). Moving forward one char means we don't have to distinguish between inside a + // word/whitespace, or at the end of a word whitespace. If we started inside, we want to move based on the + // current/starting character type, if we started at the end, we want to move based on the next character + // type. By always using next, we use the correct character type. + + // Move forward one char + // Skip again if new char is newline + // If on whitespace, move to end of next word (skips current/preceding whitespace) + // If on word, move to one before start of next word (skips following whitespace) + + // Increment, and skip the newline char, unless we've just landed on an empty line + end++ + if (end < chars.length && chars[end] == '\n' && !isEmptyLine(chars, end)) { + end++ + } + + if (end >= chars.length) { + end-- + return@repeat + } + + end = if (isWhitespace(editor, chars[end], isBig)) { + // Move to end of next word (skips current/preceding whitespace) + findNextWordEnd(editor, end, 1, isBig, stopOnEmptyLine = true) + } + else { + // Move to one before start of next word (skips following whitespace) + findNextWord(chars, chars.length, editor, end, 1, isBig, spaceWords = true) - 1 + } + + } else { + // Inner object. Whitespace is not included in a move, but included as a separate (counted) move + // + // Selection ends on whitespace: Move to end of current character type or end of line. + // "${s} ${se} word" -> "${s} ${se}word" + // "${s} ${se} \n word" -> "${s} ${se}\n word" + // Selection ends on end of whitespace: Move to end of next character type. + // "${s} ${se}word" -> "${s} word${se}" + // "${s} ${se}\nword " -> "${s} \nword${se} " + // "${s} ${se}\n word" -> "${s} \n ${se}word" + // Selection ends on word: Move to end of current character type. + // "${s}wo${se}rd word" -> "${s}word${se} word" + // "${s}wo${se}rd, word" -> "${s}word${se}, word" + // Selection ends on end of word: Move to end of next character type or end of line. + // "${s}word${se} word " -> "${s}word ${se}word " + // "${s}word${se}, word " -> "${s}word,${se} word " + // "${s}word${se} \n word " -> "${s}word ${se}\n word " + // Selection ends on word char at end of line: Move to end of next character type SKIPPING NEWLINE! + // "${s}word${se}\nword " -> "${s}word\nword${se} " + // "${s} word${se}\nword " -> "${s} word\nword${se} " + // "${s}word${se}\n word " -> "${s}word\n ${se}word " + // Selection ends on whitespace at end of line: Move to end of next character type SKIPPING NEWLINE! + // "${s}word ${se}\n word" -> "${s}word \n ${se}word" + // "${s}word ${se}\nword " -> "${s}word \nword${se} " + // + // This can be generalised to move forward on character, skip again if it's a newline, then move to end of + // the now current character type. + + // Increment, and skip the newline char, unless we've just landed on an empty line + end++ + if (end < chars.length && chars[end] == '\n' && !isEmptyLine(chars, end)) { + end++ + } + + if (end >= chars.length) { + end-- + return@repeat + } + + val characterType = charType(editor, chars[end], isBig) + while (end < chars.length && charType(editor, chars[end], isBig) == characterType) { + end++ + + // Break at end of line. We only wrap when we start at the end of the line, and that's handled above + if (end < chars.length && chars[end] == '\n') break + } + + // We've gone past the current character type, or hit a newline. Go back, unless that would put us onto a + // newline char + if (chars[end - 1] != '\n' || end >= chars.length) { + end-- + } + } + } + } + else if (!onWordEnd || hasSelection || (count > 1 && dir == 1) || (onSpace && isOuter)) { end = if (dir == 1) { val c = count - if (onWordEnd && !hasSelection && (!(onSpace && isOuter) || (onSpace && !isOuter))) 1 else 0 - findNextWordEnd(editor, pos, c, isBig, !isOuter) + findNextWordEnd(editor, pos, c, isBig, !isOuter, allowMoveFromWordEnd = false) } else { - findNextWordEnd(editor, pos, 1, isBig, !isOuter) + findNextWordEnd(editor, pos, 1, isBig, !isOuter, allowMoveFromWordEnd = false) } } diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/helper/CharacterHelper.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/helper/CharacterHelper.kt index 9edd0b2a1..15e9011ed 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/helper/CharacterHelper.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/helper/CharacterHelper.kt @@ -46,6 +46,10 @@ object CharacterHelper { } } + @JvmStatic + fun isWhitespace(editor: VimEditor, ch: Char, isBig: Boolean): Boolean = + charType(editor, ch, isBig) == CharacterType.WHITESPACE + @JvmStatic fun isInvisibleControlCharacter(ch: Char): Boolean { val type = Character.getType(ch).toByte()