Skip to content

Commit

Permalink
Improve full support for max line width
Browse files Browse the repository at this point in the history 
Signed-off-by: Jessica He <jhe@redhat.com>
  • Loading branch information
@JessicaJHee @datho7561
JessicaJHee authored and datho7561 committed Nov 16, 2022
1 parent d505e7e commit f6c8d9a
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 66 deletions.
3 changes: 1 addition & 2 deletions ...clipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMElementFormatter.java
View file
Expand Up @@ -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());
Expand Down
25 changes: 21 additions & 4 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java
View file
Expand Up @@ -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...
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions ...t/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java
View file
Expand Up @@ -59,7 +59,6 @@ public void closeTagMissing() throws BadLocationException {
String content = "<a>";
String expected = content;
assertFormat(content, expected);
assertFormat(expected, expected);
}

@Test
Expand All @@ -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
Expand Down
33 changes: 17 additions & 16 deletions ...st/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterMaxLineWithTest.java
View file
Expand Up @@ -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);
}

Expand Down
49 changes: 30 additions & 19 deletions ...va/org/eclipse/lemminx/services/format/experimental/XMLFormatterMixedContentWithTest.java
View file
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
}

Expand All @@ -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,
Expand All @@ -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);
}
}
6 changes: 4 additions & 2 deletions ...pse/lemminx/services/format/experimental/XMLFormatterPreserveAttributeLineBreaksTest.java
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down
14 changes: 4 additions & 10 deletions ...rg/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveEmptyContentTest.java
View file
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down
1 change: 0 additions & 1 deletion ...java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveSpacesTest.java
View file
Expand Up @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions ...est/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterQuoteStyleTest.java
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions ...ava/org/eclipse/lemminx/services/format/experimental/XMLFormatterSplitAttributesTest.java
View file
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion ...e/lemminx/services/format/experimental/XMLFormatterWithXSDGrammarAwareFormattingTest.java
View file
Expand Up @@ -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);
}
Expand Down

0 comments on commit f6c8d9a

Please sign in to comment.