Skip to content

Commit

Permalink
Fixed forward slash occasionally breaking autoclose
Browse files Browse the repository at this point in the history 
... and autoclosing elements with capitals in their name

Fixes redhat-developer/vscode-xml#126 , eclipse-lemminx#354

Signed-off-by: Nikolas <[email protected]>
  • Loading branch information
@NikolasKomonen @fbricon
NikolasKomonen authored and fbricon committed Apr 24, 2019
1 parent fe527c3 commit 3168b2c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 16 deletions.
4 changes: 2 additions & 2 deletions org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ public DOMDocument parse(TextDocument document, URIResolverExtensionManager reso

case EndTag:
// end tag (ex: </root>)
String closeTag = scanner.getTokenText().toLowerCase();
String closeTag = scanner.getTokenText();
DOMNode current = curr;

/**
eg: <a><b><c></d> will set a,b,c end position to the start of |</d>
*/
while (!(curr.isElement() && ((DOMElement) curr).isSameTag(closeTag)) && curr.parent != null) {
while (!(curr.isElement() && ((DOMElement) curr).isSameTag(closeTag.toLowerCase())) && curr.parent != null) {
curr.end = endTagOpenOffset;
curr = curr.parent;
}
Expand Down
26 changes: 17 additions & 9 deletions org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/XMLScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ TokenType internalScan() {
}
return finishToken(offset, TokenType.Unknown);

case WithinTag:
case WithinTag: {
if (stream.skipWhitespace()) {
return finishToken(offset, TokenType.Whitespace);
}
Expand All @@ -272,18 +272,26 @@ TokenType internalScan() {
return finishToken(offset, TokenType.PrologEnd);
}

lastAttributeName = nextAttributeName();
if (lastAttributeName.length() > 0) {
state = ScannerState.AfterAttributeName;
return finishToken(offset, TokenType.AttributeName);
}
lastAttributeName = nextAttributeName();
if (lastAttributeName.length() > 0) {
state = ScannerState.AfterAttributeName;
return finishToken(offset, TokenType.AttributeName);
}

if (stream.advanceIfChar(_FSL)) { // /
state = ScannerState.WithinContent;
state = ScannerState.WithinTag;
if(stream.advanceIfChar(_RAN)) { // >
state = ScannerState.WithinContent;
return finishToken(offset, TokenType.StartTagSelfClose);
}
return finishToken(offset, TokenType.Unknown);
}
int c = stream.peekChar();
if (c == _DQO || c == _SIQ) { // " || '
state = ScannerState.BeforeAttributeValue;
return internalScan();
}

if (stream.advanceIfChar(_RAN)) { // >
state = ScannerState.WithinContent;
return finishToken(offset, TokenType.StartTagClose);
Expand All @@ -294,7 +302,7 @@ TokenType internalScan() {
return internalScan();
}
return finishToken(offset, TokenType.Unknown);

}
case AfterAttributeName:
if (stream.skipWhitespace()) {
return finishToken(offset, TokenType.Whitespace);
Expand Down
25 changes: 20 additions & 5 deletions org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLCompletions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand All @@ -30,6 +31,7 @@
import org.eclipse.lsp4xml.commons.BadLocationException;
import org.eclipse.lsp4xml.commons.TextDocument;
import org.eclipse.lsp4xml.customservice.AutoCloseTagResponse;
import org.eclipse.lsp4xml.dom.DOMAttr;
import org.eclipse.lsp4xml.dom.DOMDocument;
import org.eclipse.lsp4xml.dom.DOMElement;
import org.eclipse.lsp4xml.dom.DOMNode;
Expand Down Expand Up @@ -189,7 +191,7 @@ public CompletionList doComplete(DOMDocument xmlDocument, Position position,
break;
case StartTagSelfClose:
if (offset <= scanner.getTokenEnd()) {
if (currentTag != null && currentTag.length() > 0) {
if (currentTag != null && currentTag.length() > 0 && xmlDocument.getText().charAt(offset - 1) == '>') { // if the actual character typed was '>'
collectInsideContent(completionRequest, completionResponse);
return completionResponse;
}
Expand Down Expand Up @@ -365,13 +367,25 @@ public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position posi
DOMNode node = xmlDocument.findNodeBefore(offset);
if(node.isElement() && node.getNodeName() != null) {
DOMElement element1 = (DOMElement) node;

Integer slashOffset = element1.endsWith('/', offset);
Position end = null;
if(slashOffset != null) { //The typed characted was '/'
Integer closeBracket = element1.isNextChar('>', offset); // After the slash is a close bracket
if(!element1.isInEndTag(offset) && slashOffset != null) { //The typed characted was '/'
List<DOMAttr> attrList = element1.getAttributeNodes();
if(attrList != null) {
DOMAttr lastAttr = attrList.get(attrList.size() - 1);
if(slashOffset < lastAttr.getEnd()) { //slash in attribute value
return null;
}
}
String text = xmlDocument.getText();
boolean closeBracketAfterSlash = offset < text.length() ? text.charAt(offset) == '>' : false; // After the slash is a close bracket

// Case: <a/|
if(closeBracket == null) { // no '>' after slash
// Case: <a/| ...
if(closeBracketAfterSlash == false) { // no '>' after slash
if(element1.getStartTagCloseOffset() != null) { // tag has closing '>', but slash is in incorrect area (not directly before the '>')
return null;
}
snippet = ">$0";
if(element1.hasEndTag()) { // Case: <a/| </a>
try {
Expand All @@ -383,6 +397,7 @@ public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position posi
}
else {
DOMNode nextSibling = node.getNextSibling();
//If there is text in between the tags it will skip this
if(nextSibling != null && nextSibling.isElement()){ // Case: <a/|></a>
DOMElement element2 = (DOMElement) nextSibling;
if(!element2.hasStartTag() && node.getNodeName().equals(element2.getNodeName())) {
Expand Down
20 changes: 20 additions & 0 deletions org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLCompletionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.eclipse.lsp4xml.XMLAssert.testCompletionFor;
import static org.eclipse.lsp4xml.XMLAssert.testTagCompletion;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;

import java.util.Arrays;
Expand Down Expand Up @@ -146,6 +147,19 @@ public void testAutoCloseTagCompletionWithRange() {
assertAutoCloseEndTagCompletionWithRange("<a/|></a>", ">$0", new Range(new Position(0, 3), new Position(0,8)));
assertAutoCloseEndTagCompletionWithRange("<a/| </a>", ">$0", new Range(new Position(0, 3), new Position(0,8)));
assertAutoCloseEndTagCompletionWithRange("<a> <a/|> </a> </a>", ">$0", new Range(new Position(0, 7), new Position(0,13)));
assertAutoCloseEndTagCompletionWithRange("<a var=\"asd\"/|></a>", ">$0", new Range(new Position(0, 13), new Position(0,18)));
assertAutoCloseEndTagCompletionWithRange("<a var=\"asd\" /| </a>", ">$0", new Range(new Position(0, 16), new Position(0,21)));
assertAutoCloseEndTagCompletionWithRange("<aB/|></aB>", ">$0", new Range(new Position(0, 4), new Position(0,10)));
}

@Test
public void testAutoCloseTagCompletionWithSlashAtBadLocations() {
assertAutoCloseEndTagCompletionWithRange("<a zz=\"a/|\"></a>", null, null);
assertAutoCloseEndTagCompletionWithRange("<a zz=/|\"aa\"> </a>", null, null);
assertAutoCloseEndTagCompletionWithRange("<a /| > </a>", null, null);
assertAutoCloseEndTagCompletionWithRange("<a> </a/|>", null, null);
assertAutoCloseEndTagCompletionWithRange("<a> </|a>", null, null);

}

@Test
Expand All @@ -154,6 +168,7 @@ public void testAutoCloseEnabledDisabled() throws BadLocationException {
testCompletionFor("<a><div|<a>", true, c("div", "<div></div>"));
testCompletionFor("<a> <div| <a>", false, c("div", "<div>"));
testCompletionFor("<a> <div| <a>", true, c("div", "<div></div>"));
assertAutoCloseEndTagCompletionWithRange("<a/|>Text</a>", null, null);
}

@Test
Expand Down Expand Up @@ -209,6 +224,11 @@ public void assertAutoCloseEndTagCompletionWithRange(String xmlText, String expe
fail("Couldn't get position at offset");
}
AutoCloseTagResponse response = languageService.doTagComplete(xmlDocument, position);
if(response == null) {
assertNull(expectedTextEdit);
assertNull(range);
return;
}
String completionList = response.snippet;
assertEquals(expectedTextEdit, completionList);
assertEquals(range, response.range);
Expand Down

0 comments on commit 3168b2c

Please sign in to comment.