Inline completion feature#1784
Conversation
|
It looks promising! Please create the vscode-xml PR and reference it in this PR. |
| /** | ||
| * Helper method to extract string from Either<String, StringValue> | ||
| */ | ||
| private String getInsertTextAsString(Either<String, org.eclipse.lsp4j.StringValue> insertText) { |
There was a problem hiding this comment.
Should we have this method in an utility class?
| * @return the inline completion list | ||
| * @throws BadLocationException if the position is invalid | ||
| */ | ||
| private InlineCompletionList testInlineCompletionFor(String xml) throws BadLocationException { |
There was a problem hiding this comment.
Instead of having this test:
String xml = "<root><child>|</child></root>";
InlineCompletionList result = testInlineCompletionFor(xml);
assertNotNull(result);
assertEquals(1, result.getItems().size());
assertEquals("nested", getInsertTextAsString(result.getItems().get(0).getInsertText()));You should have this test
String xml = "<root><child>|</child></root>";
testInlineCompletionFor(xml, "nested");by using XMLAssert#testInlineCompletionFor
| @Test | ||
| public void testCloseRootElement() throws BadLocationException { | ||
| String xml = "<root>|"; | ||
| XMLAssert.testInlineCompletionFor(new XMLLanguageService(), xml, null, "</root>"); |
There was a problem hiding this comment.
Test must be easy to read, instead of
XMLAssert.testInlineCompletionFor(new XMLLanguageService(), xml, null, "</root>");
it should be testInlineCompletionFor(xml, "</root>");
PLease:
- use import static
- define a new testInlineCompletionFor(String, String) which call your existing testInlineCompletionFor.
|
|
||
| @Test | ||
| public void testCloseRootElement() throws BadLocationException { | ||
| String xml = "<root>|"; |
There was a problem hiding this comment.
XMLCloseTagInlineCompletion only works after closing tags
There was a problem hiding this comment.
@venmanyarun I have noticed that, see my comment #1784 (comment)
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * XML inline completion service tests |
There was a problem hiding this comment.
XML inline completion service tests with custom participants.
| * @return the inline completion list | ||
| * @throws BadLocationException if the position is invalid | ||
| */ | ||
| private InlineCompletionList testInlineCompletionFor(String xml) throws BadLocationException { |
There was a problem hiding this comment.
Don't redefine testInlineCompletionFor but use your XMLAssert.testInlineCompletionFor
There was a problem hiding this comment.
Updated all calls to use XMLAssert.testInlineCompletionFor implementations
| * @param insertText the insert text as Either | ||
| * @return the string value or null | ||
| */ | ||
| public static String getInsertTextAsString(Either<String, StringValue> insertText) { |
There was a problem hiding this comment.
Sorry I though this utility class could be used in the core (not in the test).
Please remove this method and define it as private in XMLAssert.
|
@datho7561 @fbricon this PR provides inline completion with participant API and one impementation exists to show close tag: If you have So I wonder if we should provide inline completion with also this usecase What do you think about that? |
keeping tag autoclose should be fine Both features should have the ability to be toggled |
Thanks for your feedback @fbricon and do you think we should also cover this usecase |
it depends. Adding |
|
The PR is very good and we can improve inline completion features with another PR. Let's merge it.
I agree! I am not sure that inline completion can support snippet syntax. |
|
Great job @venmanyarun ! |
3 participants
Fixes #1780
This PR provides InlineCompletion participant API and it provides a basic inline completion to close the tag name. This inline support works only when you have this usecase
<name>|Tested in vscode-xml with redhat-developer/vscode-xml#1151