From f12cf5d1cbd47f59bdb4b632bed57b11b8195be2 Mon Sep 17 00:00:00 2001 From: David Kwon Date: Tue, 21 Apr 2020 16:16:23 -0400 Subject: [PATCH] Formatting support for trimFinalNewLines and insertFinalNewLine settings Signed-off-by: David Kwon --- .../lemminx/services/XMLFormatter.java | 52 ++++-- .../settings/XMLFormattingOptions.java | 16 +- .../org/eclipse/lemminx/utils/XMLBuilder.java | 126 +++++++++----- .../lemminx/services/XMLFormatterTest.java | 158 ++++++++++++++++-- .../lemminx/settings/SettingsTest.java | 2 +- 5 files changed, 280 insertions(+), 74 deletions(-) diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java index 10c5e3c84..7948bdf8a 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java @@ -61,6 +61,7 @@ private static class XMLFormatterDocument { private DOMDocument rangeDomDocument; private XMLBuilder xmlBuilder; private int indentLevel; + private boolean linefeedOnNextWrite; /** * XML formatter document. @@ -70,6 +71,7 @@ public XMLFormatterDocument(TextDocument textDocument, Range range, XMLFormattin this.range = range; this.options = options; this.emptyElements = options.getEmptyElements(); + this.linefeedOnNextWrite = false; } /** @@ -261,14 +263,15 @@ private boolean startTagExistsInFullDocument(DOMNode node) { private void format(DOMNode node) throws BadLocationException { + if (linefeedOnNextWrite && (!node.isText() || !((DOMText) node).isWhitespace())) { + this.xmlBuilder.linefeed(); + linefeedOnNextWrite = false; + } + if (node.getNodeType() != DOMNode.DOCUMENT_NODE) { - boolean doLineFeed; - if (node.getOwnerDocument().isDTD()) { - doLineFeed = false; - } else { - doLineFeed = !(node.isComment() && ((DOMComment) node).isCommentSameLineEndTag()) - && (!node.isText() || (!((DOMText) node).isWhitespace() && ((DOMText) node).hasSiblings())); - } + boolean doLineFeed = !node.getOwnerDocument().isDTD() && + !(node.isComment() && ((DOMComment) node).isCommentSameLineEndTag()) && + (!node.isText() || (!((DOMText) node).isWhitespace() && ((DOMText) node).hasSiblings())); if (this.indentLevel > 0 && doLineFeed) { // add new line + indent @@ -317,12 +320,12 @@ private void format(DOMNode node) throws BadLocationException { /** * Format the given DOM prolog - * + * * @param node the DOM prolog to format. */ private void formatProlog(DOMNode node) { addPrologToXMLBuilder(node, this.xmlBuilder); - this.xmlBuilder.linefeed(); + linefeedOnNextWrite = true; } /** @@ -331,10 +334,13 @@ private void formatProlog(DOMNode node) { * @param textNode the DOM text node to format. */ private void formatText(DOMText textNode) { - // Generate content String content = textNode.getData(); - xmlBuilder.addContent(content, textNode.isWhitespace(), textNode.hasSiblings(), textNode.getDelimiter(), - this.indentLevel); + if (textNode.equals(this.fullDomDocument.getLastChild())) { + xmlBuilder.addContent(content); + } else { + xmlBuilder.addContent(content, textNode.isWhitespace(), textNode.hasSiblings(), + textNode.getDelimiter()); + } } /** @@ -363,7 +369,8 @@ private void formatDocumentType(DOMDocumentType documentType) { if (documentType.isClosed()) { xmlBuilder.endDoctype(); } - xmlBuilder.linefeed(); + linefeedOnNextWrite = true; + } else { formatDTD(documentType, 0, this.endOffset, this.xmlBuilder); } @@ -373,7 +380,7 @@ private void formatDocumentType(DOMDocumentType documentType) { * Format the given DOM ProcessingIntsruction. * * @param element the DOM ProcessingIntsruction to format. - * + * */ private void formatProcessingInstruction(DOMNode node) { addPIToXMLBuilder(node, this.xmlBuilder); @@ -386,14 +393,14 @@ private void formatProcessingInstruction(DOMNode node) { * Format the given DOM Comment * * @param element the DOM Comment to format. - * + * */ private void formatComment(DOMComment comment) { this.xmlBuilder.startComment(comment); this.xmlBuilder.addContentComment(comment.getData()); this.xmlBuilder.endComment(); if (this.indentLevel == 0) { - this.xmlBuilder.linefeed(); + linefeedOnNextWrite = true; } } @@ -616,6 +623,19 @@ private List getFormatTextEdit() throws BadLocationException Position endPosition = this.textDocument.positionAt(this.endOffset); Range r = new Range(startPosition, endPosition); List edits = new ArrayList<>(); + + // check if format range reaches the end of the document + if (this.endOffset == this.textDocument.getText().length()) { + + if (options.isTrimFinalNewlines()) { + this.xmlBuilder.trimFinalNewlines(); + } + + if (this.options.isInsertFinalNewline() && !this.xmlBuilder.isLastLineEmptyOrWhitespace()) { + this.xmlBuilder.linefeed(); + } + } + edits.add(new TextEdit(r, this.xmlBuilder.toString())); return edits; } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java index 2090b2693..f4e659728 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java @@ -25,6 +25,8 @@ public class XMLFormattingOptions extends FormattingOptions { public static final String DEFAULT_QUOTATION = "\""; + public static final int DEFAULT_PRESERVER_NEW_LINES = 2; + public static final int DEFAULT_TAB_SIZE = 2; // All possible keys private static final String SPLIT_ATTRIBUTES = "splitAttributes"; @@ -85,7 +87,7 @@ enum Quotations { * * } * - * + * * *
  • {@link #ignore} : keeps the original XML content for empty elements. *
    • @@ -115,7 +117,9 @@ public XMLFormattingOptions(boolean initializeDefaults) { /** * Necessary: Initialize default values in case client does not provide one */ - public void initializeDefaultSettings() { + private void initializeDefaultSettings() { + super.setTabSize(DEFAULT_TAB_SIZE); + super.setInsertSpaces(true); this.setSplitAttributes(false); this.setJoinCDATALines(false); this.setFormatComments(true); @@ -125,15 +129,16 @@ public void initializeDefaultSettings() { this.setSpaceBeforeEmptyCloseTag(true); this.setQuotations(DOUBLE_QUOTES_VALUE); this.setPreserveEmptyContent(false); - this.setPreservedNewlines(2); + this.setPreservedNewlines(DEFAULT_PRESERVER_NEW_LINES); this.setEmptyElement(EmptyElements.ignore); } public XMLFormattingOptions(int tabSize, boolean insertSpaces, boolean initializeDefaultSettings) { - super(tabSize, insertSpaces); if (initializeDefaultSettings) { initializeDefaultSettings(); } + super.setTabSize(tabSize); + super.setInsertSpaces(insertSpaces); } public XMLFormattingOptions(int tabSize, boolean insertSpaces) { @@ -315,7 +320,6 @@ public void setPreservedNewlines(final int preservedNewlines) { } public int getPreservedNewlines() { - final Number value = this.getNumber(XMLFormattingOptions.PRESERVED_NEWLINES); if ((value != null)) { return value.intValue(); @@ -355,4 +359,4 @@ public static XMLFormattingOptions create(FormattingOptions options, FormattingO return new XMLFormattingOptions(options).merge(sharedFormattingOptions); } -} \ No newline at end of file +} diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLBuilder.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLBuilder.java index e80b2e1bb..8b92f09c5 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLBuilder.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLBuilder.java @@ -79,7 +79,7 @@ public XMLBuilder endElement(String prefix, String name, boolean isEndTagClosed) xml.append(":"); } xml.append(name); - if(isEndTagClosed) { + if (isEndTagClosed) { xml.append(">"); } return this; @@ -107,18 +107,18 @@ public XMLBuilder addSingleAttribute(String name, String value) { * * It will not perform any linefeeds and only basic indentation. * - * @param name attribute name + * @param name attribute name * @param value attribute value * @return */ public XMLBuilder addSingleAttribute(String name, String value, boolean surroundWithQuotes) { xml.append(" "); addAttributeContents(name, true, value, surroundWithQuotes); - + return this; } - public XMLBuilder addAttributesOnSingleLine(DOMAttr attr, Boolean surroundWithQuotes) { + public XMLBuilder addAttributesOnSingleLine(DOMAttr attr, boolean surroundWithQuotes) { xml.append(" "); addAttributeContents(attr.getName(), attr.hasDelimiter(), attr.getValue(), surroundWithQuotes); @@ -167,24 +167,33 @@ public XMLBuilder addAttribute(DOMAttr attr, int level, boolean surroundWithQuot return this; } - /** - * Builds the attribute {name, '=', and value}. - * - * Never puts quotes around unquoted values unless indicated to by 'surroundWithQuotes' - */ - private void addAttributeContents(String name, Boolean equalsSign, String originalValue, boolean surroundWithQuotes) { - if(name != null) { + /** + * * Builds the attribute {name, '=', and value}. + * + * Never puts quotes around unquoted values unless indicated to by + * 'surroundWithQuotes' + * + * @param name name of the attribute + * @param equalsSign true if equals sign exists, false otherwise + * @param originalValue value of the attribute + * @param surroundWithQuotes true if value should be added with quotes, false otherwise + */ + private void addAttributeContents(String name, boolean equalsSign, String originalValue, + boolean surroundWithQuotes) { + if (name != null) { xml.append(name); } - if(equalsSign) { + if (equalsSign) { xml.append("="); } - if(originalValue != null) { + if (originalValue != null) { String quote = formattingOptions.isQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE) ? "\"" : "'"; - - if(DOMAttr.isQuoted(originalValue)) { - if(originalValue.charAt(0) == '\'' && formattingOptions.isQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE) || - originalValue.charAt(0) == '\"' && formattingOptions.isQuotations(XMLFormattingOptions.SINGLE_QUOTES_VALUE)) { + + if (DOMAttr.isQuoted(originalValue)) { + if (originalValue.charAt(0) == '\'' + && formattingOptions.isQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE) + || originalValue.charAt(0) == '\"' + && formattingOptions.isQuotations(XMLFormattingOptions.SINGLE_QUOTES_VALUE)) { originalValue = DOMAttr.convertToQuotelessValue(originalValue); xml.append(quote); @@ -193,24 +202,21 @@ private void addAttributeContents(String name, Boolean equalsSign, String origin } xml.append(quote); return; - } - else { + } else { xml.append(originalValue); return; - } - } - else if(surroundWithQuotes) { + } + } else if (surroundWithQuotes) { xml.append(quote); if (originalValue != null) { xml.append(originalValue); } xml.append(quote); return; - } - else { + } else { xml.append(originalValue); } - } + } } public XMLBuilder linefeed() { @@ -221,32 +227,49 @@ public XMLBuilder linefeed() { return this; } + /** + * Returns this XMLBuilder with text added + * + * @param text the text to add + * @return this XMLBuilder with text added + */ public XMLBuilder addContent(String text) { - return addContent(text, false, false, null, 0); + return addContent(text, false, false, null); } - public XMLBuilder addContent(String text, Boolean isWhitespaceContent, Boolean hasSiblings, String delimiter, int level) { - if(!isWhitespaceContent) { - if(isJoinContentLines()) { + /** + * Returns this XMLBuilder with text added depending on + * isWhitespaceContent, hasSiblings and + * delimiter + * + * @param text the proposed text to add + * @param isWhitespaceContent whether or not the text contains whitespace content + * @param hasSiblings whether or not the corresponding text node has siblings + * @param delimiter line delimiter + * @return this XMLBuilder with text added depending on + * isWhitespaceContent, hasSiblings and + * delimiter + */ + public XMLBuilder addContent(String text, boolean isWhitespaceContent, boolean hasSiblings, + String delimiter) { + if (!isWhitespaceContent) { + if (isJoinContentLines()) { text = StringUtils.normalizeSpace(text); - } - else if(hasSiblings) { + } else if (hasSiblings) { text = text.trim(); } xml.append(text); - } - else if (!hasSiblings && isPreserveEmptyContent()) { + } else if (!hasSiblings && isPreserveEmptyContent()) { xml.append(text); - } - else if(hasSiblings) { + } else if (hasSiblings) { int preservedNewLines = getPreservedNewlines(); - if(preservedNewLines > 0) { - int newLineCount = StringUtils.getNumberOfNewLines(text, isWhitespaceContent, delimiter, preservedNewLines); + if (preservedNewLines > 0) { + int newLineCount = StringUtils.getNumberOfNewLines(text, isWhitespaceContent, delimiter, + preservedNewLines); for (int i = 0; i < newLineCount - 1; i++) { // - 1 because the node after will insert a delimiter xml.append(delimiter); } } - } return this; } @@ -287,6 +310,16 @@ public String toString() { return xml.toString(); } + /** + * Trims the trailing newlines for the current xml StringBuilder + */ + public void trimFinalNewlines() { + int i = xml.length() - 1; + while (i >= 0 && Character.isWhitespace(xml.charAt(i))) { + xml.deleteCharAt(i--); + } + } + public XMLBuilder startCDATA() { xml.append(" 0 && Character.isSpaceChar(this.xml.charAt(i))) { + i--; + } + return i > 0 && (this.xml.charAt(i) == '\r' || this.xml.charAt(i) == '\n'); + } + private boolean isJoinCommentLines() { return formattingOptions != null && formattingOptions.isJoinCommentLines(); } @@ -394,10 +438,10 @@ private boolean isPreserveEmptyContent() { } private int getPreservedNewlines() { - if(formattingOptions != null) { + if (formattingOptions != null) { return formattingOptions.getPreservedNewlines(); } - return 2; // default + return XMLFormattingOptions.DEFAULT_PRESERVER_NEW_LINES; } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java index 29dab4cf7..48e5e6c25 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java @@ -411,7 +411,7 @@ public void testNestedAttributesNoSplit() throws BadLocationException { @Test public void testSplitAttributesProlog() throws BadLocationException { String content = ""; - String expected = "" + lineSeparator(); + String expected = ""; XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); formattingOptions.setSplitAttributes(true); format(content, expected, formattingOptions); @@ -549,7 +549,7 @@ public void testJoinCommentLines() throws BadLocationException { " " + lineSeparator() + // " line 2" + lineSeparator() + // " -->"; - String expected = "" + lineSeparator(); + String expected = ""; XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); formattingOptions.setJoinCommentLines(true); format(content, expected, formattingOptions); @@ -596,7 +596,7 @@ public void testCommentFormatSameLine() throws BadLocationException { " "; String expected = "" + lineSeparator() + // " Content" + lineSeparator() + // - " " + lineSeparator(); + " "; XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); formattingOptions.setJoinCommentLines(true); @@ -1204,9 +1204,8 @@ public void testDoctypeInternalDeclSpacesBetweenParameters() throws BadLocationE " Jani\r\n" + // " Reminder\r\n" + // " Don't forget me this weekend\r\n" + // - " "; + ""; String expected = "\r\n" + // " \r\n" + // " \r\n" + // @@ -1338,7 +1337,7 @@ public void testDoctypeInternalWithText() throws BadLocationException { " df\r\n" + // " gd\r\n" + // " \r\n" + // - "]>\r\n"; + "]>"; format(content, expected, formattingOptions); } @@ -1499,7 +1498,7 @@ public void testDoctypeInvalidParameter() throws BadLocationException { String expected = "\r\n" + // " \r\n" + // - "]>\r\n"; + "]>"; format(content, expected, formattingOptions); } @@ -1784,7 +1783,7 @@ public void testUseDoubleQuotesMultipleAttributesSplit() throws BadLocationExcep String content = " \n"; String expected = ""; + + " name3=\" value3 \">\n"; format(content, expected, formattingOptions); } @@ -1795,7 +1794,7 @@ public void testUseSingleQuotesMultipleAttributesSplit() throws BadLocationExcep formattingOptions.setQuotations(XMLFormattingOptions.SINGLE_QUOTES_VALUE); String content = " \n"; String expected = ""; + + " name3=\' value3 \'>\n"; format(content, expected, formattingOptions); } @@ -2055,6 +2054,145 @@ public void testNoSpacesOnNewLine() throws BadLocationException { format(content, expected, formattingOptions); } + @Test + public void testDontInsertFinalNewLine1() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + formattingOptions.setInsertFinalNewline(true); + String content = ""; + format(content, content, formattingOptions); + } + + @Test + public void testDontInsertFinalNewLine2() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + formattingOptions.setInsertFinalNewline(true); + String content = "" + lineSeparator(); + String expected = "" + lineSeparator(); + format(content, expected, formattingOptions); + } + + @Test + public void testDontInsertFinalNewLine3() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + formattingOptions.setInsertFinalNewline(true); + String content = "" + lineSeparator() + " "; + String expected = "" + lineSeparator() + " "; + format(content, expected, formattingOptions); + } + + @Test + public void testInsertFinalNewLine1() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setInsertFinalNewline(true); + String content = ""; + String expected = "" + lineSeparator(); + format(content, expected, formattingOptions); + } + + @Test + public void testInsertFinalNewLine2() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(true); + formattingOptions.setInsertFinalNewline(true); + String content = "\r\n\r\n"; + String expected = "\r\n"; + format(content, expected, formattingOptions); + } + + @Test + public void testInsertFinalNewLine3() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(true); + formattingOptions.setInsertFinalNewline(true); + String content = "\n\n"; + String expected = "\n"; + format(content, expected, formattingOptions); + } + + @Test + public void testDontInsertFinalNewLineWithRange() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setInsertFinalNewline(true); + String content = "
    " + lineSeparator() + // + " |" + lineSeparator() + // + "
    "; + String expected = "
    " + lineSeparator() + // + " " + lineSeparator() + // + "
    "; + format(content, expected, formattingOptions); + } + + @Test + public void testInsertFinalNewLineWithRange2() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setInsertFinalNewline(true); + String content = "
    " + lineSeparator() + // + " |" + lineSeparator() + // + "
    |"; + String expected = "
    " + lineSeparator() + // + " " + lineSeparator() + // + "
    " + lineSeparator(); + format(content, expected, formattingOptions); + } + + // Problem + @Test + public void testInsertFinalNewLineWithRange3() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setInsertFinalNewline(true); + String content = "
    " + lineSeparator() + // + " |" + lineSeparator() + // + lineSeparator() + "|" + lineSeparator() + // + "

    " + lineSeparator() + // + "
    "; + String expected = "
    " + lineSeparator() + // + " " + lineSeparator() + // + lineSeparator() + // + "

    " + lineSeparator() + // + "
    "; + format(content, expected, formattingOptions); + } + + @Test + public void testDontTrimFinalNewLines() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + String content = "" + lineSeparator() + lineSeparator() + lineSeparator(); + String expected = "" + lineSeparator() + lineSeparator() + lineSeparator(); + format(content, expected, formattingOptions); + } + + @Test + public void testDontTrimFinalNewLines2() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + String content = "" + lineSeparator() + // + " " + lineSeparator() + // + lineSeparator(); + String expected = "" + lineSeparator() + // + " " + lineSeparator() + // + lineSeparator(); + format(content, expected, formattingOptions); + } + + @Test + public void testDontTrimFinalNewLines3() throws BadLocationException { + XMLFormattingOptions formattingOptions = createDefaultFormattingOptions(); + formattingOptions.setTrimFinalNewlines(false); + String content = "" + lineSeparator() + // + " text " + lineSeparator() + // + " more text " + lineSeparator() + // + " " + lineSeparator(); + String expected = "" + lineSeparator() + // + " text " + lineSeparator() + // + " more text " + lineSeparator() + // + " " + lineSeparator(); + format(content, expected, formattingOptions); + } + // ------------ Tests with format empty elements settings @Test @@ -2277,6 +2415,6 @@ private static void format(String unformatted, String expected, XMLFormattingOpt } private static XMLFormattingOptions createDefaultFormattingOptions() { - return new XMLFormattingOptions(2, true); + return new XMLFormattingOptions(true); } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/settings/SettingsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/settings/SettingsTest.java index 4806298e0..ed5f9939d 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/settings/SettingsTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/settings/SettingsTest.java @@ -168,7 +168,7 @@ public void formatSettings() { formattingOptions.setTabSize(5); formattingOptions.setInsertSpaces(false); - XMLFormattingOptions xmlFormattingOptions = new XMLFormattingOptions(formattingOptions); + XMLFormattingOptions xmlFormattingOptions = new XMLFormattingOptions(formattingOptions, false); xmlFormattingOptions.setQuotations("InvalidValue"); // set a value that is not recognized