From cfe489d1c22272f3650b24a6f54b4a5c05583523 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 2 May 2017 11:19:08 -0700 Subject: [PATCH] Refactoring: Dashed-completion relaxation ... Is now less coupled with the completion engine, but handled internally by the YamllAssistContext instead. --- .../completion/ScoreableProposal.java | 86 +++++++-------- .../ide/vscode/commons/util/Unicodes.java | 18 +++ .../yaml/completion/YTypeAssistContext.java | 30 +++-- .../yaml/completion/YamlAssistContext.java | 15 ++- .../yaml/completion/YamlCompletionEngine.java | 13 +-- .../vscode/concourse/ConcourseEditorTest.java | 104 +++++++++++++----- 6 files changed, 173 insertions(+), 93 deletions(-) create mode 100644 headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Unicodes.java diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/ScoreableProposal.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/ScoreableProposal.java index 3fdb5bc4b..f510c4303 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/ScoreableProposal.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/completion/ScoreableProposal.java @@ -17,49 +17,49 @@ import org.springframework.ide.vscode.commons.util.Assert; public abstract class ScoreableProposal implements ICompletionProposal { - public static final double DEEMP_EXISTS = 0.1; - public static final double DEEMP_DEPRECATION = 0.2; - public static final double DEEMP_DASH_PROPOSAL = 0.5; - public static final double DEEMP_INDENTED_PROPOSAL = 1.0; + public static final double DEEMP_EXISTS = 0.1; + public static final double DEEMP_DEPRECATION = 0.2; + public static final double DEEMP_DASH_PROPOSAL = 0.5; + public static final double DEEMP_INDENTED_PROPOSAL = 1.0; + public static final double DEEMP_DEDENTED_PROPOSAL = 1.5; + private static final double DEEMP_VALUE = 10_000; // should be large enough to move deemphasized stuff to bottom of list. - private static final double DEEMP_VALUE = 10_000; // should be large enough to move deemphasized stuff to bottom of list. + private double deemphasizedBy = 0.0; - private double deemphasizedBy = 0.0; - - /** - * A sorter suitable for sorting ScoreableProposals based on their score. - */ - public static final Comparator COMPARATOR = new Comparator() { - @Override - public int compare(ICompletionProposal p1, ICompletionProposal p2) { - if (p1 instanceof ScoreableProposal && p2 instanceof ScoreableProposal) { - double s1 = ((ScoreableProposal)p1).getScore(); - double s2 = ((ScoreableProposal)p2).getScore(); - if (Math.abs(s1-s2)<1E-5) { - String name1 = ((ScoreableProposal)p1).getLabel(); - String name2 = ((ScoreableProposal)p2).getLabel(); - return name1.compareTo(name2); - } else { - return Double.compare(s2, s1); - } - } - return 0; - } - }; - public abstract double getBaseScore(); - public final double getScore() { - return getBaseScore() - deemphasizedBy; - } + /** + * A sorter suitable for sorting ScoreableProposals based on their score. + */ + public static final Comparator COMPARATOR = new Comparator() { @Override - public ScoreableProposal deemphasize(double howmuch) { - Assert.isLegal(howmuch>0.0); - deemphasizedBy+= howmuch*DEEMP_VALUE; - return this; - } - public boolean isDeemphasized() { - return deemphasizedBy > 0; + public int compare(ICompletionProposal p1, ICompletionProposal p2) { + if (p1 instanceof ScoreableProposal && p2 instanceof ScoreableProposal) { + double s1 = ((ScoreableProposal)p1).getScore(); + double s2 = ((ScoreableProposal)p2).getScore(); + if (Math.abs(s1-s2)<1E-5) { + String name1 = ((ScoreableProposal)p1).getLabel(); + String name2 = ((ScoreableProposal)p2).getLabel(); + return name1.compareTo(name2); + } else { + return Double.compare(s2, s1); + } + } + return 0; } + }; + public abstract double getBaseScore(); + public final double getScore() { + return getBaseScore() - deemphasizedBy; + } + @Override + public ScoreableProposal deemphasize(double howmuch) { + Assert.isLegal(howmuch>0.0); + deemphasizedBy+= howmuch*DEEMP_VALUE; + return this; + } + public boolean isDeemphasized() { + return deemphasizedBy > 0; + } // @Override // public boolean isAutoInsertable() { @@ -125,9 +125,9 @@ public abstract class ScoreableProposal implements ICompletionProposal { // return completionOffset; // } - @Override - public String toString() { - return getLabel(); - } + @Override + public String toString() { + return getLabel(); + } - } \ No newline at end of file +} \ No newline at end of file diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Unicodes.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Unicodes.java new file mode 100644 index 000000000..e4b478319 --- /dev/null +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/Unicodes.java @@ -0,0 +1,18 @@ +/******************************************************************************* + * Copyright (c) 2017 Pivotal, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.util; + +public class Unicodes { + + public static final char RIGHT_ARROW = '→'; + public static final char LEFT_ARROW = '←'; + +} 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 4a28c7ef9..743016fdc 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 @@ -93,11 +93,15 @@ public class YTypeAssistContext extends AbstractYamlAssistContext { } } String query = getPrefix(doc, node, offset); - List valueCompletions = getValueCompletions(doc, node, offset, query); - if (!valueCompletions.isEmpty()) { - return valueCompletions; + List completions = getValueCompletions(doc, node, offset, query); + if (completions.isEmpty()) { + completions = getKeyCompletions(doc, offset, query); } - return getKeyCompletions(doc, offset, query); + if (typeUtil.isSequencable(type)) { + completions = new ArrayList<>(completions); + completions.addAll(getDashedCompletions(doc, node, offset)); + } + return completions; } public List getKeyCompletions(YamlDocument doc, int offset, String query) throws Exception { @@ -300,8 +304,7 @@ public class YTypeAssistContext extends AbstractYamlAssistContext { return typeUtil.getPropertiesMap(getType()).get(name); } - @Override - public YamlAssistContext relax() { + protected YamlAssistContext relaxForDashes() { try { if (typeUtil.isSequencable(type)) { YType itemType = typeUtil.getDomainType(type); @@ -318,7 +321,20 @@ public class YTypeAssistContext extends AbstractYamlAssistContext { } catch (Exception e) { Log.log(e); } - return super.relax(); + return null; + } + + @Override + public Collection getDashedCompletions(YamlDocument doc, SNode current, int offset) { + try { + YamlAssistContext relaxed = relaxForDashes(); + if (relaxed!=null) { + return relaxed.getCompletions(doc, current, offset); + } + } catch (Exception e) { + Log.log(e); + } + return ImmutableList.of(); } private Collection addDashes(Collection basicCompletions, YamlDocument doc, SNode node) { diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlAssistContext.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlAssistContext.java index 184101ea0..b571b191d 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlAssistContext.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlAssistContext.java @@ -20,6 +20,8 @@ import org.springframework.ide.vscode.commons.yaml.path.YamlPathSegment; import org.springframework.ide.vscode.commons.yaml.structure.YamlDocument; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SNode; +import com.google.common.collect.ImmutableList; + /** * @author Kris De Volder */ @@ -36,11 +38,12 @@ public interface YamlAssistContext extends YamlNavigable { YamlDocument getDocument(); /** - * Allows a context to implement a 'relaxation' transformation. A relaxed context - * should compute the same proposals as the original context but may also add - * additional proposals. E.g. a {@link YTypeAssistContext} uses this to - * relax contexts for sequence types to include proposals for the elements - * of the sequence. + * The completion engine calls this instead of the more general `getCompletions` when it wants only completions + * that are suitable as list item. I.e. they must start with a '- '. + *

+ * Implementors that don't support '- ' completions can just return an empty list. */ - default YamlAssistContext relax() { return null; } + default Collection getDashedCompletions(YamlDocument doc, SNode current, int offset) { + return ImmutableList.of(); + } } diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java index 3d2b859c6..dfc9c9791 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YamlCompletionEngine.java @@ -25,6 +25,7 @@ import org.springframework.ide.vscode.commons.languageserver.completion.IComplet import org.springframework.ide.vscode.commons.languageserver.completion.ScoreableProposal; import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.util.Log; +import org.springframework.ide.vscode.commons.util.Unicodes; import org.springframework.ide.vscode.commons.util.text.IDocument; import org.springframework.ide.vscode.commons.yaml.path.YamlPath; import org.springframework.ide.vscode.commons.yaml.structure.YamlDocument; @@ -95,7 +96,7 @@ public class YamlCompletionEngine implements ICompletionEngine { public ICompletionProposal indented(ICompletionProposal proposal) { ScoreableProposal transformed = new TransformedCompletion(proposal) { @Override public String tranformLabel(String originalLabel) { - return "➔ " + originalLabel; + return Unicodes.RIGHT_ARROW+" " + originalLabel; } @Override public DocumentEdits transformEdit(DocumentEdits originalEdit) { originalEdit.indentFirstEdit(YamlIndentUtil.INDENT_STR); @@ -168,13 +169,11 @@ public class YamlCompletionEngine implements ICompletionEngine { } if (context!=null) { Collection all = new ArrayList<>(); - if (!onlyDashes) { + if (onlyDashes) { + all.addAll(context.getDashedCompletions(doc, current, offset)); + } else { all.addAll(context.getCompletions(doc, current, offset)); } - YamlAssistContext relaxedContext = context.relax(); - if (relaxedContext!=null) { - all.addAll(relaxedContext.getCompletions(doc, current, offset)); - } return all; } } @@ -220,7 +219,7 @@ public class YamlCompletionEngine implements ICompletionEngine { } } else if (node.getNodeType()==SNodeType.RAW) { if (adjustIndentStr.startsWith("- ")) { - // We are trying to determine context node for a completion that starts witjh a '- '. + // We are trying to determine context node for a completion that starts with a '- '. // Yaml indentation rules means we have to treat this differently because '-' doesn't // have to be indented to be considered as nested under a key node! int cursorIndent = doc.getColumn(offset); 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 5792ca42c..90e3601b9 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 @@ -2878,20 +2878,20 @@ public class ConcourseEditorTest { "source", "type", //For the nested context: - "➔ branch", - "➔ commit_verification_key_ids", - "➔ commit_verification_keys", - "➔ disable_ci_skip", - "➔ git_config", - "➔ gpg_keyserver", - "➔ ignore_paths", - "➔ password", - "➔ paths", - "➔ private_key", - "➔ skip_ssl_verification", - "➔ tag_filter", - "➔ uri", - "➔ username" + "→ branch", + "→ commit_verification_key_ids", + "→ commit_verification_keys", + "→ disable_ci_skip", + "→ git_config", + "→ gpg_keyserver", + "→ ignore_paths", + "→ password", + "→ paths", + "→ private_key", + "→ skip_ssl_verification", + "→ tag_filter", + "→ uri", + "→ username" ); editor.assertCompletionWithLabel("check_every", @@ -2902,14 +2902,14 @@ public class ConcourseEditorTest { " check_every: <*>" ); - editor.assertCompletionWithLabel("➔ branch", + editor.assertCompletionWithLabel("→ branch", "resources:\n" + "- name: foo\n" + " type: git\n" + " source:\n" + " branch: <*>" ); - editor.assertCompletionWithLabel("➔ commit_verification_key_ids", + editor.assertCompletionWithLabel("→ commit_verification_key_ids", "resources:\n" + "- name: foo\n" + " type: git\n" + @@ -2976,20 +2976,20 @@ public class ConcourseEditorTest { "- task", "- try", //Completions for nested context (i.e. task step) - "➔ attempts", - "➔ config", - "➔ ensure", - "➔ file", - "➔ image", - "➔ input_mapping", - "➔ on_failure", - "➔ on_success", - "➔ output_mapping", - "➔ params", - "➔ privileged", - "➔ tags", - "➔ timeout", - "➔ task" + "→ attempts", + "→ config", + "→ ensure", + "→ file", + "→ image", + "→ input_mapping", + "→ on_failure", + "→ on_success", + "→ output_mapping", + "→ params", + "→ privileged", + "→ tags", + "→ timeout", + "→ task" ); } @@ -3374,6 +3374,50 @@ public class ConcourseEditorTest { " - <*>" ); } + + @Ignore // Doesn't work yet. Enable once implemented. + @Test public void relaxedContentAssistLessSpaces() throws Exception { + Editor editor; + + editor = harness.newEditor( + "jobs:\n" + + "- name: build-docker-image\n" + + " serial: true\n" + + " plan:\n" + + " - get: docker-git\n" + + " trigger: true\n" + + " <*>" + ); + editor.assertCompletionWithLabel("← - put", + "jobs:\n" + + "- name: build-docker-image\n" + + " serial: true\n" + + " plan:\n" + + " - get: docker-git\n" + + " trigger: true\n" + + " - put: <*>" + ); + + editor = harness.newEditor( + "jobs:\n" + + "- name: build-docker-image\n" + + " serial: true\n" + + " plan:\n" + + " - get: docker-git\n" + + " trigger: true\n" + + " pu<*>" + ); + editor.assertCompletionWithLabel("← - put", + "jobs:\n" + + "- name: build-docker-image\n" + + " serial: true\n" + + " plan:\n" + + " - get: docker-git\n" + + " trigger: true\n" + + " - put: <*>" + ); + + } //////////////////////////////////////////////////////////////////////////////