Improve completion proposal sorting, highlighting and insertion
* Show most matching first
* Better handling of prefix/suffix insertion
* Better handling of matching parts
Change-Id: I0609b897d9489e30adb4626e67de537af9df1bd0
Signed-off-by: Mickael Istria <mistria@redhat.com>
diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/CompletionTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/CompletionTest.java
index 3184271..b06e8e8 100644
--- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/CompletionTest.java
+++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/completion/CompletionTest.java
@@ -15,6 +15,7 @@
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@@ -178,7 +179,7 @@
assertEquals("FirstClass", viewer.getDocument().get());
assertEquals(new Point("FirstClass".length(), 0), lsCompletionProposal.getSelection(viewer.getDocument()));
}
-
+
@Test
public void testApplyCompletionReplaceAndTyping() throws CoreException, InvocationTargetException, BadLocationException {
Range range = new Range(new Position(0, 0), new Position(0, 20));
@@ -229,4 +230,31 @@
return item;
}
+ @Test
+ public void testItemOrdering() throws Exception {
+ Range range = new Range(new Position(0, 0), new Position(0, 1));
+ List<CompletionItem> items = Arrays.asList(new CompletionItem[] {
+ createCompletionItem("AA", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ createCompletionItem("AB", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ createCompletionItem("BA", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ createCompletionItem("BB", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ createCompletionItem("CB", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ createCompletionItem("CC", CompletionItemKind.Class, new Range(new Position(0, 0), new Position(0, 1))),
+ });
+ MockLanguageSever.INSTANCE.setCompletionList(new CompletionList(false, items));
+
+ String content = "B";
+ ITextViewer viewer = TestUtils.openTextViewer(TestUtils.createUniqueTestFile(project,content));
+
+ int invokeOffset = 1;
+ ICompletionProposal[] proposals = contentAssistProcessor.computeCompletionProposals(viewer, invokeOffset);
+ assertEquals(4, proposals.length); // only those containing a "B"
+ assertEquals("BA", proposals[0].getDisplayString());
+ assertEquals("BB", proposals[1].getDisplayString());
+ assertEquals("AB", proposals[2].getDisplayString());
+ assertEquals("CB", proposals[3].getDisplayString());
+
+ ((LSCompletionProposal)proposals[0]).apply(viewer, '\n', 0, invokeOffset);
+ assertEquals("BA", viewer.getDocument().get());
+ }
}
diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java
index b6ee421..53c99f9 100644
--- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java
+++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSCompletionProposal.java
@@ -68,7 +68,8 @@
private static final String EDIT_AREA_OPEN_PATTERN = "{{"; //$NON-NLS-1$
private static final String EDIT_AREA_CLOSE_PATTERN = "}}"; //$NON-NLS-1$
private CompletionItem item;
- private int initialOffset;
+ private int initialOffset = -1;
+ private int bestOffset = -1;
private ITextViewer viewer;
private IRegion selection;
private LinkedPosition firstPosition;
@@ -76,17 +77,26 @@
public LSCompletionProposal(@NonNull CompletionItem item, int offset, LSPDocumentInfo info) {
this.item = item;
- this.initialOffset = offset;
this.info = info;
+ this.initialOffset = offset;
+ this.bestOffset = getPrefixCompletionStart(info.getDocument(), offset);
+ }
+
+ public int getBestOffset() {
+ return this.bestOffset;
+ }
+
+ public CompletionItem getItem() {
+ return this.item;
}
@Override
public StyledString getStyledDisplayString(IDocument document, int offset, BoldStylerProvider boldStylerProvider) {
String rawString = getDisplayString();
StyledString res = new StyledString(rawString);
- if (offset > this.initialOffset) {
+ if (offset > this.bestOffset) {
try {
- String subString = document.get(this.initialOffset, offset - this.initialOffset);
+ String subString = document.get(this.bestOffset, offset - this.bestOffset);
if (item.getTextEdit() != null) {
int start = LSPEclipseUtils.toOffset(item.getTextEdit().getRange().getStart(), document);
int end = offset;
@@ -94,8 +104,9 @@
}
int lastIndex = 0;
subString = subString.toLowerCase();
+ String lowerRawString = rawString.toLowerCase();
for (Character c : subString.toCharArray()) {
- int index = rawString.indexOf(c, lastIndex);
+ int index = lowerRawString.indexOf(c, lastIndex);
if (index < 0) {
return res;
} else {
@@ -201,11 +212,32 @@
@Override
public CharSequence getPrefixCompletionText(IDocument document, int completionOffset) {
- return item.getInsertText().substring(completionOffset - this.initialOffset);
+ return item.getInsertText().substring(completionOffset - this.bestOffset);
}
@Override
public int getPrefixCompletionStart(IDocument document, int completionOffset) {
+ if (this.item.getTextEdit() != null) {
+ try {
+ return LSPEclipseUtils.toOffset(this.item.getTextEdit().getRange().getStart(), document);
+ } catch (BadLocationException e) {
+ LanguageServerPlugin.logError(e);
+ }
+ }
+ String insertText = getInsertText();
+ try {
+ String subDoc = document.get(
+ Math.max(0, completionOffset - insertText.length()),
+ Math.min(insertText.length(), completionOffset));
+ for (int i = 0; i < insertText.length() && i < completionOffset; i++) {
+ String tentativeCommonString = subDoc.substring(i);
+ if (insertText.startsWith(tentativeCommonString)) {
+ return completionOffset - tentativeCommonString.length();
+ }
+ }
+ } catch (BadLocationException e) {
+ LanguageServerPlugin.logError(e);
+ }
return completionOffset;
}
@@ -223,11 +255,11 @@
if (item.getLabel() == null || item.getLabel().isEmpty()) {
return false;
}
- if (offset < this.initialOffset) {
+ if (offset < this.bestOffset) {
return false;
}
try {
- String subString = document.get(this.initialOffset, offset - this.initialOffset);
+ String subString = document.get(this.bestOffset, offset - this.bestOffset);
String insert = getInsertText();
if (item.getTextEdit() != null) {
int start = LSPEclipseUtils.toOffset(item.getTextEdit().getRange().getStart(), document);
@@ -267,18 +299,17 @@
@Override
public void apply(IDocument document) {
- apply(document, Character.MIN_VALUE, 0, this.initialOffset);
+ apply(document, Character.MIN_VALUE, 0, this.bestOffset);
}
private void apply(IDocument document, char trigger, int stateMask, int offset) {
String insertText = null;
- int insertionOffset = this.initialOffset;
TextEdit textEdit = item.getTextEdit();
try {
if (textEdit == null) {
insertText = getInsertText();
- Position start = LSPEclipseUtils.toPosition(this.initialOffset, document);
- Position end = LSPEclipseUtils.toPosition(this.initialOffset, document); // need 2 distinct objects
+ Position start = LSPEclipseUtils.toPosition(this.bestOffset, document);
+ Position end = LSPEclipseUtils.toPosition(offset, document); // need 2 distinct objects
textEdit = new TextEdit(new Range(start, end), insertText);
}
{ // workaround https://github.com/Microsoft/vscode/issues/17036
@@ -297,14 +328,24 @@
textEdit.getRange().setEnd(documentEnd);
}
}
- {
- // shift range if user typed something after invoking completion
- if (insertionOffset != offset) {
- int shift = offset - insertionOffset;
- textEdit.getRange().getEnd().setCharacter(textEdit.getRange().getEnd().getCharacter() + shift);
+ if (offset > this.initialOffset) {
+ // characters were added after completion was activated
+ int shift = offset - this.initialOffset;
+ textEdit.getRange().getEnd().setCharacter(textEdit.getRange().getEnd().getCharacter() + shift);
+ }
+ if (insertText != null) {
+ // try to reuse existing characters after completion location
+ int shift = offset - this.bestOffset;
+ int commonSize = 0;
+ while (commonSize < insertText.length() - shift
+ && document.getLength() > offset + commonSize
+ && document.getChar(this.bestOffset + shift + commonSize) == insertText.charAt(commonSize + shift)) {
+ commonSize++;
}
+ textEdit.getRange().getEnd().setCharacter(textEdit.getRange().getEnd().getCharacter() + commonSize);
}
insertText = textEdit.getNewText();
+ // TODO apply additional text edits too. apply from bottom to top of the document
LSPEclipseUtils.applyEdit(textEdit, document);
selection = new Region(LSPEclipseUtils.toOffset(textEdit.getRange().getStart(), document) + textEdit.getNewText().length(), 0);
} catch (BadLocationException e) {
@@ -313,9 +354,10 @@
}
if (viewer != null) {
- LinkedHashMap<String, LinkedPositionGroup> groups = new LinkedHashMap<>();
- int currentOffset = insertText.indexOf(EDIT_AREA_OPEN_PATTERN);
try {
+ int insertionOffset = LSPEclipseUtils.toOffset(textEdit.getRange().getStart(), document);
+ LinkedHashMap<String, LinkedPositionGroup> groups = new LinkedHashMap<>();
+ int currentOffset = insertText.indexOf(EDIT_AREA_OPEN_PATTERN);
while (currentOffset != -1) {
int closeOffset = insertText.indexOf(EDIT_AREA_CLOSE_PATTERN,
currentOffset + EDIT_AREA_OPEN_PATTERN.length());
@@ -361,6 +403,9 @@
private String getInsertText() {
String insertText = this.item.getInsertText();
+ if (this.item.getTextEdit() != null) {
+ insertText = this.item.getTextEdit().getNewText();
+ }
if (insertText == null) {
insertText = this.item.getLabel();
}
@@ -411,4 +456,36 @@
return getAdditionalProposalInfo();
}
+ public String getSortText() {
+ if (item.getSortText() != null && !item.getSortText().isEmpty()) {
+ return item.getSortText();
+ }
+ return item.getLabel();
+ }
+
+ public int getNumberOfModifsBeforeOffset() {
+ if (this.item.getTextEdit() == null) {
+ // only insertion and offset is moved back in case document contains prefix
+ // of insertion, so no change done before offset
+ return 0;
+ }
+ int res = 0;
+ try {
+ int startOffset = LSPEclipseUtils.toOffset(this.item.getTextEdit().getRange().getStart(), this.info.getDocument());
+ String insert = this.item.getTextEdit().getNewText();
+ String subDoc = this.info.getDocument().get(startOffset, Math.min(
+ startOffset + insert.length(),
+ this.info.getDocument().getLength()));
+ for (int i = 0; i < subDoc.length() && i < insert.length(); i++) {
+ if (subDoc.charAt(i) != insert.charAt(i)) {
+ res++;
+ }
+ }
+ } catch (BadLocationException e) {
+ LanguageServerPlugin.logError(e);
+ }
+ return res;
+
+ }
+
}
diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSContentAssistProcessor.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSContentAssistProcessor.java
index 4b1b2ee..bc8ad5d 100644
--- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSContentAssistProcessor.java
+++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSContentAssistProcessor.java
@@ -11,8 +11,6 @@
package org.eclipse.lsp4e.operations.completion;
import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.List;
import java.util.concurrent.CompletableFuture;
@@ -101,20 +99,8 @@
return new ICompletionProposal[0];
}
- List<CompletionItem> items = new ArrayList<>(completionList.getItems());
- Collections.sort(items, new Comparator<CompletionItem>() {
- @Override
- public int compare(CompletionItem o1, CompletionItem o2) {
- String c1 = getComparableLabel(o1);
- String c2 = getComparableLabel(o2);
- if (c1 == null) {
- return -1;
- }
- return c1.compareToIgnoreCase(c2);
- }
- });
- List<ICompletionProposal> proposals = new ArrayList<>();
- for (CompletionItem item : items) {
+ List<LSCompletionProposal> proposals = new ArrayList<>();
+ for (CompletionItem item : completionList.getItems()) {
if (item != null) {
LSCompletionProposal proposal = new LSCompletionProposal(item, offset, info);
if (proposal.validate(info.getDocument(), offset, null)) {
@@ -122,16 +108,28 @@
}
}
}
+ proposals.sort((o1, o2) -> {
+ // prefer the one that matches the most backward
+ if (o1.getBestOffset() < o2.getBestOffset()) {
+ return -1;
+ } else if (o1.getBestOffset() > o2.getBestOffset()) {
+ return +1;
+ } else if (o1.getNumberOfModifsBeforeOffset() < o2.getNumberOfModifsBeforeOffset()) {
+ return -1;
+ } else if (o1.getNumberOfModifsBeforeOffset() > o2.getNumberOfModifsBeforeOffset()) {
+ return +1;
+ } else {
+ String c1 = o1.getSortText();
+ String c2 = o2.getSortText();
+ if (c1 == null) {
+ return -1;
+ }
+ return c1.compareToIgnoreCase(c2);
+ }
+ });
return proposals.toArray(new ICompletionProposal[proposals.size()]);
}
- private String getComparableLabel(CompletionItem item) {
- if (item.getSortText() != null && !item.getSortText().isEmpty()) {
- return item.getSortText();
- }
- return item.getLabel();
- }
-
@Override
public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
// TODO
