Improve text formatting for experimental formatter

Signed-off-by: Jessica He <[email protected]>

org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java

@@ -13,6 +13,7 @@
 
 import java.util.List;
 
+import org.eclipse.lemminx.dom.DOMElement;
 import org.eclipse.lemminx.dom.DOMText;
 import org.eclipse.lsp4j.TextEdit;
 
@@ -38,10 +39,13 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 		FormatElementCategory formatElementCategory = parentConstraints.getFormatElementCategory();
 		String text = formatterDocument.getText();
 		int availableLineWidth = parentConstraints.getAvailableLineWidth();
+		int indentLevel = parentConstraints.getIndentLevel();
+		int maxLineWidth = getMaxLineWidth();
 
 		int spaceStart = -1;
 		int spaceEnd = -1;
 		boolean containsNewLine = false;
+		boolean isEmpty = true;
 
 		for (int i = textNode.getStart(); i < textNode.getEnd(); i++) {
 			char c = text.charAt(i);
@@ -57,28 +61,40 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 				}
 			} else {
 				// Text content...
+				isEmpty = false;
 				spaceEnd = i;
 				int contentStart = i;
 				while (i + 1 < textNode.getEnd() && !Character.isWhitespace(text.charAt(i + 1))) {
 					i++;
 				}
-				int contentEnd = i;
-				availableLineWidth -= (contentEnd + 1 - contentStart);
-
-				if (formatElementCategory != FormatElementCategory.PreserveSpace
-						&& formatElementCategory != FormatElementCategory.IgnoreSpace) {
+				int contentEnd = i + 1;
+				availableLineWidth -= contentEnd - contentStart;
+				if (formatElementCategory != FormatElementCategory.PreserveSpace) {
+					if (textNode.getStart() != contentStart && (isJoinContentLines() || !containsNewLine)) {
+						// Decrement width for normalized space between text content (not done at
+						// beginning)
+						availableLineWidth--;
+					}
 					if (availableLineWidth < 0) {
 						if (spaceStart != -1) {
-							insertLineBreak(spaceStart, contentStart, edits);
-							availableLineWidth = getMaxLineWidth() - (contentEnd - contentStart + 1);
+							replaceLeftSpacesWithIndentation(indentLevel, spaceStart, contentStart,
+									true, edits);
+							availableLineWidth = maxLineWidth - (contentEnd - contentStart)
+									- indentLevel * getTabSize();
+							containsNewLine = false;
 						}
-					} else if (isJoinContentLines() || (spaceStart == textNode.getStart() || !containsNewLine)) {
+					} else if (isJoinContentLines() || !containsNewLine) {
 						// Case of isJoinContent == true: join all text content with single space
 						// Case of isJoinContent == false: normalize space only between element start
 						// tag and start of text content or doesn't contain a new line
-						replaceSpacesWithOneSpace(spaceStart, spaceEnd-1, edits);
+						replaceSpacesWithOneSpace(spaceStart, spaceEnd - 1, edits);
 						containsNewLine = false;
-						availableLineWidth --;
+					} else if (containsNewLine) {
+						replaceLeftSpacesWithIndentation(indentLevel, spaceStart, spaceEnd,
+								true, edits);
+						containsNewLine = false;
+						availableLineWidth = maxLineWidth - (contentEnd - contentStart)
+								- indentLevel * getTabSize();
 					} else {
 						availableLineWidth -= spaceEnd - spaceStart;
 					}
@@ -89,8 +105,24 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 			}
 		}
 		if (formatElementCategory != FormatElementCategory.PreserveSpace
-				&& formatElementCategory != FormatElementCategory.IgnoreSpace) {
-			replaceSpacesWithOneSpace(spaceStart, spaceEnd, edits);
+				&& formatElementCategory != FormatElementCategory.IgnoreSpace && spaceEnd + 1 != text.length()) {
+			DOMElement parentElement = textNode.getParentElement();
+			// Don't format final spaces if text is at the end of the file
+			if (!containsNewLine || isEmpty && parentElement.getChildNodes().getLength() == 1
+					|| isJoinContentLines()) {
+				// Replace spaces with single space in the case of:
+				// 1. there is no new line
+				// 2. the text content is empty and is the only child in the parent element tag
+				// 3. isJoinContentLines
+				replaceSpacesWithOneSpace(spaceStart, spaceEnd, edits);
+			} else {
+				if (formatElementCategory == FormatElementCategory.NormalizeSpace
+						|| parentElement.getLastChild() == textNode) {
+					// Decrement indent level if is mixed content and text content is the last child
+					indentLevel--;
+				}
+				replaceLeftSpacesWithIndentation(indentLevel, spaceStart, spaceEnd + 1, true, edits);
+			}
 		}
 	}
 
@@ -102,8 +134,8 @@ private int getMaxLineWidth() {
 		return formatterDocument.getMaxLineWidth();
 	}
 
-	private void insertLineBreak(int start, int end, List<TextEdit> edits) {
-		formatterDocument.insertLineBreak(start, end, edits);
+	private int getTabSize() {
+		return formatterDocument.getSharedSettings().getFormattingSettings().getTabSize();
 	}
 
 	private boolean isPreserveEmptyContent() {
@@ -114,6 +146,11 @@ private void replaceSpacesWithOneSpace(int spaceStart, int spaceEnd, List<TextEd
 		formatterDocument.replaceSpacesWithOneSpace(spaceStart, spaceEnd, edits);
 	}
 
+	private int replaceLeftSpacesWithIndentation(int indentLevel, int from, int to, boolean addLineSeparator,
+			List<TextEdit> edits) {
+		return formatterDocument.replaceLeftSpacesWithIndentation(indentLevel, from, to, addLineSeparator, edits);
+	}
+
 	private boolean isJoinContentLines() {
 		return formatterDocument.getSharedSettings().getFormattingSettings().isJoinContentLines();
 	}

org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalIndentTest.java

@@ -147,17 +147,17 @@ public void multipleRootNestedIssue634() throws BadLocationException {
 				"  </child>\r\n" + //
 				"</parent>";
 		String expected = "<parent>\r\n" + //
-				"  <child> test </child>\r\n" + //
+				"  <child>\r\n" + //
+				"    test\r\n" + //
+				"  </child>\r\n" + //
 				"</parent>\r\n" + //
 				"<parent>\r\n" + //
-				"  <child> test </child>\r\n" + //
+				"  <child>\r\n" + //
+				"    test\r\n" + //
+				"  </child>\r\n" + //
 				"</parent>";
 		assertFormat(content, expected, //
-				te(1, 9, 2, 4, " "),
-				te(2, 8, 3, 2, " "),
-				te(4, 9, 4, 9, "\r\n"),
-				te(5, 9, 6, 4, " "),
-				te(6, 8, 7, 2, " "));
+				te(4, 9, 4, 9, "\r\n"));
 		assertFormat(expected, expected);
 	}
 
@@ -212,14 +212,8 @@ public void multipleLineContentIssue600JoinContentLinesFalse() throws BadLocatio
 				"    bar\r\n" + //
 				"  </b>\r\n" + //
 				"</a>";
-		String expected = "<a>\r\n" + //
-				"  <b> foo\r\n" + //
-				"    bar </b>\r\n" + //
-				"</a>";
-		assertFormat(content, expected, settings, //
-				te(1, 5, 2, 4, " "),
-				te(3, 7, 4, 2, " "));
-		assertFormat(expected, expected, settings);
+		String expected = content;
+		assertFormat(content, expected, settings);
 	}
 
 	// From issue: https://github.com/redhat-developer/vscode-xml/issues/600
@@ -286,15 +280,18 @@ public void xsDocumentationTextContentIssue662JoinContentFalse() throws BadLocat
 		String expected = "<xs:schema attributeFormDefault=\"unqualified\" elementFormDefault=\"unqualified\">\r\n" + //
 				"  <xs:complexType name=\"myType\">\r\n" + //
 				"    <xs:annotation>\r\n" + //
-				"      <xs:documentation> Content that spans\r\n" + //
-				"    multiple lines. </xs:documentation>\r\n" + //
+				"      <xs:documentation>\r\n" + //
+				"        Content that spans\r\n" + //
+				"        multiple lines.\r\n" + //
+				"      </xs:documentation>\r\n" + //
 				"    </xs:annotation>\r\n" + //
 				"  </xs:complexType>\r\n" + //
 				"</xs:schema>";
 		assertFormat(content, expected, settings, //
 				te(2, 19, 3, 11, "\r\n      "), //
-				te(3, 29, 4, 4, " "), //
-				te(5, 19, 6, 0, " "), //
+				te(3, 29, 4, 4, "\r\n        "), //
+				te(4, 22, 5, 4, "\r\n        "), //
+				te(5, 19, 6, 0, "\r\n      "), //
 				te(6, 19, 7, 11, "\r\n    "), //
 				te(7, 27, 8, 11, "\r\n  "));
 		assertFormat(expected, expected, settings);

org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java

@@ -89,6 +89,7 @@ public void singleEndTag() throws BadLocationException {
 
 	@Test
 	public void invalidEndTag() throws BadLocationException {
+
 		String content = "</";
 		String expected = content;
 		assertFormat(content, expected);
@@ -696,6 +697,23 @@ public void testCommentMultiLineContent() throws BadLocationException {
 
 	@Test
 	public void testCommentLongWrap() throws BadLocationException {
+		String content = "<a>\n" + //
+				"  Content <!-- comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment -->"
+				+ //
+				"</a>";
+		String expected = "<a>\n" + //
+				"  Content <!-- comment comment comment comment comment comment comment comment\n" + //
+				"  comment comment comment comment comment comment comment comment comment\n" + //
+				"  comment comment comment comment comment comment comment comment --></a>";
+		SharedSettings settings = new SharedSettings();
+		assertFormat(content, expected, settings, //
+				te(1, 78, 1, 79, "\n  "), //
+				te(1, 150, 1, 151, "\n  "));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void testCommentLongWrapJoinContentLines() throws BadLocationException {
 		String content = "<a>\n" + //
 				"  Content <!-- comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment -->"
 				+ //
@@ -704,6 +722,7 @@ public void testCommentLongWrap() throws BadLocationException {
 				"  comment comment comment comment comment comment comment comment comment\n" + //
 				"  comment comment comment comment comment comment comment comment --></a>";
 		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setJoinContentLines(true);
 		assertFormat(content, expected, settings, //
 				te(0, 3, 1, 2, " "), //
 				te(1, 78, 1, 79, "\n  "), //
@@ -713,20 +732,40 @@ public void testCommentLongWrap() throws BadLocationException {
 
 	@Test
 	public void testCommentLongTextContentWrap() throws BadLocationException {
+		String content = "<a> content content content content content content content content content content content content content content content content content content <!-- comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment -->\n"
+				+ //
+				"</a>";
+		String expected = "<a> content content content content content content content content content\n" + //
+				"  content content content content content content content content content <!--\n" + //
+				"  comment comment comment comment comment comment comment comment comment\n" + //
+				"  comment comment comment comment comment comment comment comment comment\n" + //
+				"  comment comment comment comment comment comment comment -->\n" + //
+				"</a>";
+		SharedSettings settings = new SharedSettings();
+		assertFormat(content, expected, settings, //
+				te(0, 75, 0, 76, "\n  "), //
+				te(0, 152, 0, 153, "\n  "),
+				te(0, 224, 0, 225, "\n  "), //
+				te(0, 296, 0, 297, "\n  "));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void testCommentLongTextContentWrapNewLine() throws BadLocationException {
 		String content = "<a>\n" + //
 				"  content content content content content content content content content content content content content content content content content content <!-- comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment comment -->\n"
 				+ //
 				"</a>";
-		String expected = "<a> content content content content content content content content content\n" + //
-				"content content content content content content content content content <!--\n" + //
+		String expected = "<a>\n" + //
+				"  content content content content content content content content content\n" + //
+				"  content content content content content content content content content <!--\n" + //
 				"  comment comment comment comment comment comment comment comment comment\n" + //
 				"  comment comment comment comment comment comment comment comment comment\n" + //
 				"  comment comment comment comment comment comment comment -->\n" + //
 				"</a>";
 		SharedSettings settings = new SharedSettings();
 		assertFormat(content, expected, settings, //
-				te(0, 3, 1, 2, " "), //
-				te(1, 73, 1, 74, "\n"), //
+				te(1, 73, 1, 74, "\n  "), //
 				te(1, 150, 1, 151, "\n  "),
 				te(1, 222, 1, 223, "\n  "), //
 				te(1, 294, 1, 295, "\n  "));
@@ -768,19 +807,22 @@ public void testElementContentNotNormalized() throws BadLocationException {
 				" Content4\r" + //
 				"  Content5\r" + //
 				"</a>";
-		String expected = "<a> Content\r" + //
-				"     Content2\r" + //
-				"      Content3\r" + //
-				" Content4\r" + //
-				"  Content5 </a>";
+		String expected = "<a>\r" + //
+				"  Content\r" + //
+				"  Content2\r" + //
+				"  Content3\r" + //
+				"  Content4\r" + //
+				"  Content5\r" + //
+				"</a>";
 
 		assertFormat(content, expected, //
-				te(0, 3, 1, 1, " "), //
-				te(5, 10, 6, 0, " "));
+				te(0, 3, 1, 1, "\r  "), //
+				te(1, 8, 2, 5, "\r  "), //
+				te(2, 13, 3, 6, "\r  "),
+				te(3, 14, 4, 1, "\r  "));
 		assertFormat(expected, expected);
 	}
 
-	@Disabled
 	@Test
 	public void testContentFormatting2() throws BadLocationException {
 		String content = "<a>\r" + //
@@ -793,31 +835,40 @@ public void testContentFormatting2() throws BadLocationException {
 		String expected = "<a>\r" + //
 				"  Content\r" + //
 				"  <b>\r" + //
-				"   Content2\r" + //
+				"    Content2\r" + //
 				"    Content3\r" + //
-				" </b>\r" + //
+				"  </b>\r" + //
 				"</a>";
 
-		assertFormat(content, expected);
+		assertFormat(content, expected, //
+				te(0, 3, 1, 1, "\r  "), //
+				te(1, 8, 2, 1, "\r  "), //
+				te(2, 4, 3, 3, "\r    "),
+				te(4, 12, 5, 1, "\r  "));
+		assertFormat(expected, expected);
 	}
 
-	@Disabled
 	@Test
 	public void testContentFormattingDontMoveEndTag() throws BadLocationException {
 		String content = "<a>\r" + //
 				" Content\r" + //
 				" <b>\r" + //
 				"   Content2\r" + //
-				"    Content3 </b>\r" + //
+				"    Content3     </b>\r" + //
 				"</a>";
 		String expected = "<a>\r" + //
 				"  Content\r" + //
 				"  <b>\r" + //
-				"   Content2\r" + //
+				"    Content2\r" + //
 				"    Content3 </b>\r" + //
 				"</a>";
 
-		assertFormat(content, expected);
+		assertFormat(content, expected, //
+				te(0, 3, 1, 1, "\r  "), //
+				te(1, 8, 2, 1, "\r  "), //
+				te(2, 4, 3, 3, "\r    "),
+				te(4, 12, 4, 17, " "));
+		assertFormat(expected, expected);
 	}
 
 	@Test
@@ -828,28 +879,29 @@ public void testContentFormatting3() throws BadLocationException {
 		assertFormat(content, expected);
 	}
 
-	@Disabled
 	@Test
 	public void testContentFormatting6() throws BadLocationException {
 		String content = "<a>\r" + //
 				"\r" + //
 				" Content\r" + //
 				"</a>";
 		String expected = "<a>\r" + //
-				"\r" + //
-				" Content\r" + //
+				"  Content\r" + //
 				"</a>";
-		assertFormat(content, expected);
+		assertFormat(content, expected, //
+				te(0, 3, 2, 1, "\r  "));
+		assertFormat(expected, expected);
 
 		content = "<a>\r\n" + //
 				"\r\n" + //
 				" Content\r\n" + //
 				"</a>";
 		expected = "<a>\r\n" + //
-				"\r\n" + //
-				" Content\r\n" + //
+				"  Content\r\n" + //
 				"</a>";
-		assertFormat(content, expected);
+		assertFormat(content, expected, //
+				te(0, 3, 2, 1, "\r\n  "));
+		assertFormat(expected, expected);
 	}
 
 	private static void assertFormat(String unformatted, String actual, TextEdit... expectedEdits)