From 1cb420faa3ddb5cb7d8905208646406dfc4b24ab Mon Sep 17 00:00:00 2001 From: samabcde Date: Wed, 10 May 2023 00:21:20 +0800 Subject: [PATCH] VIM-2615 add support to sort u command, fix natural sort issue when both string not contain number --- .../maddyhome/idea/vim/group/ChangeGroup.java | 24 +- .../commands/SortCommandTest.kt | 551 +++++++++++++++--- .../maddyhome/idea/vim/api/VimChangeGroup.kt | 2 +- .../vimscript/model/commands/SortCommand.kt | 14 +- 4 files changed, 486 insertions(+), 105 deletions(-) diff --git a/src/main/java/com/maddyhome/idea/vim/group/ChangeGroup.java b/src/main/java/com/maddyhome/idea/vim/group/ChangeGroup.java index 5d70ff5abd3..97d68db0e10 100644 --- a/src/main/java/com/maddyhome/idea/vim/group/ChangeGroup.java +++ b/src/main/java/com/maddyhome/idea/vim/group/ChangeGroup.java @@ -8,7 +8,6 @@ package com.maddyhome.idea.vim.group; import com.google.common.base.Splitter; -import com.google.common.collect.Lists; import com.intellij.codeInsight.actions.AsyncActionExecutionService; import com.intellij.openapi.Disposable; import com.intellij.openapi.actionSystem.DataContext; @@ -62,6 +61,8 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import static com.maddyhome.idea.vim.api.VimInjectorKt.injector; import static com.maddyhome.idea.vim.api.VimInjectorKt.options; @@ -500,9 +501,11 @@ public void indentRange(@NotNull VimEditor editor, * @param editor The editor to replace text in * @param range The range to sort * @param lineComparator The comparator to use to sort + * @param unique The flag that only keep the first of a sequence of identical lines * @return true if able to sort the text, false if not */ - public boolean sortRange(@NotNull VimEditor editor, @NotNull VimCaret caret, @NotNull LineRange range, @NotNull Comparator lineComparator) { + public boolean sortRange(@NotNull VimEditor editor, @NotNull VimCaret caret, @NotNull LineRange range, @NotNull Comparator lineComparator, + boolean unique) { final int startLine = range.startLine; final int endLine = range.endLine; final int count = endLine - startLine + 1; @@ -513,7 +516,7 @@ public boolean sortRange(@NotNull VimEditor editor, @NotNull VimCaret caret, @No final int startOffset = editor.getLineStartOffset(startLine); final int endOffset = editor.getLineEndOffset(endLine); - return sortTextRange(editor, caret, startOffset, endOffset, lineComparator); + return sortTextRange(editor, caret, startOffset, endOffset, lineComparator, unique); } /** @@ -523,19 +526,28 @@ public boolean sortRange(@NotNull VimEditor editor, @NotNull VimCaret caret, @No * @param start The starting position for the sort * @param end The ending position for the sort * @param lineComparator The comparator to use to sort + * @param unique The flag that only keep the first of a sequence of identical lines * @return true if able to sort the text, false if not */ private boolean sortTextRange(@NotNull VimEditor editor, @NotNull VimCaret caret, int start, int end, - @NotNull Comparator lineComparator) { + @NotNull Comparator lineComparator, + boolean unique) { final String selectedText = ((IjVimEditor) editor).getEditor().getDocument().getText(new TextRangeInterval(start, end)); - final List lines = Lists.newArrayList(Splitter.on("\n").split(selectedText)); + final List lines; + Stream sorted = + StreamSupport.stream(Splitter.on("\n").split(selectedText).spliterator(), false).sorted(lineComparator); + if (unique) { + lines = sorted.distinct().toList(); + } + else { + lines = sorted.toList(); + } if (lines.size() < 1) { return false; } - lines.sort(lineComparator); replaceText(editor, caret, start, end, StringUtil.join(lines, "\n")); return true; } diff --git a/src/test/java/org/jetbrains/plugins/ideavim/ex/implementation/commands/SortCommandTest.kt b/src/test/java/org/jetbrains/plugins/ideavim/ex/implementation/commands/SortCommandTest.kt index 612389427c6..016a4d5ddaf 100644 --- a/src/test/java/org/jetbrains/plugins/ideavim/ex/implementation/commands/SortCommandTest.kt +++ b/src/test/java/org/jetbrains/plugins/ideavim/ex/implementation/commands/SortCommandTest.kt @@ -13,116 +13,481 @@ import org.jetbrains.plugins.ideavim.SkipNeovimReason import org.jetbrains.plugins.ideavim.TestWithoutNeovim import org.jetbrains.plugins.ideavim.VimTestCase import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.MethodSource import javax.swing.KeyStroke @Suppress("SpellCheckingInspection") class SortCommandTest : VimTestCase() { - @Test - fun testBasicSort() { - configureByText( - """ - Test - Hello World! - - """.trimIndent(), - ) - val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) - keys.addAll(injector.parser.stringToKeys("\$j")) - typeText(keys) - typeText(commandToKeys("sort")) - assertState( - """ - Hello World! - Test - - """.trimIndent(), - ) + private fun assertSort(testCase: TestCase) { + val (content, visualSelect, sortCommand, expected) = testCase + configureByText(content) + if (visualSelect.isNotBlank()) { + val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) + keys.addAll(injector.parser.stringToKeys(visualSelect)) + typeText(keys) + } + typeText(commandToKeys(sortCommand)) + assertState(expected) } - @Test - fun testMultipleSortLine() { - configureByText("zee\nyee\na\nb\n") - val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) - keys.addAll(injector.parser.stringToKeys("$3j")) - typeText(keys) - typeText(commandToKeys("sort")) - assertState("a\nb\nyee\nzee\n") - } + data class TestCase(val content: String, val visualSelect: String = "", val sortCommand: String, val expected: String) - @TestWithoutNeovim(reason = SkipNeovimReason.DIFFERENT) - @Test - fun testInverseSort() { - configureByText("kay\nzee\nyee\na\nb\n") - val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) - keys.addAll(injector.parser.stringToKeys("$4j")) - typeText(keys) - typeText(commandToKeys("sort !")) - assertState("zee\nyee\nkay\nb\na\n") - } + companion object { + @JvmStatic + fun defaultSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort", + expected = """ + 10 + 2 + AB + ac + duplicate + duplicate + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort", + expected = """ + 10 + 2 + AB + ac + duplicate + duplicate + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort", + expected = """ + z + 10 + 2 + AB + ac + duplicate + duplicate + a + """.trimIndent() + ) + ) + } - @Test - fun testCaseSensitiveSort() { - configureByText("apple\nAppetite\nApp\napparition\n") - val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) - keys.addAll(injector.parser.stringToKeys("$3j")) - typeText(keys) - typeText(commandToKeys("sort")) - assertState("App\nAppetite\napparition\napple\n") - } + @JvmStatic + fun numericSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort n", + expected = """ + AB + ac + duplicate + duplicate + 2 + 10 + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort n", + expected = """ + AB + ac + duplicate + duplicate + 2 + 10 + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort n", + expected = """ + z + AB + ac + duplicate + duplicate + 2 + 10 + a + """.trimIndent() + ) + ) + } - @Test - fun testCaseInsensitiveSort() { - configureByText("apple\nAppetite\nApp\napparition\n") - val keys: MutableList = Lists.newArrayList(KeyStroke.getKeyStroke("control V")) - keys.addAll(injector.parser.stringToKeys("$3j")) - typeText(keys) - typeText(commandToKeys("sort i")) - assertState("App\napparition\nAppetite\napple\n") - } + @JvmStatic + fun caseInsensitiveSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort i", + expected = """ + 10 + 2 + AB + ac + duplicate + duplicate + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort i", + expected = """ + 10 + 2 + AB + ac + duplicate + duplicate + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort i", + expected = """ + z + 10 + 2 + AB + ac + duplicate + duplicate + a + """.trimIndent() + ) + ) + } - @Test - fun testRangeSort() { - configureByText("zee\nc\na\nb\nwhatever\n") - typeText(commandToKeys("2,4sort")) - assertState("zee\na\nb\nc\nwhatever\n") - } + @JvmStatic + fun reverseSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort!", + expected = """ + duplicate + duplicate + ac + AB + 2 + 10 + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort!", + expected = """ + duplicate + duplicate + ac + AB + 2 + 10 + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort!", + expected = """ + z + duplicate + duplicate + ac + AB + 2 + 10 + a + """.trimIndent() + ) + ) + } - @Test - fun testNumberSort() { - configureByText("120\n70\n30\n2000") - typeText(commandToKeys("sort n")) - assertState("30\n70\n120\n2000") - } + @JvmStatic + fun uniquetSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort u", + expected = """ + 10 + 2 + AB + ac + duplicate + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort u", + expected = """ + 10 + 2 + AB + ac + duplicate + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort u", + expected = """ + z + 10 + 2 + AB + ac + duplicate + a + """.trimIndent() + ) + ) + } - @Test - fun testNaturalOrderSort() { - configureByText("hello1000\nhello102\nhello70000\nhello1001") - typeText(commandToKeys("sort n")) - assertState("hello102\nhello1000\nhello1001\nhello70000") - } - @TestWithoutNeovim(reason = SkipNeovimReason.DIFFERENT) - @Test - fun testNaturalOrderReverseSort() { - configureByText("hello1000\nhello102\nhello70000\nhello1001") - typeText(commandToKeys("sort n!")) - assertState("hello70000\nhello1001\nhello1000\nhello102") + @JvmStatic + fun numericCaseInsensitiveReverseUniqueSortTestCases(): List { + return listOf( + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + """.trimIndent(), + sortCommand = "sort! niu", + expected = """ + 10 + 2 + duplicate + ac + AB + """.trimIndent() + ), + TestCase( + content = """ + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + visualSelect = "$5j", + sortCommand = "sort! niu", + expected = """ + 10 + 2 + duplicate + ac + AB + a + """.trimIndent() + ), + TestCase( + content = """ + z + ac + AB + 10 + 2 + duplicate + duplicate + a + """.trimIndent(), + sortCommand = "2,7sort! niu", + expected = """ + z + 10 + 2 + duplicate + ac + AB + a + """.trimIndent() + ) + ) + } } + + @ParameterizedTest + @MethodSource("defaultSortTestCases") + fun `test default sort is case sensitive, not numeric, ascending and not unique`( + testCase: TestCase, + ) = assertSort(testCase) + + @ParameterizedTest + @MethodSource("numericSortTestCases") + fun `test numeric sort is case sensitive, numeric, ascending and not unique`( + testCase: TestCase, + ) = assertSort(testCase) + + @ParameterizedTest + @MethodSource("caseInsensitiveSortTestCases") + fun `test case insensive sort is case insensitive, not numeric, ascending and not unique`( + testCase: TestCase, + ) = assertSort(testCase) + @TestWithoutNeovim(reason = SkipNeovimReason.DIFFERENT) - @Test - fun testNaturalOrderInsensitiveReverseSort() { - configureByText("Hello1000\nhello102\nhEllo70000\nhello1001") - typeText(commandToKeys("sort ni!")) - assertState("hEllo70000\nhello1001\nHello1000\nhello102") - } + @ParameterizedTest + @MethodSource("reverseSortTestCases") + fun `test reverse sort is case sensitive, not numeric, descending and not unique`( + testCase: TestCase, + ) = assertSort(testCase) - @Test - fun testGlobalSort() { - configureByText("zee\nc\na\nb\nwhatever") - typeText(commandToKeys("sort")) - assertState("a\nb\nc\nwhatever\nzee") - } + @ParameterizedTest + @MethodSource("uniquetSortTestCases") + fun `test unique sort is case sensitive, not numeric, ascending and unique`( + testCase: TestCase, + ) = assertSort(testCase) + + @TestWithoutNeovim(reason = SkipNeovimReason.DIFFERENT) + @ParameterizedTest + @MethodSource("numericCaseInsensitiveReverseUniqueSortTestCases") + fun `test numeric, case insensitive, reverse and unique sort is case insensitive, numeric, descending and unique`( + testCase: TestCase, + ) = assertSort(testCase) @Test fun testSortWithPrecedingWhiteSpace() { diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroup.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroup.kt index 1bf805a6db9..61dbbc4dc1d 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroup.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimChangeGroup.kt @@ -163,7 +163,7 @@ public interface VimChangeGroup { public fun changeNumber(editor: VimEditor, caret: VimCaret, count: Int): Boolean - public fun sortRange(editor: VimEditor, caret: VimCaret, range: LineRange, lineComparator: Comparator): Boolean + public fun sortRange(editor: VimEditor, caret: VimCaret, range: LineRange, lineComparator: Comparator, unique: Boolean): Boolean public fun reset() diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/vimscript/model/commands/SortCommand.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/vimscript/model/commands/SortCommand.kt index 590c75d58d8..18a4c472826 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/vimscript/model/commands/SortCommand.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/vimscript/model/commands/SortCommand.kt @@ -36,12 +36,13 @@ public data class SortCommand(val ranges: Ranges, val argument: String) : Comman val reverse = nonEmptyArg && "!" in arg val ignoreCase = nonEmptyArg && "i" in arg val number = nonEmptyArg && "n" in arg + val unique = nonEmptyArg && "u" in arg val lineComparator = LineComparator(ignoreCase, number, reverse) if (editor.inBlockSubMode) { val primaryCaret = editor.primaryCaret() val range = getSortLineRange(editor, primaryCaret) - val worked = injector.changeGroup.sortRange(editor, primaryCaret, range, lineComparator) + val worked = injector.changeGroup.sortRange(editor, primaryCaret, range, lineComparator, unique) primaryCaret.moveToInlayAwareOffset( injector.motion.moveCaretToLineStartSkipLeading(editor, range.startLine), ) @@ -51,7 +52,7 @@ public data class SortCommand(val ranges: Ranges, val argument: String) : Comman var worked = true for (caret in editor.nativeCarets()) { val range = getSortLineRange(editor, caret) - if (!injector.changeGroup.sortRange(editor, caret, range, lineComparator)) { + if (!injector.changeGroup.sortRange(editor, caret, range, lineComparator, unique)) { worked = false } caret.moveToInlayAwareOffset(injector.motion.moveCaretToLineStartSkipLeading(editor, range.startLine)) @@ -105,13 +106,16 @@ public data class SortCommand(val ranges: Ranges, val argument: String) : Comman o2ToCompare = o2ToCompare.uppercase(Locale.getDefault()) } return if (myNumber) { - // About natural sort order - http://www.codinghorror.com/blog/2007/12/sorting-for-humans-natural-sort-order.html + // About natural sort order - https://blog.codinghorror.com/sorting-for-humans-natural-sort-order/ val n1 = injector.searchGroup.findDecimalNumber(o1ToCompare) val n2 = injector.searchGroup.findDecimalNumber(o2ToCompare) if (n1 == null) { - if (n2 == null) 0 else -1 + if (n2 == null) { + // no number, fallback to default + o1ToCompare.compareTo(o2ToCompare) + } else -1 } else { - if (n2 == null) 1 else n1.compareTo(n2) + if (n2 == null) 1 else n1.compareTo(n2) // what if tied? } } else { o1ToCompare.compareTo(o2ToCompare)