Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Working Completions with Testing #20

Merged
merged 2 commits into from
Jul 23, 2018

Conversation

NikolasKomonen
Copy link
Contributor

@NikolasKomonen NikolasKomonen commented Jul 17, 2018

Current Completions include the Closing End Tag completion and also Tag Name suggestions with closing.

Signed-off-by: Nikolas Komonen <nikolaskomonen@gmail.com>
Signed-off-by: Nikolas Komonen <nikolaskomonen@gmail.com>
@@ -108,6 +118,30 @@ public XMLDocument parse(String text, String uri, boolean full) {
}
break;
}

case CDATATagOpen: {
Node cdataNode = new Node(scanner.getTokenOffset(), text.length(), new ArrayList<>(), curr, xmlDocument);//TODO: might need arraylist
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why that TODO comment?

@@ -183,7 +189,7 @@ private CompletionList collectAutoCloseTagSuggestion(int tagCloseEnd, String tag
item.setLabel("</" + tag + ">");
item.setKind(CompletionItemKind.Property);
item.setFilterText("</" + tag + ">");
item.setTextEdit(new TextEdit(new Range(pos, pos), "$0</" + tag + ">"));
item.setTextEdit(new TextEdit(new Range(pos, pos), "$0</" + tag + ">")); //"\n\t$0\n</" + tag + ">"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be careful, not all clients support snippets. You should open a ticket to not return $number placeholders if client capabilities do not support snippets

XMLDocument xmlDocument = completionRequest.getXMLDocument();
List<CompletionItem> list= new ArrayList<CompletionItem>();
if(xmlDocument.children.size() > 0){
Position start,end;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 declaration / line

XMLDocument xmlDocument = completionRequest.getXMLDocument();
List<CompletionItem> list= new ArrayList<CompletionItem>();
if(xmlDocument.children.size() > 0){
Position start,end;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 declaration / line

//Tags that look like '<{CHARS}'
private CompletionList collectOpenTagSuggestions(int tokenOffset, int tokenEnd, String tag) {
XMLDocument xmlDocument = completionRequest.getXMLDocument();
List<CompletionItem> list= new ArrayList<CompletionItem>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare that list right before using it (L310), else it might be useless resource allocation

start = xmlDocument.positionAt(tokenOffset);
end = xmlDocument.positionAt(tokenEnd);

} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to log the error

// TODO Auto-generated method stub
return null;
XMLDocument xmlDocument = completionRequest.getXMLDocument();
List<CompletionItem> list= new ArrayList<CompletionItem>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare that list right before using it (L336), else it might be useless resource allocation

@@ -308,6 +352,53 @@ private static int scanNextForEndPos(int offset, Scanner scanner, TokenType next
return offset;
}

private void collectAllTags(List<CompletionItem> list, Node node, Range range, XMLDocument xmlDocument) {
if(node.tag == null) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always surround with {}

private void collectAllTags(List<CompletionItem> list, Node node, Range range, XMLDocument xmlDocument) {
if(node.tag == null) return;
CompletionItem item = createCompletionItem(node.tag, range);
if(!list.contains(item)) list.add(item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always surround with {}

if(node.tag != null) {
if(node.tag.regionMatches(0, tag, 0, len)){
CompletionItem item = createCompletionItem(node.tag, range);
if(!list.contains(item)) list.add(item);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always surround with {}

item.setKind(CompletionItemKind.Property);
item.setFilterText("![CDATA[]]");
item.setTextEdit(new TextEdit(range, "![CDATA[$0]]>"));
item.setInsertTextFormat(InsertTextFormat.Snippet);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, need to eventually check for snippet support (in a future commit)

tempRange = new Range(startPos, endPos);

} catch (BadLocationException e) {
e.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a logger to trace errors

}

@Test
public void testSelfClosingCompex() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compex?

}

@Test
public void testEmptyTagT() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testEmptyTag

@Test
public void unneededEndTagCompletion() {
assertEndTagCompletion("<a><a>|</a>", 6, "$0</a>");
assertEndTagCompletion("<a><b>|</b></a>", 6, "$0</b>");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we avoid completing with </b> here, as that'd would unbalance the xml document?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this example, , the parser considers the 2nd and 3rd as a complete element and the 1st tag as incomplete so its hard to determine with this current implementation which tag is incomplete.

@NikolasKomonen NikolasKomonen merged commit 2d8eb4b into eclipse-lemminx:master Jul 23, 2018
@NikolasKomonen NikolasKomonen deleted the Completions branch August 24, 2018 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants