From 723f68f48ce9d8b37a057a1f603b45cea3679db7 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 09:34:46 -0600 Subject: [PATCH 1/8] First cut at completion sorting improvements. --- .../completion/CompletionItemComparator.java | 65 +++++++++++++++---- .../completion/LSPCompletionContributor.java | 26 ++++++-- .../CompletionItemComparatorTest.java | 5 +- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index 1763de2ee..85fd8b283 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -10,15 +10,25 @@ ******************************************************************************/ package com.redhat.devtools.lsp4ij.features.completion; -import java.util.Comparator; - +import com.intellij.openapi.util.text.StringUtil; import org.eclipse.lsp4j.CompletionItem; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Comparator; + /** * Compares {@link CompletionItem}s by their sortText property (falls back to comparing labels) */ public class CompletionItemComparator implements Comparator { + private final String currentWord; + private final boolean caseSensitive; + + public CompletionItemComparator(@Nullable String currentWord, boolean caseSensitive) { + this.currentWord = currentWord; + this.caseSensitive = caseSensitive; + } + @Override public int compare(CompletionItem item1, CompletionItem item2) { if (item1 == item2) { @@ -29,24 +39,51 @@ public int compare(CompletionItem item1, CompletionItem item2) { return 1; } - int comparison = compareNullable(item1.getSortText(), item2.getSortText()); + // If one is a better match for the current word than the other, prioritize it higher + // TODO: Take into account case-sensitivity with the following priorities: + // 1. Case-sensitive exact match + // 2. Case-insensitive exact match if a case-insensitive language + // 3. Prefix case-insensitive match if the all upper-case prefix + // 4. Prefix case-sensitive camel-hump match + // 5. Prefix case-insensitive camel-hump match if a case-insensitive language + int comparison = compareAgainstCurrentWord(item1, item2); + if (comparison != 0) { + return comparison; + } + + comparison = compare(item1.getSortText(), item2.getSortText()); + if (comparison != 0) { + return comparison; + } // If sortText is equal, fall back to comparing labels - if (comparison == 0) { - comparison = item1.getLabel().compareTo(item2.getLabel()); + return compare(item1.getLabel(), item2.getLabel()); + } + + private boolean startsWith(@Nullable String prefix) { + if ((currentWord == null) || (prefix == null)) { + return false; } + return caseSensitive ? StringUtil.startsWith(currentWord, prefix) : StringUtil.startsWithIgnoreCase(currentWord, prefix); + } - return comparison; + private int compare(@Nullable String s1, @Nullable String s2) { + return StringUtil.compare(s1, s2, !caseSensitive); } - private int compareNullable(@Nullable String s1, @Nullable String s2) { - if (s1 == s2) { - return 0; - } else if (s1 == null) { - return -1; - } else if (s2 == null) { - return 1; + private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull CompletionItem item2) { + if (currentWord != null) { + String label1 = item1.getLabel(); + String label2 = item2.getLabel(); + if ((label1 != null) && startsWith(label1) && + ((label2 == null) || !startsWith(label2))) { + return -1; + } else if ((label2 != null) && startsWith(label2) && + ((label1 == null) || !startsWith(label1))) { + return 1; + } } - return s1.compareTo(s2); + + return 0; } } \ No newline at end of file diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java index 3b34f462e..34ec88c8d 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java @@ -17,6 +17,7 @@ import com.intellij.openapi.editor.Editor; import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.progress.ProgressManager; +import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.UserDataHolder; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiFile; @@ -102,8 +103,6 @@ public void fillCompletionVariants(@NotNull CompletionParameters parameters, @No } } - private static final CompletionItemComparator completionProposalComparator = new CompletionItemComparator(); - private void addCompletionItems(@NotNull CompletionParameters parameters, @NotNull CompletionPrefix completionPrefix, @NotNull Either, CompletionList> completion, @@ -119,8 +118,10 @@ private void addCompletionItems(@NotNull CompletionParameters parameters, items.addAll(completionList.getItems()); } - // Sort by item.sortText - items.sort(completionProposalComparator); + // Sort the completions + String currentWord = getCurrentWord(parameters); + boolean caseSensitive = languageServer.getClientFeatures().isCaseSensitive(parameters.getOriginalFile()); + items.sort(new CompletionItemComparator(currentWord, caseSensitive)); int size = items.size(); Set addedLookupStrings = new HashSet<>(); @@ -186,6 +187,23 @@ private void addCompletionItems(@NotNull CompletionParameters parameters, } } + @Nullable + private static String getCurrentWord(@NotNull CompletionParameters parameters) { + PsiFile originalFile = parameters.getOriginalFile(); + VirtualFile virtualFile = originalFile.getVirtualFile(); + Document document = virtualFile != null ? LSPIJUtils.getDocument(virtualFile) : null; + if (document != null) { + int offset = parameters.getOffset(); + TextRange wordTextRange = LSPIJUtils.getWordRangeAt(document, originalFile, offset); + if (wordTextRange != null) { + CharSequence documentChars = document.getCharsSequence(); + CharSequence wordChars = documentChars.subSequence(wordTextRange.getStartOffset(), wordTextRange.getEndOffset()); + return wordChars.toString(); + } + } + return null; + } + protected void updateWithItemDefaults(@NotNull CompletionItem item, @Nullable CompletionItemDefaults itemDefaults) { if (itemDefaults == null) { diff --git a/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java index 5f2f2b7c3..b72f4d927 100644 --- a/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java +++ b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java @@ -16,10 +16,11 @@ import java.util.ArrayList; import java.util.List; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; public class CompletionItemComparatorTest { - private final CompletionItemComparator comparator = new CompletionItemComparator(); + private final CompletionItemComparator comparator = new CompletionItemComparator(null, false); private final CompletionItem one = newItem("one", "1"); private final CompletionItem nil = newItem("", null); From 373fc0850821ae7fce985bb73546c08f88fcb007 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 10:02:13 -0600 Subject: [PATCH 2/8] And now the prefix is also taken into account for both strict and loose/camel-hump checks. --- .../completion/CompletionItemComparator.java | 68 ++++++++++++++----- .../completion/LSPCompletionContributor.java | 3 +- .../CompletionItemComparatorTest.java | 2 +- 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index 85fd8b283..c344f9d76 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -10,6 +10,7 @@ ******************************************************************************/ package com.redhat.devtools.lsp4ij.features.completion; +import com.intellij.codeInsight.completion.PrefixMatcher; import com.intellij.openapi.util.text.StringUtil; import org.eclipse.lsp4j.CompletionItem; import org.jetbrains.annotations.NotNull; @@ -21,10 +22,14 @@ * Compares {@link CompletionItem}s by their sortText property (falls back to comparing labels) */ public class CompletionItemComparator implements Comparator { + private final PrefixMatcher prefixMatcher; private final String currentWord; private final boolean caseSensitive; - public CompletionItemComparator(@Nullable String currentWord, boolean caseSensitive) { + public CompletionItemComparator(@Nullable PrefixMatcher prefixMatcher, + @Nullable String currentWord, + boolean caseSensitive) { + this.prefixMatcher = prefixMatcher; this.currentWord = currentWord; this.caseSensitive = caseSensitive; } @@ -39,18 +44,19 @@ public int compare(CompletionItem item1, CompletionItem item2) { return 1; } - // If one is a better match for the current word than the other, prioritize it higher - // TODO: Take into account case-sensitivity with the following priorities: - // 1. Case-sensitive exact match - // 2. Case-insensitive exact match if a case-insensitive language - // 3. Prefix case-insensitive match if the all upper-case prefix - // 4. Prefix case-sensitive camel-hump match - // 5. Prefix case-insensitive camel-hump match if a case-insensitive language + // If one is a better match for the current word than the other, sort it higher int comparison = compareAgainstCurrentWord(item1, item2); if (comparison != 0) { return comparison; } + // If one is a better completion for the current prefix than the other, sort it higher + comparison = compareAgainstPrefix(item1, item2); + if (comparison != 0) { + return comparison; + } + + // Order by language server-provided sort text comparison = compare(item1.getSortText(), item2.getSortText()); if (comparison != 0) { return comparison; @@ -60,26 +66,54 @@ public int compare(CompletionItem item1, CompletionItem item2) { return compare(item1.getLabel(), item2.getLabel()); } - private boolean startsWith(@Nullable String prefix) { - if ((currentWord == null) || (prefix == null)) { + private boolean startsWith(@Nullable String string, @Nullable String prefix) { + if ((string == null) || (prefix == null)) { return false; } - return caseSensitive ? StringUtil.startsWith(currentWord, prefix) : StringUtil.startsWithIgnoreCase(currentWord, prefix); + return caseSensitive ? StringUtil.startsWith(string, prefix) : StringUtil.startsWithIgnoreCase(string, prefix); } - private int compare(@Nullable String s1, @Nullable String s2) { - return StringUtil.compare(s1, s2, !caseSensitive); + private int compare(@Nullable String string1, @Nullable String string2) { + return StringUtil.compare(string1, string2, !caseSensitive); } private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull CompletionItem item2) { if (currentWord != null) { String label1 = item1.getLabel(); String label2 = item2.getLabel(); - if ((label1 != null) && startsWith(label1) && - ((label2 == null) || !startsWith(label2))) { + + if (startsWith(currentWord, label1) && + ((label2 == null) || !startsWith(currentWord, label2))) { + return -1; + } else if (startsWith(currentWord, label2) && + ((label1 == null) || !startsWith(currentWord, label1))) { + return 1; + } + } + + return 0; + } + + private int compareAgainstPrefix(@NotNull CompletionItem item1, @NotNull CompletionItem item2) { + if (prefixMatcher != null) { + String prefix = prefixMatcher.getPrefix(); + String label1 = item1.getLabel(); + String label2 = item2.getLabel(); + + // Start starts with + if (startsWith(label1, prefix) && + (!startsWith(label2, prefix))) { + return -1; + } else if (startsWith(label2, prefix) && + (!startsWith(label1, prefix))) { + return 1; + } + // Loose/camel-hump starts with + else if ((label1 != null) && prefixMatcher.isStartMatch(label1) && + ((label2 == null) || !prefixMatcher.isStartMatch(label2))) { return -1; - } else if ((label2 != null) && startsWith(label2) && - ((label1 == null) || !startsWith(label1))) { + } else if ((label2 != null) && prefixMatcher.isStartMatch(label2) && + ((label1 == null) || !prefixMatcher.isStartMatch(label1))) { return 1; } } diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java index 34ec88c8d..8a805f16b 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java @@ -119,9 +119,10 @@ private void addCompletionItems(@NotNull CompletionParameters parameters, } // Sort the completions + PrefixMatcher prefixMatcher = result.getPrefixMatcher(); String currentWord = getCurrentWord(parameters); boolean caseSensitive = languageServer.getClientFeatures().isCaseSensitive(parameters.getOriginalFile()); - items.sort(new CompletionItemComparator(currentWord, caseSensitive)); + items.sort(new CompletionItemComparator(prefixMatcher, currentWord, caseSensitive)); int size = items.size(); Set addedLookupStrings = new HashSet<>(); diff --git a/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java index b72f4d927..8b7fc9092 100644 --- a/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java +++ b/src/test/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparatorTest.java @@ -20,7 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class CompletionItemComparatorTest { - private final CompletionItemComparator comparator = new CompletionItemComparator(null, false); + private final CompletionItemComparator comparator = new CompletionItemComparator(null, null, false); private final CompletionItem one = newItem("one", "1"); private final CompletionItem nil = newItem("", null); From 0aaba07209ae631d4457cbd385c8aa7d3fa093ae Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 10:08:00 -0600 Subject: [PATCH 3/8] More flexibility when comparing against the current word. --- .../features/completion/CompletionItemComparator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index c344f9d76..8d3a9cf37 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -82,11 +82,11 @@ private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull Co String label1 = item1.getLabel(); String label2 = item2.getLabel(); - if (startsWith(currentWord, label1) && - ((label2 == null) || !startsWith(currentWord, label2))) { + if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && + ((label2 == null) || !(startsWith(currentWord, label2) || startsWith(label2, currentWord)))) { return -1; - } else if (startsWith(currentWord, label2) && - ((label1 == null) || !startsWith(currentWord, label1))) { + } else if ((startsWith(currentWord, label2) || startsWith(label2, currentWord)) && + ((label1 == null) || !(startsWith(currentWord, label1) || startsWith(label1, currentWord)))) { return 1; } } From 8ef74d0936621e5233f1851389325d7378397647 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 10:12:34 -0600 Subject: [PATCH 4/8] Yet more flexibility when comparing against the current word. --- .../completion/CompletionItemComparator.java | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index 8d3a9cf37..7102d1af1 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -66,6 +66,14 @@ public int compare(CompletionItem item1, CompletionItem item2) { return compare(item1.getLabel(), item2.getLabel()); } + private int compare(@Nullable String string1, @Nullable String string2) { + return StringUtil.compare(string1, string2, !caseSensitive); + } + + private boolean equals(@Nullable String string1, @Nullable String string2) { + return StringUtil.compare(string1, string2, !caseSensitive) == 0; + } + private boolean startsWith(@Nullable String string, @Nullable String prefix) { if ((string == null) || (prefix == null)) { return false; @@ -73,8 +81,11 @@ private boolean startsWith(@Nullable String string, @Nullable String prefix) { return caseSensitive ? StringUtil.startsWith(string, prefix) : StringUtil.startsWithIgnoreCase(string, prefix); } - private int compare(@Nullable String string1, @Nullable String string2) { - return StringUtil.compare(string1, string2, !caseSensitive); + private boolean contains(@Nullable String string, @Nullable String substring) { + if ((string == null) || (substring == null)) { + return false; + } + return caseSensitive ? StringUtil.contains(string, substring) : StringUtil.containsIgnoreCase(string, substring); } private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull CompletionItem item2) { @@ -82,13 +93,30 @@ private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull Co String label1 = item1.getLabel(); String label2 = item2.getLabel(); - if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && + // Exact match + if (equals(currentWord, label1) && + ((label2 == null) || !equals(currentWord, label2))) { + return -1; + } else if (equals(currentWord, label2) && + ((label1 == null) || !equals(currentWord, label1))) { + return 1; + } + // Starts with + else if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && ((label2 == null) || !(startsWith(currentWord, label2) || startsWith(label2, currentWord)))) { return -1; } else if ((startsWith(currentWord, label2) || startsWith(label2, currentWord)) && ((label1 == null) || !(startsWith(currentWord, label1) || startsWith(label1, currentWord)))) { return 1; } + // Contains + else if (contains(currentWord, label1) && + ((label2 == null) || !contains(currentWord, label2))) { + return -1; + } else if (contains(currentWord, label2) && + ((label1 == null) || !contains(currentWord, label1))) { + return 1; + } } return 0; From 43063be7a0a421728da758749f6148a0e94e3701 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 10:14:08 -0600 Subject: [PATCH 5/8] Yet even more flexibility when comparing against the current word. Hopefully this does it. --- .../features/completion/CompletionItemComparator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index 7102d1af1..f717ac30f 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -110,11 +110,11 @@ else if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && return 1; } // Contains - else if (contains(currentWord, label1) && - ((label2 == null) || !contains(currentWord, label2))) { + else if ((contains(currentWord, label1) || contains(label1, currentWord)) && + ((label2 == null) || !(contains(currentWord, label2) || contains(label2, currentWord)))) { return -1; - } else if (contains(currentWord, label2) && - ((label1 == null) || !contains(currentWord, label1))) { + } else if ((contains(currentWord, label2) || contains(label2, currentWord)) && + ((label1 == null) || !(contains(currentWord, label1) || contains(label1, currentWord)))) { return 1; } } From c115402dd1eaef5ca9e5274f52bea4de121bdf53 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sat, 14 Dec 2024 10:49:48 -0600 Subject: [PATCH 6/8] Current word and prefix comparisons are not performed for quoted string completion offerings. Removed current word "contains" checks because they could short-circuit more accurate prefix checks. --- .../completion/CompletionItemComparator.java | 86 +++++++++---------- 1 file changed, 39 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java index f717ac30f..1be0a069c 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/CompletionItemComparator.java @@ -81,41 +81,30 @@ private boolean startsWith(@Nullable String string, @Nullable String prefix) { return caseSensitive ? StringUtil.startsWith(string, prefix) : StringUtil.startsWithIgnoreCase(string, prefix); } - private boolean contains(@Nullable String string, @Nullable String substring) { - if ((string == null) || (substring == null)) { - return false; - } - return caseSensitive ? StringUtil.contains(string, substring) : StringUtil.containsIgnoreCase(string, substring); - } - private int compareAgainstCurrentWord(@NotNull CompletionItem item1, @NotNull CompletionItem item2) { if (currentWord != null) { String label1 = item1.getLabel(); String label2 = item2.getLabel(); - - // Exact match - if (equals(currentWord, label1) && - ((label2 == null) || !equals(currentWord, label2))) { - return -1; - } else if (equals(currentWord, label2) && - ((label1 == null) || !equals(currentWord, label1))) { - return 1; - } - // Starts with - else if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && - ((label2 == null) || !(startsWith(currentWord, label2) || startsWith(label2, currentWord)))) { - return -1; - } else if ((startsWith(currentWord, label2) || startsWith(label2, currentWord)) && - ((label1 == null) || !(startsWith(currentWord, label1) || startsWith(label1, currentWord)))) { - return 1; - } - // Contains - else if ((contains(currentWord, label1) || contains(label1, currentWord)) && - ((label2 == null) || !(contains(currentWord, label2) || contains(label2, currentWord)))) { - return -1; - } else if ((contains(currentWord, label2) || contains(label2, currentWord)) && - ((label1 == null) || !(contains(currentWord, label1) || contains(label1, currentWord)))) { - return 1; + // Don't do this for completion offerings that are quoted strings + if (((label1 == null) || !StringUtil.isQuotedString(label1)) && + ((label2 == null) || !StringUtil.isQuotedString(label2))) { + // Exact match + if (equals(currentWord, label1) && + ((label2 == null) || !equals(currentWord, label2))) { + return -1; + } else if (equals(currentWord, label2) && + ((label1 == null) || !equals(currentWord, label1))) { + return 1; + } + + // Starts with + else if ((startsWith(currentWord, label1) || startsWith(label1, currentWord)) && + ((label2 == null) || !(startsWith(currentWord, label2) || startsWith(label2, currentWord)))) { + return -1; + } else if ((startsWith(currentWord, label2) || startsWith(label2, currentWord)) && + ((label1 == null) || !(startsWith(currentWord, label1) || startsWith(label1, currentWord)))) { + return 1; + } } } @@ -127,22 +116,25 @@ private int compareAgainstPrefix(@NotNull CompletionItem item1, @NotNull Complet String prefix = prefixMatcher.getPrefix(); String label1 = item1.getLabel(); String label2 = item2.getLabel(); - - // Start starts with - if (startsWith(label1, prefix) && - (!startsWith(label2, prefix))) { - return -1; - } else if (startsWith(label2, prefix) && - (!startsWith(label1, prefix))) { - return 1; - } - // Loose/camel-hump starts with - else if ((label1 != null) && prefixMatcher.isStartMatch(label1) && - ((label2 == null) || !prefixMatcher.isStartMatch(label2))) { - return -1; - } else if ((label2 != null) && prefixMatcher.isStartMatch(label2) && - ((label1 == null) || !prefixMatcher.isStartMatch(label1))) { - return 1; + // Don't do this for completion offerings that are quoted strings + if (((label1 == null) || !StringUtil.isQuotedString(label1)) && + ((label2 == null) || !StringUtil.isQuotedString(label2))) { + // Start starts with + if (startsWith(label1, prefix) && + (!startsWith(label2, prefix))) { + return -1; + } else if (startsWith(label2, prefix) && + (!startsWith(label1, prefix))) { + return 1; + } + // Loose/camel-hump starts with + else if ((label1 != null) && prefixMatcher.isStartMatch(label1) && + ((label2 == null) || !prefixMatcher.isStartMatch(label2))) { + return -1; + } else if ((label2 != null) && prefixMatcher.isStartMatch(label2) && + ((label1 == null) || !prefixMatcher.isStartMatch(label1))) { + return 1; + } } } From 4edcca268ce8dbf8122b57345c562e1cc6caac3c Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sun, 22 Dec 2024 11:39:23 -0600 Subject: [PATCH 7/8] Made context-aware code completion sorting an opt-in client-side configuration option that defaults to disabled. Note that even when disabled, this still includes the changes to use the configured language case-sensitivity when comparing completions as that should likely have been included in the original changes for code completions using language case-sensitivity. --- .../client/features/LSPCompletionFeature.java | 33 ++++++++++++------- .../completion/LSPCompletionContributor.java | 15 ++++++--- .../ClientConfigurationSettings.java | 19 +++++++++-- .../launching/UserDefinedClientFeatures.java | 1 + .../UserDefinedCompletionFeature.java | 31 +++++++++++++++++ .../jsonSchema/clientSettings.schema.json | 13 ++++++++ 6 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedCompletionFeature.java diff --git a/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java b/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java index ce6bea948..9dd0d5e2d 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java @@ -97,7 +97,7 @@ public boolean isCompletionSupported(@NotNull PsiFile file) { * @param file the file. * @return true the file associated with a language server can support resolve completion and false otherwise. */ - public boolean isResolveCompletionSupported(@Nullable PsiFile file) { + public boolean isResolveCompletionSupported(@NotNull PsiFile file) { return getCompletionCapabilityRegistry().isResolveCompletionSupported(file); } @@ -182,7 +182,7 @@ public Icon getIcon(@NotNull CompletionItem item) { /** * Returns true if the IntelliJ lookup is strike out and false otherwise. * - * @param item + * @param item the completion item * @return true if the IntelliJ lookup is strike out and false otherwise. */ public boolean isStrikeout(@NotNull CompletionItem item) { @@ -215,12 +215,12 @@ public boolean isItemTextBold(@NotNull CompletionItem item) { /** * Don't override this method, we need to revisit the API and the prefix computation (to customize it). * - * @param context - * @param completionPrefix - * @param result - * @param lookupItem - * @param priority - * @param item + * @param context the completion context + * @param completionPrefix the completion prefix + * @param result the completion result set + * @param lookupItem the lookup item + * @param priority the completion priority + * @param item the completion item */ @ApiStatus.Internal public void addLookupItem(@NotNull LSPCompletionContext context, @@ -260,11 +260,11 @@ public void addLookupItem(@NotNull LSPCompletionContext context, } /** - * Returns true if completion item must be resolved and false otherwise when completion item is applied. + * Returns true if completion item must be resolved and false otherwise when completion item is used. * - * @param item the completion item which is applied. + * @param item the completion item which is used. * @param file the file. - * @return true if completion item must be resolved and false otherwise when completion item is applied. + * @return true if completion item must be resolved and false otherwise when completion item is used. */ public boolean shouldResolveOnApply(@NotNull CompletionItem item, @NotNull PsiFile file) { @@ -293,4 +293,15 @@ public void setServerCapabilities(@Nullable ServerCapabilities serverCapabilitie completionCapabilityRegistry.setServerCapabilities(serverCapabilities); } } + + /** + * Determines whether or not client-side context-aware completion sorting should be used for the specified file. + * + * @param file the file + * @return true if client-side context-aware completion sorting should be used; otherwise false + */ + public boolean useContextAwareSorting(@NotNull PsiFile file) { + // Default to disabled + return false; + } } diff --git a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java index 8a805f16b..5cc405652 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/features/completion/LSPCompletionContributor.java @@ -26,6 +26,7 @@ import com.redhat.devtools.lsp4ij.LSPIJUtils; import com.redhat.devtools.lsp4ij.LanguageServerItem; import com.redhat.devtools.lsp4ij.client.ExecuteLSPFeatureStatus; +import com.redhat.devtools.lsp4ij.client.features.LSPClientFeatures; import com.redhat.devtools.lsp4ij.client.features.LSPCompletionFeature; import com.redhat.devtools.lsp4ij.client.features.LSPCompletionProposal; import com.redhat.devtools.lsp4ij.client.indexing.ProjectIndexingManager; @@ -118,15 +119,19 @@ private void addCompletionItems(@NotNull CompletionParameters parameters, items.addAll(completionList.getItems()); } - // Sort the completions - PrefixMatcher prefixMatcher = result.getPrefixMatcher(); - String currentWord = getCurrentWord(parameters); - boolean caseSensitive = languageServer.getClientFeatures().isCaseSensitive(parameters.getOriginalFile()); + PsiFile originalFile = parameters.getOriginalFile(); + LSPClientFeatures clientFeatures = languageServer.getClientFeatures(); + + // Sort the completions as appropriate based on client configuration + boolean useContextAwareSorting = clientFeatures.getCompletionFeature().useContextAwareSorting(originalFile); + PrefixMatcher prefixMatcher = useContextAwareSorting ? result.getPrefixMatcher() : null; + String currentWord = useContextAwareSorting ? getCurrentWord(parameters) : null; + boolean caseSensitive = clientFeatures.isCaseSensitive(originalFile); items.sort(new CompletionItemComparator(prefixMatcher, currentWord, caseSensitive)); int size = items.size(); Set addedLookupStrings = new HashSet<>(); - var completionFeature = languageServer.getClientFeatures().getCompletionFeature(); + var completionFeature = clientFeatures.getCompletionFeature(); LSPCompletionFeature.LSPCompletionContext context = new LSPCompletionFeature.LSPCompletionContext(parameters, languageServer); // Items now sorted by priority, low index == high priority for (int i = 0; i < size; i++) { diff --git a/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/ClientConfigurationSettings.java b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/ClientConfigurationSettings.java index f3e9b15f7..82351b77a 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/ClientConfigurationSettings.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/ClientConfigurationSettings.java @@ -20,7 +20,17 @@ */ public class ClientConfigurationSettings { /** - * Client-side code workspace symbol settings. + * Client-side code completion settings. + */ + public static class ClientConfigurationCompletionSettings { + /** + * Whether or not client-side context-aware completion sorting should be used. Defaults to false. + */ + public boolean useContextAwareSorting = false; + } + + /** + * Client-side workspace symbol settings. */ public static class ClientConfigurationWorkspaceSymbolSettings { /** @@ -35,7 +45,12 @@ public static class ClientConfigurationWorkspaceSymbolSettings { public boolean caseSensitive = false; /** - * Client-side code workspace symbol settings + * Client-side code completion settings + */ + public @NotNull ClientConfigurationCompletionSettings completion = new ClientConfigurationCompletionSettings(); + + /** + * Client-side workspace symbol settings */ public @NotNull ClientConfigurationWorkspaceSymbolSettings workspaceSymbol = new ClientConfigurationWorkspaceSymbolSettings(); } \ No newline at end of file diff --git a/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedClientFeatures.java b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedClientFeatures.java index 093294da3..3ae59b3a1 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedClientFeatures.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedClientFeatures.java @@ -26,6 +26,7 @@ public UserDefinedClientFeatures() { super(); // Use the extended feature implementations + setCompletionFeature(new UserDefinedCompletionFeature()); setWorkspaceSymbolFeature(new UserDefinedWorkspaceSymbolFeature()); } diff --git a/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedCompletionFeature.java b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedCompletionFeature.java new file mode 100644 index 000000000..986f8311e --- /dev/null +++ b/src/main/java/com/redhat/devtools/lsp4ij/server/definition/launching/UserDefinedCompletionFeature.java @@ -0,0 +1,31 @@ +/******************************************************************************* + * Copyright (c) 2024 Red Hat Inc. and others. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 + * which is available at https://www.apache.org/licenses/LICENSE-2.0. + * + * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 + * + * Contributors: + * Red Hat Inc. - initial API and implementation + *******************************************************************************/ +package com.redhat.devtools.lsp4ij.server.definition.launching; + +import com.intellij.psi.PsiFile; +import com.redhat.devtools.lsp4ij.client.features.LSPCompletionFeature; +import org.jetbrains.annotations.NotNull; + +/** + * Adds client-side code completion configuration features. + */ +public class UserDefinedCompletionFeature extends LSPCompletionFeature { + + @Override + public boolean useContextAwareSorting(@NotNull PsiFile file) { + UserDefinedLanguageServerDefinition serverDefinition = (UserDefinedLanguageServerDefinition) getClientFeatures().getServerDefinition(); + ClientConfigurationSettings clientConfiguration = serverDefinition.getLanguageServerClientConfiguration(); + return clientConfiguration != null ? clientConfiguration.completion.useContextAwareSorting : super.useContextAwareSorting(file); + } +} \ No newline at end of file diff --git a/src/main/resources/jsonSchema/clientSettings.schema.json b/src/main/resources/jsonSchema/clientSettings.schema.json index 907db4f3f..5d858af0c 100644 --- a/src/main/resources/jsonSchema/clientSettings.schema.json +++ b/src/main/resources/jsonSchema/clientSettings.schema.json @@ -11,6 +11,19 @@ "description": "Whether or not the language grammar is case-sensitive.", "default": false }, + "completion": { + "type": "object", + "title": "Client-side completion configuration", + "additionalProperties": false, + "properties": { + "useContextAwareSorting": { + "type": "boolean", + "title": "Use context-aware completion sorting", + "description": "Whether or not client-side context-aware completion sorting should be used.", + "default": false + } + } + }, "workspaceSymbol": { "type": "object", "title": "Client-side workspace symbol configuration", From 78feed2d4abb5db5b09253114100195ad162d6f7 Mon Sep 17 00:00:00 2001 From: Scott Wells Date: Sun, 22 Dec 2024 11:44:58 -0600 Subject: [PATCH 8/8] Reverted some comment changes that should not have occurred. --- .../lsp4ij/client/features/LSPCompletionFeature.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java b/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java index 9dd0d5e2d..ed8c607e8 100644 --- a/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java +++ b/src/main/java/com/redhat/devtools/lsp4ij/client/features/LSPCompletionFeature.java @@ -260,11 +260,11 @@ public void addLookupItem(@NotNull LSPCompletionContext context, } /** - * Returns true if completion item must be resolved and false otherwise when completion item is used. + * Returns true if completion item must be resolved and false otherwise when completion item is applied. * - * @param item the completion item which is used. + * @param item the completion item which is applied. * @param file the file. - * @return true if completion item must be resolved and false otherwise when completion item is used. + * @return true if completion item must be resolved and false otherwise when completion item is applied. */ public boolean shouldResolveOnApply(@NotNull CompletionItem item, @NotNull PsiFile file) {