Merge pull request #156 from NikolasKomonen/formatErrorFixes

Formatted node content doesnt get touched
eclipse-lemminx · Oct 9, 2018 · 6bd29e7 · 6bd29e7
2 parents 68c9d4e + e4803a8
commit 6bd29e7
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 15 deletions.
diff --git a/extensions/deleteThis.xml b/extensions/deleteThis.xml
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/CharacterData.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/CharacterData.java
@@ -45,6 +45,27 @@ public boolean hasMultiLine() {
 		}
 	}
 
+	/**
+	 * If data ends with a new line character
+	 * 
+	 * Returns false if a character is found before a new line, 
+	 * but will ignore whitespace while searching
+	 * @return
+	 */
+	public boolean endsWithNewLine() {
+		for(int i = data.length() - 1; i >= 0; i--) {
+			char c = data.charAt(i);
+			if(!Character.isWhitespace(c)) {
+				return false;
+			}
+			if(c == '\n') {
+				return true;
+			}
+
+		}
+		return false;
+	}
+
 	public String getNormalizedData() {
 		if (normalizedData == null) {
 			normalizedData = StringUtils.normalizeSpace(getData());

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/Node.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/Node.java
@@ -361,6 +361,20 @@ public Element getParentElement() {
 		return null;
 	}
 
+	/**
+	 * Checks if previous sibling node is of 'type'
+	 * @param type
+	 * @return
+	 */
+	public boolean isPreviousNodeType(Short type) {
+		int currentIndex = this.getParent().getChildren().indexOf(this);
+		return currentIndex > 0 ? this.getParent().getChild(currentIndex - 1).isText() : false;
+	}
+
+	public boolean isFirstChildNode() {
+		return this.getParent().getChildren().indexOf(this) == 0;
+	}
+
 	public boolean isComment() {
 		return getNodeType() == Node.COMMENT_NODE;
 	}

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLFormatter.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLFormatter.java
@@ -87,7 +87,10 @@ public List<? extends TextEdit> format(TextDocument document, Range range, XMLFo
 
 	private void format(Node node, int level, int end, XMLBuilder xml) {
 		if (node.getNodeType() != Node.DOCUMENT_NODE) {
-			boolean doLineFeed = !(node.isComment() && ((Comment) node).isCommentSameLineEndTag()) && !node.isText();
+			boolean doLineFeed = !(node.isComment() && ((Comment) node).isCommentSameLineEndTag()) 
+									&& (!node.isPreviousNodeType(Node.TEXT_NODE) || xml.isJoinContentLines())
+									&& (!node.isText() || ((xml.isJoinContentLines() && !node.isFirstChildNode())));
+
 			if (level > 0 && doLineFeed) {
 				// add new line + indent
 				xml.linefeed();
@@ -146,13 +149,11 @@ private void format(Node node, int level, int end, XMLBuilder xml) {
 				Text text = (Text) node;
 				if (text.hasData()) {
 					// Generate content
-					if (text.hasMultiLine() && !xml.isJoinContentLines()) {
-						xml.linefeed();
-					}
 					String content = text.getData();
 					if (!content.isEmpty()) {
 						xml.addContent(content);
 					}
+
 				}
 				return;
 			} else if (node.isElement()) {
@@ -180,14 +181,7 @@ private void format(Node node, int level, int end, XMLBuilder xml) {
 						startElementClosed = true;
 						level++;
 						for (Node child : node.getChildren()) {
-							boolean textElement = !child.isText()
-									|| (child.isCharacterData() && ((CharacterData) child).hasMultiLine())
-									|| node.getChildren().size() > 1;
-							if (child.isCharacterData()) {
-								if (xml.isJoinContentLines()) {
-									textElement = false;
-								}
-							}
+							boolean textElement = !child.isText();
 
 							hasElements = hasElements | textElement;
 

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java
@@ -118,7 +118,7 @@ public XMLBuilder addContent(String text) {
 		if (isJoinContentLines()) {
 			normalizeSpace(text, xml);
 		} else {
-			trimNewLines(text, xml);
+			xml.append(text);
 		}
 		return this;
 	}

diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java
@@ -389,10 +389,32 @@ public void testContentFormatting2() throws BadLocationException {
 		String expected = 
 		"<a>\r" +
 		" Content\r" +
-		"  <b>\r" +
+		" <b>\r" +
 		"   Content2\r" +
 		"    Content3\r" +
-		"  </b>\r" +
+		" </b>\r" +
+		"</a>";
+
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		formattingOptions.setJoinContentLines(false);
+		format(content, expected, formattingOptions);
+	}
+
+	@Test
+	public void testContentFormattingDontMoveEndTag() throws BadLocationException {
+		String content = 
+		"<a>\r" +
+		" Content\r" +
+		" <b>\r" +
+		"   Content2\r" +
+		"    Content3 </b>\r" +
+		"</a>";
+		String expected = 
+		"<a>\r" +
+		" Content\r" +
+		" <b>\r" +
+		"   Content2\r" +
+		"    Content3 </b>\r" +
 		"</a>";
 
 		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
@@ -448,6 +470,7 @@ public void testContentFormatting6() throws BadLocationException {
 				" Content\r" + //
 				"</a>";
 		String expected = "<a>\r" + //
+				"\r" + //
 				" Content\r" + //
 				"</a>";
 		format(content, expected, formattingOptions);
@@ -457,6 +480,7 @@ public void testContentFormatting6() throws BadLocationException {
 				" Content\r\n" + //
 				"</a>";
 		expected = "<a>\r\n" + //
+				"\r\n" + //
 				" Content\r\n" + //
 				"</a>";
 		format(content, expected, formattingOptions);