Skip to content

Commit

Permalink
Don't generate end element on apply completion if it exists
Browse files Browse the repository at this point in the history 
Fixes eclipse#651

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr
angelozerr committed Apr 23, 2020
1 parent 47fe693 commit fe26010
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 19 deletions.
51 changes: 50 additions & 1 deletion org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;

import org.eclipse.lemminx.utils.StringUtils;
Expand Down Expand Up @@ -378,7 +379,55 @@ public boolean isEndTagClosed() {
}

/**
* If Element has a closing end tag eg: <a> </a> -> true , <a> </b> -> false
* Returns true if the given element is an orphan end tag (which have not a
* start tag, eg: </a>) and false otherwise.
*
* @param tagName the end tag name.
* @return true if the given element is an orphan end tag (which have not a
* start tag, eg: </a>) and false otherwise.
*/
public boolean isOrphanEndTag(String tagName) {
return isSameTag(tagName) && hasEndTag() && !hasStartTag();
}

public DOMElement getOrphanEndElement(int offset, String tagName) {
if (getEnd() <= offset) {
// <employee />|
// <employee /> |
// <employee></employee> |
// check if next sibling node is an element like <\tagName>
return super.getOrphanEndElement(offset, tagName);
}
if (!isInStartTag(offset) && !isInEndTag(offset)) {
// completion inside element content
// <employee>|</employee>
// <employee> |</employee>
// <employee> | </employee>
return null;
}
if (isClosed() && isSameTag(tagName)) {
// <employe|e></employee>
return this;
}
// search if it exists an end tag
List<DOMNode> children = getChildren();
for (DOMNode child : children) {
if (child.isElement()) {
DOMElement childElement = (DOMElement) child;
if (childElement.isOrphanEndTag(tagName)) {
return childElement;
}
}
}
return null;
}

/**
* Returns true if element has a closing end tag (eg: <a> </a>) and false
* otherwise (eg: <a> </b>).
*
* @return true if element has a closing end tag (eg: <a> </a>) and false
* otherwise (eg: <a> </b>).
*/
@Override
public boolean isClosed() {
Expand Down
18 changes: 16 additions & 2 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,15 +375,16 @@ public String getAttribute(String name) {
/**
* Returns the attribute at the given index, the order is how the attributes
* appear in the document.
*
* @param index Starting at 0, index of attribute you want
* @return
*/
public DOMAttr getAttributeAtIndex(int index) {
if(!hasAttributes()) {
if (!hasAttributes()) {
return null;
}

if(index > attributeNodes.getLength() - 1) {
if (index > attributeNodes.getLength() - 1) {
return null;
}
return attributeNodes.get(index);
Expand Down Expand Up @@ -738,6 +739,19 @@ public DOMNode getPreviousNonTextSibling() {
return prev;
}

public DOMElement getOrphanEndElement(int offset, String tagName) {
DOMNode next = getNextSibling();
if (next == null || !next.isElement()) {
return null;
}
// emp| </employe>
DOMElement nextElement = (DOMElement) next;
if (nextElement.isOrphanEndTag(tagName)) {
return nextElement;
}
return null;
}

/*
* (non-Javadoc)
*
Expand Down
18 changes: 13 additions & 5 deletions ...lipse/lemminx/extensions/contentmodel/participants/ContentModelCompletionParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.lemminx.commons.BadLocationException;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMElement;
import org.eclipse.lemminx.dom.DOMNode;
import org.eclipse.lemminx.extensions.contentmodel.model.CMAttributeDeclaration;
import org.eclipse.lemminx.extensions.contentmodel.model.CMDocument;
import org.eclipse.lemminx.extensions.contentmodel.model.CMElementDeclaration;
Expand Down Expand Up @@ -219,11 +220,11 @@ private static void addTagName(NodeList list, Set<String> tags, ICompletionReque
}
}

private static void addCompletionItem(CMElementDeclaration elementDeclaration, DOMElement element,
private static void addCompletionItem(CMElementDeclaration elementDeclaration, DOMElement parentElement,
String defaultPrefix, boolean forceUseOfPrefix, ICompletionRequest request, ICompletionResponse response,
XMLGenerator generator, Set<String> tags) {
String prefix = forceUseOfPrefix ? defaultPrefix
: (element != null ? element.getPrefix(elementDeclaration.getNamespace()) : null);
: (parentElement != null ? parentElement.getPrefix(elementDeclaration.getNamespace()) : null);
String label = elementDeclaration.getName(prefix);
if (tags != null) {
if (tags.contains(label)) {
Expand All @@ -238,12 +239,20 @@ private static void addCompletionItem(CMElementDeclaration elementDeclaration, D
item.setKind(CompletionItemKind.Property);
MarkupContent documentation = XMLGenerator.createMarkupContent(elementDeclaration, request);
item.setDocumentation(documentation);
String xml = generator.generate(elementDeclaration, prefix);
String xml = generator.generate(elementDeclaration, prefix,
isGenerateEndTag(request.getNode(), request.getOffset(), label));
item.setTextEdit(new TextEdit(request.getReplaceRange(), xml));
item.setInsertTextFormat(InsertTextFormat.Snippet);
response.addCompletionItem(item, true);
}

private static boolean isGenerateEndTag(DOMNode node, int offset, String tagName) {
if (node == null) {
return true;
}
return node.getOrphanEndElement(offset, tagName) == null;
}

@Override
public void onAttributeName(boolean generateValue, ICompletionRequest request, ICompletionResponse response)
throws Exception {
Expand Down Expand Up @@ -356,8 +365,7 @@ public void onXMLContent(ICompletionRequest request, ICompletionResponse respons
end = document.positionAt(endOffset);
}
int completionOffset = request.getOffset();
String tokenStart = StringUtils.getWhitespaces(document.getText(), startOffset,
completionOffset);
String tokenStart = StringUtils.getWhitespaces(document.getText(), startOffset, completionOffset);
Range fullRange = new Range(start, end);
values.forEach(value -> {
CompletionItem item = new CompletionItem();
Expand Down
13 changes: 5 additions & 8 deletions ...lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/utils/XMLGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,31 +65,28 @@ public XMLGenerator(XMLFormattingOptions formattingOptions, boolean autoCloseTag
this.canSupportSnippets = canSupportSnippets;
}

public String generate(CMElementDeclaration elementDeclaration) {
return generate(elementDeclaration, null);
}

/**
* Returns the XML generated from the given element declaration.
*
* @param elementDeclaration
* @param prefix
* @return the XML generated from the given element declaration.
*/
public String generate(CMElementDeclaration elementDeclaration, String prefix) {
public String generate(CMElementDeclaration elementDeclaration, String prefix, boolean generateEndTag) {
XMLBuilder xml = new XMLBuilder(formattingOptions, whitespacesIndent, lineDelimiter);
generate(elementDeclaration, prefix, 0, 0, xml, new ArrayList<CMElementDeclaration>());
generate(elementDeclaration, prefix, generateEndTag, 0, 0, xml, new ArrayList<CMElementDeclaration>());
if (canSupportSnippets) {
xml.addContent(SnippetsBuilder.tabstops(0)); // "$0"
}
return xml.toString();
}

private int generate(CMElementDeclaration elementDeclaration, String prefix, int level, int snippetIndex,
private int generate(CMElementDeclaration elementDeclaration, String prefix, boolean generateEndTag, int level, int snippetIndex,
XMLBuilder xml, List<CMElementDeclaration> generatedElements) {
if (generatedElements.contains(elementDeclaration)) {
return snippetIndex;
}
boolean autoCloseTags = this.autoCloseTags && generateEndTag;
generatedElements.add(elementDeclaration);
if (level > 0) {
xml.linefeed();
Expand All @@ -106,7 +103,7 @@ private int generate(CMElementDeclaration elementDeclaration, String prefix, int
if ((level > maxLevel)) {
level++;
for (CMElementDeclaration child : children) {
snippetIndex = generate(child, prefix, level, snippetIndex, xml, generatedElements);
snippetIndex = generate(child, prefix, true, level, snippetIndex, xml, generatedElements);
}
level--;
xml.linefeed();
Expand Down
14 changes: 13 additions & 1 deletion org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ private void collectOpenTagSuggestions(boolean hasOpenBracket, Range replaceRang
if (completionRequest.isCompletionSnippetsSupported()) {
xml.append("$0");
}
if (completionRequest.isAutoCloseTags()) {
if (isGenerateEndTag(completionRequest, tag)) {
xml.append("</").append(tag).append(">");
}
}
Expand All @@ -535,6 +535,18 @@ private void collectOpenTagSuggestions(boolean hasOpenBracket, Range replaceRang
}
}

private static boolean isGenerateEndTag(CompletionRequest completionRequest, String tagName) {
if (!completionRequest.isAutoCloseTags()) {
return false;
}
DOMNode node = completionRequest.getNode();
if (node == null) {
return true;
}
int offset = completionRequest.getOffset();
return node.getOrphanEndElement(offset, tagName) == null;
}

/**
* Collect xml prolog completions.
*
Expand Down
124 changes: 122 additions & 2 deletions ...t/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaCompletionExtensionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ public void noNamespaceSchemaLocationCompletion() throws BadLocationException {
" <View><Name /><|";
// Completion only with Name
XMLAssert.testCompletionFor(xml, null, "src/test/resources/Format.xml", 5 + 2 /* CDATA and Comments */,
c("OutOfBand", "<OutOfBand>false</OutOfBand>"), c("ViewSelectedBy", "<ViewSelectedBy></ViewSelectedBy>"),
c("OutOfBand", "<OutOfBand>false</OutOfBand>"),
c("ViewSelectedBy", "<ViewSelectedBy></ViewSelectedBy>"),
c("End with '</Configuration>'", "/Configuration>"),
c("End with '</ViewDefinitions>'", "/ViewDefinitions>"), c("End with '</View>'", "/View>"));
}
Expand Down Expand Up @@ -1004,6 +1005,126 @@ public void tag() throws BadLocationException {

}

@Test
public void generateOnlyStartElementOnText() throws BadLocationException {
// </employee> already exists, completion must generate only <employee>

// completion on empty text
String xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 2, //
c("member", te(2, 0, 2, 0, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 0, 2, 0, "<employee>$1$0"), "employee")); // <-- here only start employee is
// generated

// completion on text
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"em|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 2, //
c("member", te(2, 0, 2, 2, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 0, 2, 2, "<employee>$1$0"), "employee")); // <-- here only start employee is
// generated

// completion on text inside element
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee>|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 3, //
c("person", te(2, 10, 2, 10, "<person>$1</person>$0"), "person"),
c("member", te(2, 10, 2, 10, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 10, 2, 10, "<employee>$1</employee>$0"), "employee"));

// completion on text inside element with text content
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee> |</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 3, //
c("person", te(2, 11, 2, 11, "<person>$1</person>$0"), "person"),
c("member", te(2, 11, 2, 11, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 11, 2, 11, "<employee>$1</employee>$0"), "employee"));

// completion on text inside element with text content
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee>| </employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 3, //
c("person", te(2, 10, 2, 10, "<person>$1</person>$0"), "person"),
c("member", te(2, 10, 2, 10, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 10, 2, 10, "<employee>$1</employee>$0"), "employee"));

// completion on text inside element with text content
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee> | </employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 3, //
c("person", te(2, 11, 2, 11, "<person>$1</person>$0"), "person"),
c("member", te(2, 11, 2, 11, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 11, 2, 11, "<employee>$1</employee>$0"), "employee"));

// completion on text inside element with text content
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee> | </employee></employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 3, //
c("person", te(2, 11, 2, 11, "<person>$1</person>$0"), "person"),
c("member", te(2, 11, 2, 11, "<member>$1</member>$0"), "member"), //
c("employee", te(2, 11, 2, 11, "<employee>$1</employee>$0"), "employee"));

// completion on text inside element with text content
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee></employee>\r\n" + //
"|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", null, //
c("member", te(3, 0, 3, 0, "<member>$1</member>$0"), "member"), //
c("employee", te(3, 0, 3, 0, "<employee>$1$0"), "employee"));
}

@Test
public void generateOnlyStartElementOnElement() throws BadLocationException {
// </employee> already exists, completion must generate only <employee>

// completion on start tag
String xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 2, //
c("member", te(2, 0, 2, 1, "<member>$1</member>$0"), "<member"), //
c("employee", te(2, 0, 2, 1, "<employee>$1$0"), "<employee")); // <-- here only start employee is
// generated

// completion on start tag element
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<em|</employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 2, //
c("member", te(2, 0, 2, 3, "<member>$1</member>$0"), "<member"), //
c("employee", te(2, 0, 2, 3, "<employee>$1$0"), "<employee")); // <-- here only start employee is
// generated

// completion inside tag element
xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
"<person xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"xsd/choice.xsd\">\r\n"
+ //
"<employee|></employee>";
testCompletionSnippetSupporytFor(xml, "src/test/resources/choice.xml", 2, //
c("member", te(2, 0, 2, 10, "<member>$1</member>$0"), "<member"), //
c("employee", te(2, 0, 2, 10, "<employee>$1$0"), "<employee")); // <-- here only start employee is
// generated
}

@Test
public void documentationAsPlainText() throws BadLocationException {
String xml = "<project xmlns=\"http://maven.apache.org/POM/4.0.0\"\r\n" + //
Expand All @@ -1019,7 +1140,6 @@ public void documentationAsPlainText() throws BadLocationException {
System.lineSeparator() + //
"Source: maven-4.0.0.xsd",
MarkupKind.PLAINTEXT));

}

@Test
Expand Down

0 comments on commit fe26010

Please sign in to comment.