Improve full support for max line width

Signed-off-by: Jessica He <jhe@redhat.com>
angelozerr · Nov 16, 2022 · f6c8d9a · f6c8d9a
1 parent d505e7e
commit f6c8d9a
Show file tree

Hide file tree

Showing 11 changed files with 81 additions and 66 deletions.
diff --git a/...clipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMElementFormatter.java b/...clipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMElementFormatter.java
@@ -102,8 +102,7 @@ private int formatStartTagElement(DOMElement element, XMLFormattingConstraints p
 			int parentStartCloseOffset = element.getParentElement().getStartTagCloseOffset() + 1;
 			if ((parentStartCloseOffset != startTagOpenOffset
 					&& StringUtils.isWhitespace(formatterDocument.getText(), parentStartCloseOffset,
-							startTagOpenOffset))
-					|| ((parentConstraints.getAvailableLineWidth() - width) < 0 && isMaxLineWidthSupported())) {
+							startTagOpenOffset))) {
 				replaceLeftSpacesWithIndentationPreservedNewLines(parentStartCloseOffset, startTagOpenOffset,
 						indentLevel, edits);
 				parentConstraints.setAvailableLineWidth(getMaxLineWidth());

diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java
@@ -47,7 +47,10 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 		int lineSeparatorOffset = -1;
 		boolean containsNewLine = false;
 
-		for (int i = textNode.getStart(); i < textNode.getEnd(); i++) {
+		int textStart = textNode.getStart();
+		int textEnd = textNode.getEnd();
+
+		for (int i = textStart; i < textEnd; i++) {
 			char c = text.charAt(i);
 			if (Character.isWhitespace(c)) {
 				// Whitespaces...
@@ -66,14 +69,14 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 				// Text content...
 				spaceEnd = i;
 				int contentStart = i;
-				while (i + 1 < textNode.getEnd() && !Character.isWhitespace(text.charAt(i + 1))) {
+				while (i + 1 < textEnd && !Character.isWhitespace(text.charAt(i + 1))) {
 					i++;
 				}
 				int contentEnd = i + 1;
 				if (isMaxLineWidthSupported()) {
 					int maxLineWidth = getMaxLineWidth();
 					availableLineWidth -= contentEnd - contentStart;
-					if (textNode.getStart() != contentStart && availableLineWidth >= 0
+					if (textStart != contentStart && availableLineWidth >= 0
 							&& (isJoinContentLines() || !containsNewLine || isMixedContent)) {
 						// Decrement width for normalized space between text content (not done at
 						// beginning)
@@ -110,14 +113,25 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 		if (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 || isJoinContentLines() || isMixedContent) && (!isMaxLineWidthSupported() || availableLineWidth >= 0)) {
+			if ((!containsNewLine || isJoinContentLines() || isMixedContent)
+					&& (!isMaxLineWidthSupported() || availableLineWidth >= 0)) {
 				// Replace spaces with single space in the case of:
 				// 1. there is no new line
 				// 2. isJoinContentLines
 				replaceSpacesWithOneSpace(spaceStart, spaceEnd, edits);
 				if (isMaxLineWidthSupported() && spaceStart != -1) {
 					availableLineWidth--;
 				}
+			} else if (isMaxLineWidthSupported() && availableLineWidth < 0
+					&& !Character.isWhitespace(text.charAt(textStart))) {
+				// if there is no space between element tag and text but text exceeds max line
+				// width, move text to new line. (when text is only one term)
+				// ex: ...<example>|text </example>
+				int mixedContentIndentLevel = parentConstraints.getMixedContentIndentLevel() == 0 ? indentLevel
+						: parentConstraints.getMixedContentIndentLevel();
+				replaceLeftSpacesWithIndentationPreservedNewLines(textStart, textStart, mixedContentIndentLevel,
+						edits);
+				availableLineWidth = getMaxLineWidth() - (textEnd - textStart) - mixedContentIndentLevel * getTabSize();
 			} else {
 				if (formatElementCategory == FormatElementCategory.NormalizeSpace
 						|| parentElement.getLastChild() == textNode) {
@@ -126,6 +140,9 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 				}
 				replaceLeftSpacesWithIndentationPreservedNewLines(spaceStart, spaceEnd + 1, indentLevel,
 						edits);
+				if (isMaxLineWidthSupported()) {
+					availableLineWidth = getMaxLineWidth() - (textEnd - textStart) - indentLevel * getTabSize();
+				}
 			}
 		} else if (isTrimTrailingWhitespace()) {
 			removeLeftSpaces(spaceStart, lineSeparatorOffset, edits);

diff --git a/...t/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java b/...t/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java
@@ -59,7 +59,6 @@ public void closeTagMissing() throws BadLocationException {
 		String content = "<a>";
 		String expected = content;
 		assertFormat(content, expected);
-		assertFormat(expected, expected);
 	}
 
 	@Test
@@ -76,15 +75,13 @@ public void selfClosingTag() throws BadLocationException {
 		String content = "<a></a>";
 		String expected = content;
 		assertFormat(content, expected);
-		assertFormat(expected, expected);
 	}
 
 	@Test
 	public void singleEndTag() throws BadLocationException {
 		String content = "</a>";
 		String expected = content;
 		assertFormat(content, expected);
-		assertFormat(expected, expected);
 	}
 
 	@Test

diff --git a/...st/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterMaxLineWithTest.java b/...st/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterMaxLineWithTest.java
@@ -425,28 +425,29 @@ public void mixedTextDefaultLineWidth() throws BadLocationException {
 				"\r\n" + //
 				"  Using\r\n" + //
 				"  namespaces in your documents is very easy. Consider this simple article marked up in DocBook V4.5:</para>";
-		String expected = "<para>All DocBook V5.0 elements are in the namespace <uri>http://docbook.org/ns/docbook</uri>.\r\n"
-				+ //
-				"  <acronym>XML <alt>Extensible Markup Language</alt></acronym> namespaces are\r\n" + //
-				"  used to distinguish between different element sets. In the last few years,\r\n" + //
-				"  almost all new XML grammars have used their own namespace. It is easy to\r\n" + //
-				"  create compound documents that contain elements from different XML\r\n" + //
-				"  vocabularies. DocBook V5.0 is <emphasis>following</emphasis> this <emphasis>design</emphasis>/\r\n" + //
-				"  <emphasis>rule</emphasis>. Using namespaces in your documents is very easy.\r\n" + //
-				"  Consider this simple article marked up in DocBook V4.5:</para>";
+		String expected = "<para>All DocBook V5.0 elements are in the namespace <uri>\r\n" + //
+				"  http://docbook.org/ns/docbook</uri>. <acronym>XML <alt>Extensible Markup\r\n" + //
+				"  Language</alt></acronym> namespaces are used to distinguish between different\r\n" + //
+				"  element sets. In the last few years, almost all new XML grammars have used\r\n" + //
+				"  their own namespace. It is easy to create compound documents that contain\r\n" + //
+				"  elements from different XML vocabularies. DocBook V5.0 is <emphasis>following</emphasis>\r\n" + //
+				"  this <emphasis>design</emphasis>/<emphasis>rule</emphasis>. Using namespaces\r\n" + //
+				"  in your documents is very easy. Consider this simple article marked up in\r\n" + //
+				"  DocBook V4.5:</para>";
 		assertFormat(content, expected, settings, //
-				te(0, 94, 0, 95, "\r\n  "), //
-				te(0, 130, 1, 2, " "), //
-				te(1, 41, 1, 42, "\r\n  "), //
-				te(1, 116, 1, 117, "\r\n  "), //
+				te(0, 58, 0, 58, "\r\n  "), //
+				te(1, 79, 1, 80, "\r\n  "), //
 				te(1, 131, 2, 2, " "),
-				te(2, 59, 2, 60, "\r\n  "), //
+				te(2, 24, 2, 25, "\r\n  "), //
+				te(2, 98, 2, 99, "\r\n  "), //
+				te(2, 126, 3, 2, " "), //
 				te(3, 31, 6, 2, " "), //
+				te(6, 32, 6, 33, "\r\n  "), //
 				te(6, 37, 7, 2, " "),
-				te(7, 30, 7, 30, "\r\n  "), //
 				te(7, 56, 9, 2, " "), //
 				te(9, 7, 10, 2, " "), //
-				te(10, 44, 10, 45, "\r\n  "));
+				te(10, 12, 10, 13, "\r\n  "), //
+				te(10, 86, 10, 87, "\r\n  "));
 		assertFormat(expected, expected, settings);
 	}
 

diff --git a/...va/org/eclipse/lemminx/services/format/experimental/XMLFormatterMixedContentWithTest.java b/...va/org/eclipse/lemminx/services/format/experimental/XMLFormatterMixedContentWithTest.java
@@ -28,9 +28,14 @@ public class XMLFormatterMixedContentWithTest extends AbstractCacheBasedTest {
 
 	@Test
 	public void mixedContent() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setMaxLineWidth(20);
+		settings.getFormattingSettings().setJoinContentLines(true);
 		String content = "<a>abcd    \r\n   efgh</a>";
 		String expected = "<a>abcd efgh</a>";
-		assertFormat(content, expected, 20);
+		assertFormat(content, expected, settings, //
+				te(0, 7, 1, 3, " "));
+		assertFormat(expected, expected, settings);
 	}
 
 	@Test
@@ -41,7 +46,12 @@ public void ignoreSpace() throws BadLocationException {
 				"    <c></c>" + System.lineSeparator() + //
 				"  </b>" + System.lineSeparator() + //
 				"</a>";
-		assertFormat(content, expected, null);
+		assertFormat(content, expected, //
+				te(0, 3, 0, 3, System.lineSeparator() + "  "), //
+				te(0, 6, 0, 6, System.lineSeparator() + "    "), //
+				te(0, 13, 0, 13, System.lineSeparator() + "  "), //
+				te(0, 17, 0, 17, System.lineSeparator()));
+		assertFormat(expected, expected);
 	}
 
 	@Test
@@ -50,7 +60,10 @@ public void withMixedContent() throws BadLocationException {
 		String expected = "<a>" + System.lineSeparator() + //
 				"  <b>A<c></c></b>" + System.lineSeparator() + //
 				"</a>";
-		assertFormat(content, expected, null);
+		assertFormat(content, expected, //
+				te(0, 3, 0, 3, System.lineSeparator() + "  "), //
+				te(0, 18, 0, 18, System.lineSeparator()));
+		assertFormat(expected, expected);
 	}
 
 	@Test
@@ -69,7 +82,7 @@ public void withMixedContentWhiteSpaceLeft() throws BadLocationException {
 	public void withMixedContentNoWhiteSpaceLeft() throws BadLocationException {
 		SharedSettings settings = new SharedSettings();
 		String content = "<a><b> content </b> test </a>";
-		String expected = "<a><b> content </b> test </a>";
+		String expected = content;
 		assertFormat(content, expected, settings);
 	}
 
@@ -88,9 +101,19 @@ public void withMixedContentWhiteSpaceRight() throws BadLocationException {
 	@Test
 	public void withMixedContentNoWhiteSpaceRight() throws BadLocationException {
 		SharedSettings settings = new SharedSettings();
-		String content = "<a> test <b> content </b></a>";
-		String expected = "<a> test <b> content </b></a>";
-		assertFormat(content, expected, settings);
+		settings.getFormattingSettings().setMaxLineWidth(20);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a>abcd    \r\n   efgh</a>";
+		String expected = "<a>abcd efgh</a>";
+		assertFormat(content, expected, settings, //
+				te(0, 7, 1, 3, " "));
+		assertFormat(expected, expected, settings);
+	}
+
+	private static void assertFormat(String unformatted, String expected, TextEdit... expectedEdits)
+			throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		assertFormat(unformatted, expected, settings, "test://test.html", true, expectedEdits);
 	}
 
 	private static void assertFormat(String unformatted, String expected, SharedSettings sharedSettings,
@@ -104,16 +127,4 @@ private static void assertFormat(String unformatted, String expected, SharedSett
 		sharedSettings.getFormattingSettings().setExperimental(true);
 		XMLAssert.assertFormat(null, unformatted, expected, sharedSettings, uri, considerRangeFormat, expectedEdits);
 	}
-
-	private static void assertFormat(String unformatted, String actual, Integer maxLineWidth)
-			throws BadLocationException {
-		SharedSettings sharedSettings = new SharedSettings();
-		if (maxLineWidth != null) {
-			sharedSettings.getFormattingSettings().setMaxLineWidth(maxLineWidth);
-		}
-		// Force to "experimental" formatter
-		sharedSettings.getFormattingSettings().setExperimental(true);
-		sharedSettings.getFormattingSettings().setJoinContentLines(true);
-		XMLAssert.assertFormat(unformatted, actual, sharedSettings, "test.xml", Boolean.FALSE);
-	}
 }
diff --git a/...pse/lemminx/services/format/experimental/XMLFormatterPreserveAttributeLineBreaksTest.java b/...pse/lemminx/services/format/experimental/XMLFormatterPreserveAttributeLineBreaksTest.java
@@ -166,7 +166,8 @@ public void preserveAttributeLineBreaks6() throws BadLocationException {
 		settings.getFormattingSettings().setPreserveAttributeLineBreaks(true);
 		String content = "<a attr=\"value\"\n" + //
 				"</a>";
-		assertFormat(content, content, settings);
+		String expected = content;
+		assertFormat(content, expected, settings);
 	}
 
 	@Test
@@ -175,7 +176,8 @@ public void preserveAttributeLineBreaks7() throws BadLocationException {
 		settings.getFormattingSettings().setPreserveAttributeLineBreaks(true);
 		String content = "<a attr=\"value\"\n" + //
 				"/>";
-		assertFormat(content, content, settings);
+		String expected = content;
+		assertFormat(content, expected, settings);
 	}
 
 	@Test

diff --git a/...rg/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveEmptyContentTest.java b/...rg/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveEmptyContentTest.java
@@ -34,9 +34,7 @@ public void testPreserveEmptyContentTag() throws BadLocationException {
 		String content = "<a>\n" + //
 				"     " + //
 				"</a>";
-		String expected = "<a>\n" + //
-				"     " + //
-				"</a>";
+		String expected = content;
 		assertFormat(content, expected, settings);
 	}
 
@@ -63,9 +61,7 @@ public void testPreserveTextContent() throws BadLocationException {
 		String content = "<a>\n" + //
 				"   aaa  " + //
 				"</a>";
-		String expected = "<a>\n" + //
-				"   aaa  " + //
-				"</a>";
+		String expected = content;
 		assertFormat(content, expected, settings);
 	}
 
@@ -110,8 +106,7 @@ public void testPreserveEmptyContentTagWithSiblingContent() throws BadLocationEx
 
 		String content = "<a>\n" + //
 				"   zz     <b>  </b>tt     </a>";
-		String expected = "<a>\n" + //
-				"   zz     <b>  </b>tt     </a>";
+		String expected = content;
 		assertFormat(content, expected, settings);
 	}
 
@@ -138,8 +133,7 @@ public void testPreserveEmptyContentTagWithSiblingWithComment() throws BadLocati
 
 		String content = "<a>\n" + //
 				"   zz    <b>  </b>tt <!-- Comment -->     </a>";
-		String expected = "<a>\n" + //
-				"   zz    <b>  </b>tt <!-- Comment -->     </a>";
+		String expected = content;
 		assertFormat(content, expected, settings);
 	}
 

diff --git a/...java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveSpacesTest.java b/...java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveSpacesTest.java
@@ -72,7 +72,6 @@ public void preserveSpacesWithXmlSpace2() throws BadLocationException {
 	public void preserveSpacesWithSettings() throws BadLocationException {
 		String content = "<a>b  c</a>";
 		String expected = content;
-
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setPreserveSpace(Arrays.asList("a"));
 		assertFormat(content, expected, settings);

diff --git a/...est/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterQuoteStyleTest.java b/...est/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterQuoteStyleTest.java
@@ -272,9 +272,8 @@ public void testUseSingleQuotesLocalDTDUnclosedStart() throws BadLocationExcepti
 		settings.getPreferences().setQuoteStyle(QuoteStyle.singleQuotes);
 		settings.getFormattingSettings().setEnforceQuoteStyle(EnforceQuoteStyle.preferred);
 		String content = "<!DOCTYPE note SYSTEM note.dtd\">";
-		String expected = "<!DOCTYPE note SYSTEM note.dtd\">";
+		String expected = content;
 		assertFormat(content, expected, settings);
-		assertFormat(expected, expected, settings);
 	}
 
 	@Test
@@ -283,9 +282,8 @@ public void testUseSingleQuotesLocalDTDUnclosedEnd() throws BadLocationException
 		settings.getPreferences().setQuoteStyle(QuoteStyle.singleQuotes);
 		settings.getFormattingSettings().setEnforceQuoteStyle(EnforceQuoteStyle.preferred);
 		String content = "<!DOCTYPE note SYSTEM \"note.dtd>";
-		String expected = "<!DOCTYPE note SYSTEM \"note.dtd>";
+		String expected = content;
 		assertFormat(content, expected, settings);
-		assertFormat(expected, expected, settings);
 	}
 
 	@Test
@@ -530,7 +528,6 @@ public void dontEnforceSingleQuoteStyleProlog() throws BadLocationException {
 		String content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
 		String expected = content;
 		assertFormat(content, expected, settings);
-		assertFormat(expected, expected, settings);
 	}
 
 	@Test
@@ -542,7 +539,6 @@ public void dontEnforceDoubleQuoteStyleProlog() throws BadLocationException {
 		String content = "<?xml version=\'1.0\' encoding=\'UTF-8\'?>";
 		String expected = content;
 		assertFormat(content, expected, settings);
-		assertFormat(expected, expected, settings);
 	}
 
 	@Test

diff --git a/...ava/org/eclipse/lemminx/services/format/experimental/XMLFormatterSplitAttributesTest.java b/...ava/org/eclipse/lemminx/services/format/experimental/XMLFormatterSplitAttributesTest.java
@@ -153,7 +153,7 @@ public void testNestedAttributesNoSplit() throws BadLocationException {
 	@Test
 	public void testSplitAttributesProlog() throws BadLocationException {
 		String content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
-		String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
+		String expected = content;
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setSplitAttributes(true);
 		assertFormat(content, expected, settings);
@@ -162,7 +162,7 @@ public void testSplitAttributesProlog() throws BadLocationException {
 	@Test
 	public void testSplitAttributesSingle() throws BadLocationException {
 		String content = "<a k1=\"v1\"></a>";
-		String expected = "<a k1=\"v1\"></a>";
+		String expected = content;
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setSplitAttributes(true);
 		assertFormat(content, expected, settings);

diff --git a/...e/lemminx/services/format/experimental/XMLFormatterWithXSDGrammarAwareFormattingTest.java b/...e/lemminx/services/format/experimental/XMLFormatterWithXSDGrammarAwareFormattingTest.java
@@ -57,7 +57,6 @@ public void testXSDForEmptyMixedElement() throws Exception {
 		settings.getFormattingSettings().setGrammarAwareFormatting(true);
 		try {
 			assertFormat(content, expected, settings);
-			assertFormat(expected, expected, settings);
 		} catch (Exception ex) {
 			fail("Formatter failed to process text", ex);
 		}