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-lemminx#651

Signed-off-by: azerr <[email protected]>
  • Loading branch information
angelozerr committed Apr 23, 2020
1 parent 47fe693 commit c80db9e
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,23 @@ 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 end tag (eg: </a> which is not
* closed) and false otherwise.
*
* @param tagName the end tag name.
* @return true if the given element is an end tag (eg: </a> which is not
* closed) and false otherwise.
*/
public boolean isOnlyEndTag(String tagName) {
return isSameTag(tagName) && hasEndTag() && !hasStartTag();
}

/**
* 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
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

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 +221,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 +240,62 @@ 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;
}
if (node.isText()) {
DOMNode next = node.getNextSibling();
if (next == null || !next.isElement()) {
return true;
}
// emp| </employe>
DOMElement nextElement = (DOMElement) next;
if (nextElement.isOnlyEndTag(tagName)) {
return false;
}
return true;
}
if (!node.isElement()) {
return true;
}
DOMElement element = (DOMElement) node;
if (!element.isInStartTag(offset) && !element.isInEndTag(offset)) {
// completion inside element content
// <employee>|</employee>
// <employee> |</employee>
// <employee> | </employee>
return true;
}
if (element.getEnd() <= offset) {
// <employee />|
// <employee /> |
return true;
}
if (element.isClosed() && element.isSameTag(tagName)) {
// <employe|e></employee>
return false;
}
// element is not
List<DOMNode> children = element.getChildren();
for (DOMNode child : children) {
if (child.isElement()) {
DOMElement childElement = (DOMElement) child;
if (childElement.isOnlyEndTag(tagName)) {
return false;
}
}
}
return true;
}

@Override
public void onAttributeName(boolean generateValue, ICompletionRequest request, ICompletionResponse response)
throws Exception {
Expand Down Expand Up @@ -356,8 +408,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ private void collectOpenTagSuggestions(boolean hasOpenBracket, Range replaceRang
LOGGER.log(Level.SEVERE, "While performing ICompletionParticipant#onTagOpen", e);
}
}
DOMNode currentElement = completionRequest.getNode();
DOMElement parentNode = completionRequest.getParentElement();
if (parentNode != null && !parentNode.getOwnerDocument().hasGrammar()) {
// no grammar, collect similar tags from the parent node
Expand Down Expand Up @@ -522,7 +523,7 @@ private void collectOpenTagSuggestions(boolean hasOpenBracket, Range replaceRang
if (completionRequest.isCompletionSnippetsSupported()) {
xml.append("$0");
}
if (completionRequest.isAutoCloseTags()) {
if (!currentElement.isClosed() && completionRequest.isAutoCloseTags()) {
xml.append("</").append(tag).append(">");
}
}
Expand Down
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,66 @@ 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
}

@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 +1080,6 @@ public void documentationAsPlainText() throws BadLocationException {
System.lineSeparator() + //
"Source: maven-4.0.0.xsd",
MarkupKind.PLAINTEXT));

}

@Test
Expand Down

0 comments on commit c80db9e

Please sign in to comment.