No indent list item completion below map key

vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YTypeAssistContext.java

@@ -35,6 +35,7 @@ import org.springframework.ide.vscode.yaml.structure.YamlDocument;
 import org.springframework.ide.vscode.yaml.structure.YamlStructureParser.SChildBearingNode;
 import org.springframework.ide.vscode.yaml.structure.YamlStructureParser.SKeyNode;
 import org.springframework.ide.vscode.yaml.structure.YamlStructureParser.SNode;
+import org.springframework.ide.vscode.yaml.util.YamlIndentUtil;
 
 public class YTypeAssistContext extends AbstractYamlAssistContext {
 
@@ -116,13 +117,14 @@ public class YTypeAssistContext extends AbstractYamlAssistContext {
 	 */
 	protected String appendTextFor(YType type) {
 		//Note that proper indentation after each \n" is added automatically
-		//so the strings created here do not need to contain indentation spaces.
+		//to align with the parent. The strings created here only need to contain
+		//indentation spaces to indent *more* than the parent node.
 		if (type==null) {
 			//Assume its some kind of pojo bean
-			return "\n";
+			return "\n"+YamlIndentUtil.INDENT_STR;
 		} else if (typeUtil.isMap(type)) {
 			//ready to enter nested map key on next line
-			return "\n";
+			return "\n"+YamlIndentUtil.INDENT_STR;
 		} if (typeUtil.isSequencable(type)) {
 			//ready to enter sequence element on next line
 			return "\n- ";
@@ -131,7 +133,7 @@ public class YTypeAssistContext extends AbstractYamlAssistContext {
 			return " ";
 		} else {
 			//Assume its some kind of pojo bean
-			return "\n";
+			return "\n"+YamlIndentUtil.INDENT_STR;
 		}
 	}
 

vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/completion/YamlPathEdits.java

@@ -94,17 +94,6 @@ 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
-	 * <p>
-	 * 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++) {
@@ -114,11 +103,9 @@ public class YamlPathEdits extends DocumentEdits {
 			String key = path.getSegment(i).toPropString();
 			buf.append(YamlUtil.stringEscape(key));
 			buf.append(":");
-			indent += YamlIndentUtil.INDENT_BY;
-		}
-		if (vsCode) {
-			buf.append(indentUtil.applyIndentation(appendText, YamlIndentUtil.INDENT_BY));
-		} else {
+			if (i<path.size()-1) {
+				indent += YamlIndentUtil.INDENT_BY;
+			}
 			buf.append(indentUtil.applyIndentation(appendText, indent));
 		}
 		return buf.toString();

vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/structure/YamlStructureParser.java

@@ -87,7 +87,7 @@ public class YamlStructureParser {
 	}
 
 	private YamlLineReader input;
-	
+
 	private final KeyAliases keyAliases;
 
 	public static class YamlLine {
@@ -222,6 +222,7 @@ public class YamlStructureParser {
 			return indent;
 		}
 
+		@Override
 		public final String toString() {
 			StringWriter out = new StringWriter();
 			try {
@@ -502,6 +503,7 @@ public class YamlStructureParser {
 			super(parent, doc, indent, start, end);
 		}
 
+		@Override
 		public int getTreeEnd() {
 			return getNodeEnd();
 		}
@@ -683,71 +685,56 @@ public class YamlStructureParser {
 		 * This includes all the text starting from the ':' upto the very end of this node,
 		 * including the text for this node's children (if any).
 		 */
-		public String getValue() {
+		public String getValueWithRelativeIndent() {
 			int start = getColonOffset()+1;
 			int end = getTreeEnd();
 			String indentedText = StringUtil.trimEnd(doc.textBetween(start, end));
-			List<SNode> children = getChildren();
-			int indent = determineIndentation(children);
+			int indent = getIndent();
 			if (indent>0) {
 				return stripIndentation(indent, indentedText);
 			}
 			return indentedText;
 		}
 
-		private String stripIndentation(int indent, String indentedText) {
-			StringBuilder out = new StringBuilder();
-			Pattern NEWLINE = Pattern.compile("(\\n|\\r)+");
-			boolean first = true;
-			Matcher matcher = NEWLINE.matcher(indentedText);
-			int pos = 0;
-			while (matcher.find()) {
-				int newline = matcher.start();
-				int newline_end = matcher.end();
-				String line = indentedText.substring(pos, newline);
-				if (first) {
-					first = false;
-				} else {
-					line = stripIndentationFromLine(indent, line);
-				}
-				out.append(line);
-				out.append(indentedText.substring(newline, newline_end));
-				pos = newline_end;
-			}
-			out.append(stripIndentationFromLine(indent, indentedText.substring(pos)));
-			return out.toString();
-		}
-
-		private String stripIndentationFromLine(int indent, String line) {
-			int start = 0;
-			while (start<line.length() && start < indent && line.charAt(start)==' ') {
-				start++;
-			}
-			return line.substring(start);
-		}
-
-		/**
-		 * Determine the indentation of a block of children.
-		 */
-		private int determineIndentation(List<SNode> children) {
-			//The tricky bit is that the block may start with comment nodes which provide no hints about the indentation
-			//indicated by indentation level = -1
-			//So... we must take indentation from the first node that actually has one
-			if (children!=null) {
-				for (SNode c : children) {
-					int indent = c.getIndent();
-					if (indent>=0) {
-						return indent;
-					}
-				}
-			}
-			return -1; //Couldn't determine it.
-		}
 	}
 
 	private Iterable<String> getKeyAliases(String key) {
 		return keyAliases.getKeyAliases(key);
 	}
 
+	public static String stripIndentation(int indent, String indentedText) {
+		StringBuilder out = new StringBuilder();
+		Pattern NEWLINE = Pattern.compile("(\\n|\\r)+");
+		boolean first = true;
+		Matcher matcher = NEWLINE.matcher(indentedText);
+		int pos = 0;
+		while (matcher.find()) {
+			int newline = matcher.start();
+			int newline_end = matcher.end();
+			String line = indentedText.substring(pos, newline);
+			if (first) {
+				first = false;
+			} else {
+				line = stripIndentationFromLine(indent, line);
+			}
+			out.append(line);
+			out.append(indentedText.substring(newline, newline_end));
+			pos = newline_end;
+		}
+		String line = indentedText.substring(pos);
+		if (!first) {
+			line = stripIndentationFromLine(indent, line);
+		}
+		out.append(line);
+		return out.toString();
+	}
+
+	private static String stripIndentationFromLine(int indent, String line) {
+		int start = 0;
+		while (start<line.length() && start < indent && line.charAt(start)==' ') {
+			start++;
+		}
+		return line.substring(start);
+	}
 
 }

vscode-extensions/commons/yaml-commons/src/main/java/org/springframework/ide/vscode/yaml/util/YamlIndentUtil.java

@@ -13,6 +13,8 @@ package org.springframework.ide.vscode.yaml.util;
 import org.springframework.ide.vscode.util.Assert;
 import org.springframework.ide.vscode.yaml.structure.YamlDocument;
 
+import com.google.common.base.Strings;
+
 /**
  * Helper methods to manipulate indentation levels in yaml content.
  *
@@ -26,6 +28,8 @@ public class YamlIndentUtil {
 	 */
 	public static final int INDENT_BY = 2;
 
+	public static final String INDENT_STR = Strings.repeat(" ", INDENT_BY);
+
 	/**
 	 * Some functions introduce line separators and this may depend on the context (i.e. default line separator
 	 * for the current document).

vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/cloudfoundry/manifest/editor/VscodeCompletionEngineAdapter.java

@@ -16,23 +16,25 @@ import org.springframework.ide.vscode.util.SimpleLanguageServer;
 import org.springframework.ide.vscode.util.SimpleTextDocumentService;
 import org.springframework.ide.vscode.util.TextDocument;
 import org.springframework.ide.vscode.yaml.completion.DefaultCompletionFactory;
+import org.springframework.ide.vscode.yaml.structure.YamlStructureParser;
 
 import io.typefox.lsapi.CompletionItem;
 import io.typefox.lsapi.CompletionList;
 import io.typefox.lsapi.TextDocumentPositionParams;
 import io.typefox.lsapi.impl.CompletionItemImpl;
 import io.typefox.lsapi.impl.CompletionListImpl;
+import io.typefox.lsapi.impl.PositionImpl;
 import io.typefox.lsapi.impl.TextEditImpl;
 
 /**
  * Adapts a {@link ICompletionEngine}, wrapping it, to implement {@link VscodeCompletionEngine}
  */
 public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine {
-	
+
 	final static Logger logger = LoggerFactory.getLogger(VscodeCompletionEngineAdapter.class);
-	
+
 	public static final String VS_CODE_CURSOR_MARKER = "{{}}";
-	
+
 	private SimpleLanguageServer server;
 	private ICompletionEngine engine;
 
@@ -72,7 +74,7 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine {
 		}
 		return SimpleTextDocumentService.NO_COMPLETIONS;
 	}
-	
+
 	private CompletionItemImpl adaptItem(TextDocument doc, ICompletionProposal completion, SortKeys sortkeys) throws Exception {
 		CompletionItemImpl item = new CompletionItemImpl();
 		item.setLabel(completion.getLabel());
@@ -93,12 +95,19 @@ public class VscodeCompletionEngineAdapter implements VscodeCompletionEngine {
 			edits.apply(newDoc);
 			TextEditImpl vscodeEdit = new TextEditImpl();
 			vscodeEdit.setRange(newDoc.toRange(replaceEdit.start, replaceEdit.end-replaceEdit.start));
-			vscodeEdit.setNewText(replaceEdit.newText);
+			vscodeEdit.setNewText(vscodeIndentFix(vscodeEdit.getRange().getStart(), replaceEdit.newText));
 			//TODO: cursor offset within newText? for now we assume its always at the end.
 			item.setTextEdit(vscodeEdit);
 		}
 	}
 
+	private String vscodeIndentFix(PositionImpl start, String newText) {
+		//Vscode applies some magic indent to a multi-line edit text. We do everything ourself so we have adjust for the magic
+		// and do some kind of 'inverse magic' here.
+		int vscodeMagicIndent = start.getCharacter();
+		return YamlStructureParser.stripIndentation(vscodeMagicIndent, newText);
+	}
+
 	@Override
 	public CompletableFuture<CompletionItem> resolveCompletion(CompletionItem unresolved) {
 		//TODO: item is pre-resoved so we don't do anything, but we really should somehow defer some work, such as

vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/yaml/ManifestYamlEditorTest.java

@@ -20,14 +20,14 @@ import org.springframework.ide.vscode.testharness.LanguageServerHarness;
 public class ManifestYamlEditorTest {
 
 	LanguageServerHarness harness;
-	
+
 	@Before public void setup() throws Exception {
 		harness = new LanguageServerHarness(ManifestYamlLanguageServer::new);
 		harness.intialize(null);
 	}
 
 	@Test public void testReconcileCatchesParseError() throws Exception {
-		
+
 		Editor editor = harness.newEditor(
 				"somemap: val\n"+
 				"- sequence"
@@ -36,7 +36,7 @@ public class ManifestYamlEditorTest {
 				"-|expected <block end>"
 		);
 	}
-	
+
 	@Test public void reconcileRunsOnDocumentOpenAndChange() throws Exception {
 		LanguageServerHarness harness = new LanguageServerHarness(ManifestYamlLanguageServer::new);
 		harness.intialize(null);
@@ -49,7 +49,7 @@ public class ManifestYamlEditorTest {
 		editor.assertProblems(
 				"-|expected <block end>"
 		);
-		
+
 		editor.setText(
 				"- sequence\n" +
 				"zomemap: val"
@@ -202,13 +202,23 @@ public class ManifestYamlEditorTest {
 		editor.assertProblems(/*none*/);
 	}
 
+	@Test
+	public void noListIndent() throws Exception {
+		Editor editor;
+		editor = harness.newEditor("appl<*>");
+		editor.assertCompletions(
+				"applications:\n"+
+				"- <*>"
+		);
+	}
+
 	@Test
 	public void toplevelCompletions() throws Exception {
 		Editor editor;
 		editor = harness.newEditor("<*>");
 		editor.assertCompletions(
 				"applications:\n"+
-				"  - <*>",
+				"- <*>",
 				// ---------------
 				"buildpack: <*>",
 				// ---------------
@@ -219,7 +229,7 @@ public class ManifestYamlEditorTest {
 				"domain: <*>",
 				// ---------------
 				"domains:\n"+
-				"  - <*>",
+				"- <*>",
 				// ---------------
 				"env:\n"+
 				"  <*>",
@@ -246,7 +256,7 @@ public class ManifestYamlEditorTest {
 				"random-route: <*>",
 				// ---------------
 				"services:\n"+
-				"  - <*>",
+				"- <*>",
 				// ---------------
 				"stack: <*>",
 				// ---------------
@@ -264,67 +274,67 @@ public class ManifestYamlEditorTest {
 		Editor editor;
 		editor = harness.newEditor(
 				"applications:\n" +
-				"  - <*>"
+				"- <*>"
 		);
 		editor.assertCompletions(
 				// ---------------
 				"applications:\n" +
-				"  - buildpack: <*>",
+				"- buildpack: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - command: <*>",
+				"- command: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - disk_quota: <*>",
+				"- disk_quota: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - domain: <*>",
+				"- domain: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - domains:\n"+
-				"      - <*>",
+				"- domains:\n"+
+				"  - <*>",
 				// ---------------
 				"applications:\n" +
-				"  - env:\n"+
-				"      <*>",
+				"- env:\n"+
+				"    <*>",
 				// ---------------
 				"applications:\n" +
-				"  - host: <*>",
+				"- host: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - hosts:\n"+
-				"      - <*>",
+				"- hosts:\n"+
+				"  - <*>",
 				// ---------------
 				"applications:\n" +
-				"  - instances: <*>",
+				"- instances: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - memory: <*>",
+				"- memory: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - name: <*>",
+				"- name: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - no-hostname: <*>",
+				"- no-hostname: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - no-route: <*>",
+				"- no-route: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - path: <*>",
+				"- path: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - random-route: <*>",
+				"- random-route: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - services:\n"+
-				"      - <*>",
+				"- services:\n"+
+				"  - <*>",
 				// ---------------
 				"applications:\n" +
-				"  - stack: <*>",
+				"- stack: <*>",
 				// ---------------
 				"applications:\n" +
-				"  - timeout: <*>"
+				"- timeout: <*>"
 		);
 	}
 
@@ -359,9 +369,9 @@ public class ManifestYamlEditorTest {
 		Editor editor = harness.newEditor(
 				"memory: 1G\n" +
 				"applications:\n" +
-				"  - buildpack: zbuildpack\n" +
-				"    domain: zdomain\n" +
-				"    name: foo"
+				"- buildpack: zbuildpack\n" +
+				"  domain: zdomain\n" +
+				"  name: foo"
 		);
 		editor.assertIsHoverRegion("memory");
 		editor.assertIsHoverRegion("applications");