From d4439b246b6a06ab3c6cf726d380084bfbd30df3 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Wed, 5 Oct 2016 11:05:28 -0700 Subject: [PATCH] Manifest.yml completions working now in vscode --- .../commons/completion/DocumentEdits.java | 101 ++++++++++++++++-- .../ide/vscode/util/BadLocationException.java | 2 + .../ide/vscode/util/IDocument.java | 1 + .../ide/vscode/util/TextDocument.java | 6 +- .../ide/vscode/testharness/Editor.java | 8 ++ .../vscode/yaml/completion/YamlPathEdits.java | 17 ++- .../ide/vscode/yaml/util/YamlIndentUtil.java | 2 +- .../editor/ManifestYamlLanguageServer.java | 2 +- .../editor/VscodeCompletionEngineAdapter.java | 27 +++-- .../yaml/ManifestYamlLanguageServerTest.java | 6 +- 10 files changed, 137 insertions(+), 35 deletions(-) diff --git a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/commons/completion/DocumentEdits.java b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/commons/completion/DocumentEdits.java index b82d75a8f..e1fc75d0c 100644 --- a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/commons/completion/DocumentEdits.java +++ b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/commons/completion/DocumentEdits.java @@ -77,7 +77,7 @@ public class DocumentEdits implements ProposalApplier { // transform function just like the current implementation does. // => When the cluster of 'conflicting' operations has been dealt with // the offset transform function no longer matters for the - // remaining edits who's offesets are all strictly 'smaller'. + // remaining edits who's offsets are all strictly 'smaller'. // Thus the trasnform function can be discarded. // // Assuming most edits are independent and only a few of them conflict, then @@ -91,6 +91,40 @@ public class DocumentEdits implements ProposalApplier { // // The tricky part would be to preserve the order-dependent semantics. + public static class TextReplace { + public final int start; + public final int end; + public final String newText; + + public TextReplace(int start, int end, String newText) { + super(); + this.start = start; + this.end = end; + this.newText = newText; + } + + public IRegion getRegion() { + return new Region(start, end-start); + } + } + + /** + * When an insert occurs, a single position in the file transforms ambiguously into two different + * positions after the insertion, depending on whether we 'float' marker for the position after the + * inserted block or leave it at the beginning. + */ + public enum Direction { + /** + * Transform positions around inserts to stick to the front of the inserted block. + */ + BEFORE, + /** + * + * Transform positions around inserts to stick to the end of the inserted block. + */ + AFTER + } + private class Insertion extends Edit { private int offset; private String text; @@ -109,9 +143,21 @@ public class DocumentEdits implements ProposalApplier { public String toString() { return "ins("+text+"@"+offset+")"; } + + @Override + public int getStart() { + return offset; + } + + @Override + public int getEnd() { + return offset; + } } private abstract class Edit { + public abstract int getStart(); + public abstract int getEnd(); abstract void apply(DocumentState doc) throws BadLocationException; public abstract String toString(); } @@ -136,14 +182,25 @@ public class DocumentEdits implements ProposalApplier { return "del("+start+"->"+end+")"; } + @Override + public int getStart() { + return start; + } + + @Override + public int getEnd() { + return end; + } + } private interface OffsetTransformer { - int trasform(int offset); + int transform(int offset, Direction dir); } private static final OffsetTransformer NULL_TRANSFORM = new OffsetTransformer() { - public int trasform(int offset) { + @Override + public int transform(int offset, Direction dir) { return offset; } }; @@ -164,19 +221,25 @@ public class DocumentEdits implements ProposalApplier { } public void insert(int start, final String text) throws BadLocationException { - final int tStart = org2new.trasform(start); + final int tStart = org2new.transform(start, Direction.AFTER); if (!text.isEmpty()) { if (doc!=null) { doc.replace(tStart, 0, text); } final OffsetTransformer parent = org2new; org2new = new OffsetTransformer() { - public int trasform(int org) { - int tOffset = parent.trasform(org); + public int transform(int org, Direction dir) { + int tOffset = parent.transform(org, dir); if (tOffsettStart) { return tOffset + text.length(); + } else /* tOffset==tStart*/ { + if (dir==Direction.BEFORE) { + return tOffset; + } else { + return tOffset + text.length(); + } } } }; @@ -185,9 +248,9 @@ public class DocumentEdits implements ProposalApplier { } public void delete(final int start, final int end) throws BadLocationException { - final int tStart = org2new.trasform(start); + final int tStart = org2new.transform(start, Direction.AFTER); if (end>start) { // skip work for 'delete nothing' op - final int tEnd = org2new.trasform(end); + final int tEnd = org2new.transform(end, Direction.AFTER); if (tEnd>tStart) { // skip work for 'delete nothing' op if (doc!=null) { doc.replace(tStart, tEnd-tStart, ""); @@ -195,8 +258,8 @@ public class DocumentEdits implements ProposalApplier { final OffsetTransformer parent = org2new; org2new = new OffsetTransformer() { - public int trasform(int org) { - int tOffset = parent.trasform(org); + public int transform(int org, Direction dir) { + int tOffset = parent.transform(org, dir); if (tOffset<=tStart) { return tOffset; } else if (tOffset>=tEnd) { @@ -255,6 +318,22 @@ public class DocumentEdits implements ProposalApplier { } return null; } + + public TextReplace asReplacement(IDocument doc) throws BadLocationException { + if (!edits.isEmpty()) { + int start = edits.stream().mapToInt(Edit::getStart).min().getAsInt(); + int end = edits.stream().mapToInt(Edit::getEnd).max().getAsInt(); + + DocumentState state = new DocumentState(doc); + for (Edit edit : edits) { + edit.apply(state); + } + int newStart = state.org2new.transform(start, Direction.BEFORE); + int newEnd = state.org2new.transform(end, Direction.AFTER); + return new TextReplace(start, end, state.doc.textBetween(newStart, newEnd)); + } + return null; + } @Override public void apply(IDocument _doc) throws Exception { diff --git a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/BadLocationException.java b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/BadLocationException.java index ef8ad8ff1..f38145483 100644 --- a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/BadLocationException.java +++ b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/BadLocationException.java @@ -5,6 +5,8 @@ package org.springframework.ide.vscode.util; */ public class BadLocationException extends Exception { + private static final long serialVersionUID = 1L; + public BadLocationException(Throwable e) { super(e); } diff --git a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/IDocument.java b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/IDocument.java index 5addca46c..0d9367a67 100644 --- a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/IDocument.java +++ b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/IDocument.java @@ -13,5 +13,6 @@ public interface IDocument { IRegion getLineInformation(int line); int getLineOffset(int line); void replace(int start, int len, String text); + String textBetween(int start, int end) throws BadLocationException; } diff --git a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/TextDocument.java b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/TextDocument.java index 5bcbe440f..f1c8b345f 100644 --- a/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/TextDocument.java +++ b/vscode-extensions/commons/language-server-commons/src/main/java/org/springframework/ide/vscode/util/TextDocument.java @@ -10,7 +10,6 @@ import io.typefox.lsapi.Range; import io.typefox.lsapi.TextDocumentContentChangeEvent; import io.typefox.lsapi.impl.PositionImpl; import io.typefox.lsapi.impl.RangeImpl; -import io.typefox.lsapi.impl.TextDocumentItemImpl; public class TextDocument implements IDocument { @@ -239,4 +238,9 @@ public class TextDocument implements IDocument { return new TextDocument(this); } + @Override + public String textBetween(int start, int end) throws BadLocationException { + return get(start, end-start); + } + } diff --git a/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/testharness/Editor.java b/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/testharness/Editor.java index 32df0c560..de5835953 100644 --- a/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/testharness/Editor.java +++ b/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/testharness/Editor.java @@ -10,6 +10,8 @@ import java.util.List; import javax.swing.text.BadLocationException; +import com.google.common.base.Strings; + import io.typefox.lsapi.CompletionItem; import io.typefox.lsapi.CompletionList; import io.typefox.lsapi.Diagnostic; @@ -212,12 +214,18 @@ public class Editor { String docText = document.getText(); if (edit!=null) { String replaceWith = edit.getNewText(); + //Apply indentfix, this is magic vscode seems to apply to edits returned by language server. So our harness has to + // mimick that behavior. I'm not sure this fix is really emulating it faithfully as its undocumented :-( + int indentFix = edit.getRange().getStart().getCharacter(); + replaceWith = replaceWith.replaceAll("\\n", "\n"+Strings.repeat(" ", indentFix)); + int cursorReplaceOffset = replaceWith.indexOf(VS_CODE_CURSOR_MARKER); if (cursorReplaceOffset>=0) { replaceWith = replaceWith.substring(0, cursorReplaceOffset) + replaceWith.substring(cursorReplaceOffset+VS_CODE_CURSOR_MARKER.length()); } else { cursorReplaceOffset = replaceWith.length(); } + Range rng = edit.getRange(); int start = document.toOffset(rng.getStart()); int end = document.toOffset(rng.getEnd()); diff --git a/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YamlPathEdits.java b/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YamlPathEdits.java index bf18093f1..7ce662aa5 100644 --- a/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YamlPathEdits.java +++ b/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YamlPathEdits.java @@ -94,6 +94,17 @@ public class YamlPathEdits extends DocumentEdits { insert(insertionPoint, createPathInsertionText(path, indent, startOnNewLine, appendText)); } + /** + * Yuck! This component behaves a little differently when working in service of vscode. This is because + * when vscode applies completions it already does some magic indentation fixing (which is not really + * documented see: https://github.com/Microsoft/language-server-protocol/issues/83 + *

+ * We have to counteract the magic fixing of indentation by avoiding to do these fixings ourself. Discovering + * which things we have to counteract is trial and error and probably specific to vscode's implementation + * of LSP support only. + */ + private boolean vsCode = true; + protected String createPathInsertionText(YamlPath path, int indent, boolean startOnNewLine, String appendText) { StringBuilder buf = new StringBuilder(); for (int i = 0; i < path.size(); i++) { @@ -105,7 +116,11 @@ public class YamlPathEdits extends DocumentEdits { buf.append(":"); indent += YamlIndentUtil.INDENT_BY; } - buf.append(indentUtil.applyIndentation(appendText, indent)); + if (vsCode) { + buf.append(indentUtil.applyIndentation(appendText, YamlIndentUtil.INDENT_BY)); + } else { + buf.append(indentUtil.applyIndentation(appendText, indent)); + } return buf.toString(); } diff --git a/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/util/YamlIndentUtil.java b/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/util/YamlIndentUtil.java index ff86a4a52..81cbb9c89 100644 --- a/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/util/YamlIndentUtil.java +++ b/vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/util/YamlIndentUtil.java @@ -31,7 +31,7 @@ public class YamlIndentUtil { * for the current document). */ public final String NEWLINE; - + public YamlIndentUtil(String newline) { this.NEWLINE = newline; Assert.isNotNull(NEWLINE); diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/ManifestYamlLanguageServer.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/ManifestYamlLanguageServer.java index 73ec8bfd8..e443cebfe 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/ManifestYamlLanguageServer.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/ManifestYamlLanguageServer.java @@ -110,7 +110,7 @@ public class ManifestYamlLanguageServer extends SimpleLanguageServer { c.setTextDocumentSync(TextDocumentSyncKind.Full); CompletionOptionsImpl completionProvider = new CompletionOptionsImpl(); - completionProvider.setResolveProvider(true); + completionProvider.setResolveProvider(false); c.setCompletionProvider(completionProvider); return c; diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/VscodeCompletionEngineAdapter.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/VscodeCompletionEngineAdapter.java index c4cd3c162..7a4a2a4d9 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/VscodeCompletionEngineAdapter.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/VscodeCompletionEngineAdapter.java @@ -1,7 +1,6 @@ package org.springframework.ide.vscode.cloudfoundry.manifest.editor; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -9,10 +8,10 @@ import java.util.concurrent.CompletableFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.ide.vscode.commons.completion.DocumentEdits; +import org.springframework.ide.vscode.commons.completion.DocumentEdits.TextReplace; import org.springframework.ide.vscode.commons.completion.ICompletionEngine; import org.springframework.ide.vscode.commons.completion.ICompletionProposal; import org.springframework.ide.vscode.util.Futures; -import org.springframework.ide.vscode.util.IRegion; import org.springframework.ide.vscode.util.SimpleLanguageServer; import org.springframework.ide.vscode.util.SimpleTextDocumentService; import org.springframework.ide.vscode.util.TextDocument; @@ -79,26 +78,24 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine { item.setLabel(completion.getLabel()); item.setKind(completion.getKind()); item.setSortText(sortkeys.next()); + item.setFilterText(completion.getLabel()); adaptEdits(item, doc, completion.getTextEdit()); return item; } private void adaptEdits(CompletionItemImpl item, TextDocument doc, DocumentEdits edits) throws Exception { - TextDocument newDoc = doc.copy(); - edits.apply(newDoc); - - IRegion newSelection = edits.getSelection(doc); - if (newSelection==null) { - //Every 'real' edit moves the cursor. So if the newSelection is unknown it can only - //mean we are dealing with a 'null' edit. + TextReplace replaceEdit = edits.asReplacement(doc); + if (replaceEdit==null) { + //The original edit does nothing. item.setInsertText(""); } else { - TextEditImpl fullEdit = new TextEditImpl(); - fullEdit.setRange(doc.toRange(0, doc.getLength())); - int newCursor = newSelection.getOffset(); - String newText = newDoc.getText(); - fullEdit.setNewText(newText.substring(0,newCursor)+VS_CODE_CURSOR_MARKER+newText.substring(newCursor)); - item.setTextEdit(fullEdit); + TextDocument newDoc = doc.copy(); + edits.apply(newDoc); + TextEditImpl vscodeEdit = new TextEditImpl(); + vscodeEdit.setRange(newDoc.toRange(replaceEdit.start, replaceEdit.end-replaceEdit.start)); + vscodeEdit.setNewText(replaceEdit.newText); + //TODO: cursor offset within newText? for now we assume its always at the end. + item.setTextEdit(vscodeEdit); } } diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/yaml/ManifestYamlLanguageServerTest.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/yaml/ManifestYamlLanguageServerTest.java index de65c3a94..89aef07ba 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/yaml/ManifestYamlLanguageServerTest.java +++ b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/yaml/ManifestYamlLanguageServerTest.java @@ -5,15 +5,11 @@ import static org.assertj.core.api.Assertions.assertThat; import java.io.File; import java.net.URISyntaxException; import java.nio.file.Paths; -import java.util.List; import org.junit.Test; import org.springframework.ide.vscode.cloudfoundry.manifest.editor.ManifestYamlLanguageServer; import org.springframework.ide.vscode.testharness.LanguageServerHarness; -import org.springframework.ide.vscode.testharness.TextDocumentInfo; -import io.typefox.lsapi.CompletionItem; -import io.typefox.lsapi.CompletionList; import io.typefox.lsapi.InitializeResult; import io.typefox.lsapi.TextDocumentSyncKind; @@ -66,7 +62,7 @@ public class ManifestYamlLanguageServerTest { // } private void assertExpectedInitResult(InitializeResult initResult) { - assertThat(initResult.getCapabilities().getCompletionProvider().getResolveProvider()).isTrue(); + assertThat(initResult.getCapabilities().getCompletionProvider().getResolveProvider()).isFalse(); assertThat(initResult.getCapabilities().getTextDocumentSync()).isEqualTo(TextDocumentSyncKind.Full); }