From d9f48ed693dbbf9f14df8be69933840833966cb0 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Thu, 25 Jul 2019 16:46:00 -0700 Subject: [PATCH] Fix for PT-167494938 Snippet completion was broken for concourse resource type 'extra insertions'. --- .../completion/DocumentEdits.java | 17 ++++++++++++++--- .../util/PlaceHolderString.java | 12 ++++++++++++ .../yaml/completion/YTypeAssistContext.java | 10 ++++++++-- .../commons/yaml/schema/BasicYValueHint.java | 9 +++++---- .../commons/yaml/schema/YValueHint.java | 3 ++- .../languageserver/testharness/Editor.java | 3 ++- .../ide/vscode/concourse/ConcourseModel.java | 2 +- .../vscode/concourse/ConcourseEditorTest.java | 19 +++++++++++++++++++ 8 files changed, 63 insertions(+), 12 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/DocumentEdits.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/DocumentEdits.java index 181d5db2d..aae74ee3b 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/DocumentEdits.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/DocumentEdits.java @@ -17,6 +17,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.eclipse.lsp4j.TextEdit; +import org.springframework.ide.vscode.commons.languageserver.util.PlaceHolderString; import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.util.BadLocationException; import org.springframework.ide.vscode.commons.util.text.IDocument; @@ -322,7 +323,7 @@ public class DocumentEdits implements ProposalApplier { private List edits = new ArrayList(); private IDocument doc; - final private boolean hasSnippets; + private boolean hasSnippets; /** * When this is true, the cursor is moved after each edit, to be positioned right after the @@ -337,7 +338,7 @@ public class DocumentEdits implements ProposalApplier { this.doc = doc; this.hasSnippets = hasSnippets; } - + public void delete(int start, int end) { Assert.isLegal(start<=end); edits.add(new Deletion(grabCursor, start, end)); @@ -347,6 +348,17 @@ public class DocumentEdits implements ProposalApplier { delete(offset, offset+text.length()); } + public void insertSnippet(int offset, String snippet) { + //The way we track/handle snippet usage is not totally correct. + //There is a bug here that if we compose multiple edits, some of which + //use snippet placeholders and others which don't, all will be considered + //as using snippets. This may pose problems if somehow literal text that + //looks like a placeholder is combined with real snippet. Then all will + //be treated as a snippet. + hasSnippets |= true; + edits.add(new Insertion(grabCursor, offset, snippet.toString())); + } + public void insert(int offset, String insert) { edits.add(new Insertion(grabCursor, offset, insert)); } @@ -499,5 +511,4 @@ public class DocumentEdits implements ProposalApplier { final public boolean hasSnippets() { return hasSnippets; } - } diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/PlaceHolderString.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/PlaceHolderString.java index 97bc928bb..c169c1934 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/PlaceHolderString.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/PlaceHolderString.java @@ -77,6 +77,18 @@ public class PlaceHolderString { this.string = string; } + public boolean hasPlaceHolders() { + if (placeHolders.isEmpty()) { + return false; + } + if (placeHolders.size()==1) { + PlaceHolder placeHolder = CollectionUtil.getAny(placeHolders.values()); + if (string.length()==placeHolder.getEnd()) { + return false; + } + } + return true; + } @Override public String toString() { diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java index 54f62d90e..3eaede5c9 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java @@ -26,6 +26,7 @@ import org.slf4j.LoggerFactory; import org.springframework.ide.vscode.commons.languageserver.completion.DocumentEdits; import org.springframework.ide.vscode.commons.languageserver.completion.ICompletionProposal; import org.springframework.ide.vscode.commons.languageserver.completion.ScoreableProposal; +import org.springframework.ide.vscode.commons.languageserver.util.PlaceHolderString; import org.springframework.ide.vscode.commons.util.CollectionUtil; import org.springframework.ide.vscode.commons.util.ExceptionUtil; import org.springframework.ide.vscode.commons.util.FuzzyMatcher; @@ -279,9 +280,14 @@ public class YTypeAssistContext extends AbstractYamlAssistContext { edits.insert(offset, " "); } edits.insert(offset, value.getValue()); - String extraInsertion = value.getExtraInsertion(); + PlaceHolderString extraInsertion = value.getExtraInsertion(); if (extraInsertion!=null) { - edits.insert(offset, indenter.applyIndentation(extraInsertion, referenceIndent)); + String insertText = indenter.applyIndentation(extraInsertion.toString(), referenceIndent); + if (extraInsertion.hasPlaceHolders()) { + edits.insertSnippet(offset, insertText); + } else { + edits.insert(offset, insertText); + } } completions.add(completionFactory().valueProposal( value.getValue(), query, value.getLabel(), type, diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/BasicYValueHint.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/BasicYValueHint.java index ee705e125..ac1411d96 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/BasicYValueHint.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/BasicYValueHint.java @@ -10,6 +10,7 @@ *******************************************************************************/ package org.springframework.ide.vscode.commons.yaml.schema; +import org.springframework.ide.vscode.commons.languageserver.util.PlaceHolderString; import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.util.Renderable; @@ -19,7 +20,7 @@ public class BasicYValueHint implements YValueHint { private final String value; private String label; - private Supplier extraInsertion = null; + private Supplier extraInsertion = null; private Renderable documentation; public BasicYValueHint(String value, String label) { @@ -85,12 +86,12 @@ public class BasicYValueHint implements YValueHint { } @Override - public String getExtraInsertion() { - Supplier ei = this.extraInsertion; + public PlaceHolderString getExtraInsertion() { + Supplier ei = this.extraInsertion; return ei==null ? null : ei.get(); } - public BasicYValueHint setExtraInsertion(Supplier insertions) { + public BasicYValueHint setExtraInsertion(Supplier insertions) { Assert.isLegal(this.extraInsertion==null); this.extraInsertion = insertions; return this; diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YValueHint.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YValueHint.java index c1c0abab5..bf6865d16 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YValueHint.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YValueHint.java @@ -10,6 +10,7 @@ *******************************************************************************/ package org.springframework.ide.vscode.commons.yaml.schema; +import org.springframework.ide.vscode.commons.languageserver.util.PlaceHolderString; import org.springframework.ide.vscode.commons.util.Renderable; public interface YValueHint { @@ -22,7 +23,7 @@ public interface YValueHint { * on the next line and indented to line-up relative to the indentation of * the line where the value itself is being inserted. */ - String getExtraInsertion(); + PlaceHolderString getExtraInsertion(); /** * An optional documentation string (i.e. shown in javadoc side hover in Eclipse style diff --git a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java index e35f435fc..899f9c51d 100644 --- a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java +++ b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java @@ -717,12 +717,13 @@ public class Editor { setText(saveText); } - public void assertCompletionWithLabel(Predicate expectLabel, String expectedResult) throws Exception { + public CompletionItem assertCompletionWithLabel(Predicate expectLabel, String expectedResult) throws Exception { CompletionItem completion = assertCompletionWithLabel(expectLabel); String saveText = getText(); apply(completion); assertEquals(expectedResult, getText()); setText(saveText); + return completion; } public void setSelection(int start, int end) { diff --git a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java index 87fac4100..c3c187b1f 100644 --- a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java +++ b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java @@ -424,7 +424,7 @@ public class ConcourseModel { snippet.text("\n "+p.getName()+": "); snippet.placeHolder(); } - return snippet.toString(); + return snippet.build(); } } return null; diff --git a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java index 06ca50b37..272f02df6 100644 --- a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java +++ b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java @@ -27,6 +27,7 @@ import java.util.stream.Collectors; import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.DiagnosticSeverity; +import org.eclipse.lsp4j.InsertTextFormat; import org.junit.Before; import org.junit.Ignore; import org.junit.Rule; @@ -4382,6 +4383,24 @@ public class ConcourseEditorTest { ); editor.assertProblems(/*NONE*/); } + + @Test public void cfResourceTypeCompletion() throws Exception { + Editor editor = harness.newEditor( + "resources:\n" + + "- name: foo\n" + + " type: <*>" + ); + CompletionItem item = editor.assertCompletionWithLabel(label -> label.startsWith("cf"), + "resources:\n" + + "- name: foo\n" + + " type: cf\n" + + " source:\n" + + " api: $1\n" + + " organization: $2\n" + + " space: $3<*>" + ); + assertEquals(InsertTextFormat.Snippet, item.getInsertTextFormat()); + } @Test public void cfResourceSourceCompletions() throws Exception { Editor editor = harness.newEditor(