Skip to content

Commit

Permalink
Range formatting inserts <null> when formatting inside DOCTYPE element
Browse files Browse the repository at this point in the history 
Fixes #682

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr
angelozerr committed Jun 5, 2020
1 parent 84d2689 commit f711b24
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 25 deletions.
43 changes: 21 additions & 22 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.eclipse.lemminx.dom.DTDAttlistDecl;
import org.eclipse.lemminx.dom.DTDDeclNode;
import org.eclipse.lemminx.dom.DTDDeclParameter;
import org.eclipse.lemminx.services.extensions.XMLExtensionsRegistry;
import org.eclipse.lemminx.settings.SharedSettings;
import org.eclipse.lemminx.settings.XMLFormattingOptions.EmptyElements;
import org.eclipse.lemminx.utils.XMLBuilder;
Expand All @@ -47,7 +46,6 @@
class XMLFormatter {

private static final Logger LOGGER = Logger.getLogger(XMLFormatter.class.getName());
private final XMLExtensionsRegistry extensionsRegistry;

private static class XMLFormatterDocument {
private final TextDocument textDocument;
Expand All @@ -62,6 +60,7 @@ private static class XMLFormatterDocument {
private XMLBuilder xmlBuilder;
private int indentLevel;
private boolean linefeedOnNextWrite;
private boolean withinDTDContent;

/**
* XML formatter document.
Expand Down Expand Up @@ -112,13 +111,17 @@ private void setupRangeFormatting(Range range) throws BadLocationException {
String fullText = this.textDocument.getText();
String rangeText = fullText.substring(this.startOffset, this.endOffset);

this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, this.textDocument.getUri(), null, false);
withinDTDContent = this.fullDomDocument.isWithinInternalDTD(startOffset);
String uri = this.textDocument.getUri();
if (withinDTDContent) {
uri += ".dtd";
}
this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, uri, null, false);

if (containsTextWithinStartTag()) {
adjustOffsetToStartTag();
rangeText = fullText.substring(this.startOffset, this.endOffset);
this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, this.textDocument.getUri(), null,
false);
this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, uri, null, false);
}

this.xmlBuilder = new XMLBuilder(this.sharedSettings, "",
Expand Down Expand Up @@ -161,8 +164,8 @@ private void setupFullFormatting(Range range) throws BadLocationException {
this.rangeDomDocument = this.fullDomDocument;

Position startPosition = textDocument.positionAt(startOffset);
this.xmlBuilder = new XMLBuilder(this.sharedSettings,
"", textDocument.lineDelimiter(startPosition.getLine()));
this.xmlBuilder = new XMLBuilder(this.sharedSettings, "",
textDocument.lineDelimiter(startPosition.getLine()));
}

private void enlargePositionToGutters(Position start, Position end) throws BadLocationException {
Expand All @@ -176,9 +179,10 @@ private void enlargePositionToGutters(Position start, Position end) throws BadLo
}

private int getStartingIndentLevel() throws BadLocationException {

if (withinDTDContent) {
return 1;
}
DOMNode startNode = this.fullDomDocument.findNodeAt(this.startOffset);

if (startNode.isOwnerDocument()) {
return 0;
}
Expand Down Expand Up @@ -271,9 +275,9 @@ private void format(DOMNode node) throws BadLocationException {
}

if (node.getNodeType() != DOMNode.DOCUMENT_NODE) {
boolean doLineFeed = !node.getOwnerDocument().isDTD() &&
!(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
Expand Down Expand Up @@ -374,7 +378,7 @@ private void formatDocumentType(DOMDocumentType documentType) {
linefeedOnNextWrite = true;

} else {
formatDTD(documentType, 0, this.endOffset, this.xmlBuilder);
formatDTD(documentType, this.indentLevel, this.endOffset, this.xmlBuilder);
}
}

Expand Down Expand Up @@ -633,7 +637,8 @@ private List<? extends TextEdit> getFormatTextEdit() throws BadLocationException
this.xmlBuilder.trimFinalNewlines();
}

if (this.sharedSettings.getFormattingSettings().isInsertFinalNewline() && !this.xmlBuilder.isLastLineEmptyOrWhitespace()) {
if (this.sharedSettings.getFormattingSettings().isInsertFinalNewline()
&& !this.xmlBuilder.isLastLineEmptyOrWhitespace()) {
this.xmlBuilder.linefeed();
}
}
Expand Down Expand Up @@ -679,10 +684,6 @@ private static void addPrologAttributes(DOMNode node, XMLBuilder xmlBuilder) {
}
}

public XMLFormatter(XMLExtensionsRegistry extensionsRegistry) {
this.extensionsRegistry = extensionsRegistry;
}

/**
* Returns a List containing a single TextEdit, containing the newly formatted
* changes of the document.
Expand All @@ -692,11 +693,9 @@ public XMLFormatter(XMLExtensionsRegistry extensionsRegistry) {
* @param sharedSettings settings containing formatting preferences
* @return List containing a TextEdit with formatting changes
*/
public List<? extends TextEdit> format(TextDocument textDocument, Range range,
SharedSettings sharedSettings) {
public List<? extends TextEdit> format(TextDocument textDocument, Range range, SharedSettings sharedSettings) {
try {
XMLFormatterDocument formatterDocument = new XMLFormatterDocument(textDocument, range,
sharedSettings);
XMLFormatterDocument formatterDocument = new XMLFormatterDocument(textDocument, range, sharedSettings);
return formatterDocument.format();
} catch (BadLocationException e) {
LOGGER.log(Level.SEVERE, "Formatting failed due to BadLocation", e);
Expand Down
3 changes: 1 addition & 2 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.eclipse.lemminx.settings.SharedSettings;
import org.eclipse.lemminx.settings.XMLCodeLensSettings;
import org.eclipse.lemminx.settings.XMLFoldingSettings;
import org.eclipse.lemminx.settings.XMLHoverSettings;
import org.eclipse.lemminx.settings.XMLSymbolSettings;
import org.eclipse.lemminx.uriresolver.CacheResourceDownloadingException;
import org.eclipse.lemminx.utils.XMLPositionUtility;
Expand Down Expand Up @@ -84,7 +83,7 @@ public void checkCanceled() {
private final XMLRename rename;

public XMLLanguageService() {
this.formatter = new XMLFormatter(this);
this.formatter = new XMLFormatter();
this.highlighting = new XMLHighlighting(this);
this.symbolsProvider = new XMLSymbolsProvider(this);
this.completions = new XMLCompletions(this);
Expand Down
28 changes: 27 additions & 1 deletion org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,33 @@ public void rangeSelectWithinText() throws BadLocationException {
format(content, expected);
}

@Test
public void rangeSelectEntityNoIndent() throws BadLocationException {
String content = "<?xml version='1.0' standalone='no'?>\r\n" + //
"<!DOCTYPE root-element [\r\n" + //
"|<!ENTITY local \"LOCALLY DECLARED ENTITY\">|\r\n" + //
"]>";
String expected = "<?xml version='1.0' standalone='no'?>\r\n" + //
"<!DOCTYPE root-element [\r\n" + //
" <!ENTITY local \"LOCALLY DECLARED ENTITY\">\r\n" + //
"]>";
format(content, expected);
}

@Test
public void rangeSelectEntityWithIndent() throws BadLocationException {
String content = "<?xml version='1.0' standalone='no'?>\r\n" + //
"<!DOCTYPE root-element [\r\n" + //
" |<!ENTITY local \"LOCALLY DECLARED ENTITY\">|\r\n" + //
"]>";
String expected = "<?xml version='1.0' standalone='no'?>\r\n" + //
"<!DOCTYPE root-element [\r\n" + //
" <!ENTITY local \"LOCALLY DECLARED ENTITY\">\r\n" + //
"]>";
format(content, expected);
}


@Test
public void testProlog() throws BadLocationException {
String content = "<?xml version= \"1.0\" encoding=\"UTF-8\" ?>\r\n";
Expand Down Expand Up @@ -2471,7 +2498,6 @@ public void dontEnforceDoubleQuoteStyle() throws BadLocationException {
format(content, expected, settings);
}


@Test
public void testTemplate() throws BadLocationException {
String content = "";
Expand Down

0 comments on commit f711b24

Please sign in to comment.