From bbe8fb05b6717aaf4d032fd0c8afff21e807adf7 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Tue, 23 Mar 2021 15:06:59 -0400 Subject: [PATCH] New setting to prevent auto self close destroying content Adds the setting `xml.completion.autoCloseRemovesContent`, which defaults to `true` (to preserve existing behaviour). When the setting is set to `false`, and the auto close tags funcitonality is enabled, `` and `` won't be deleted when an `/` is inserted at the pipe: ```xml ``` Closes redhat-developer/vscode-xml#440 Signed-off-by: David Thompson --- .../eclipse/lemminx/XMLLanguageServer.java | 2 +- .../lemminx/services/XMLCompletions.java | 14 ++--- .../lemminx/services/XMLLanguageService.java | 15 +++--- .../settings/XMLCompletionSettings.java | 34 +++++++++--- .../java/org/eclipse/lemminx/XMLAssert.java | 27 +++++++++- .../lemminx/services/XMLCompletionTest.java | 54 ++++++++++++++++++- 6 files changed, 120 insertions(+), 26 deletions(-) diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java index 97915ac1b..ee5b5f87c 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLLanguageServer.java @@ -280,7 +280,7 @@ public long getParentProcessId() { @Override public CompletableFuture closeTag(TextDocumentPositionParams params) { return xmlTextDocumentService.computeDOMAsync(params.getTextDocument(), (cancelChecker, xmlDocument) -> { - return getXMLLanguageService().doAutoClose(xmlDocument, params.getPosition(), cancelChecker); + return getXMLLanguageService().doAutoClose(xmlDocument, params.getPosition(), getSettings().getCompletionSettings(), cancelChecker); }); } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java index 1166bf714..6f5b7843e 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLCompletions.java @@ -44,6 +44,7 @@ import org.eclipse.lemminx.services.extensions.XMLExtensionsRegistry; import org.eclipse.lemminx.services.snippets.IXMLSnippetContext; import org.eclipse.lemminx.settings.SharedSettings; +import org.eclipse.lemminx.settings.XMLCompletionSettings; import org.eclipse.lemminx.utils.StringUtils; import org.eclipse.lemminx.utils.XMLPositionUtility; import org.eclipse.lsp4j.CompletionItem; @@ -467,7 +468,7 @@ private boolean isBalanced(DOMNode node) { return true; } - public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position position, CancelChecker cancelChecker) { + public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position position, XMLCompletionSettings completionSettings, CancelChecker cancelChecker) { int offset; try { offset = xmlDocument.offsetAt(position); @@ -521,13 +522,8 @@ public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position posi } } String text = xmlDocument.getText(); - boolean closeBracketAfterSlash = offset < text.length() ? text.charAt(offset) == '>' : false; // After - // the - // slash - // is - // a - // close - // bracket + // After the slash is a close bracket + boolean closeBracketAfterSlash = offset < text.length() ? text.charAt(offset) == '>' : false; // Case: ' after slash @@ -536,7 +532,7 @@ public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position posi return null; } snippet = ">$0"; - if (element1.hasEndTag()) { // Case: + if (element1.hasEndTag() && completionSettings.isAutoCloseRemovesContent()) { // Case: try { end = xmlDocument.positionAt(element1.getEnd()); } catch (BadLocationException e) { diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java index a4eb99402..e05942ee0 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java @@ -27,6 +27,7 @@ import org.eclipse.lemminx.services.extensions.XMLExtensionsRegistry; import org.eclipse.lemminx.settings.SharedSettings; import org.eclipse.lemminx.settings.XMLCodeLensSettings; +import org.eclipse.lemminx.settings.XMLCompletionSettings; import org.eclipse.lemminx.settings.XMLFoldingSettings; import org.eclipse.lemminx.settings.XMLSymbolSettings; import org.eclipse.lemminx.uriresolver.CacheResourceDownloadingException; @@ -156,7 +157,7 @@ public Hover doHover(DOMDocument xmlDocument, Position position, SharedSettings return hover.doHover(xmlDocument, position, sharedSettings, cancelChecker); } - public List doDiagnostics(DOMDocument xmlDocument, + public List doDiagnostics(DOMDocument xmlDocument, XMLValidationSettings validationSettings, CancelChecker cancelChecker) { return diagnostics.doDiagnostics(xmlDocument, validationSettings, cancelChecker); } @@ -257,22 +258,22 @@ public List doCodeActions(CodeActionContext context, Range range, DO return codeActions.doCodeActions(context, range, document, sharedSettings); } - public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position position) { - return doTagComplete(xmlDocument, position, NULL_CHECKER); + public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, XMLCompletionSettings completionSettings, Position position) { + return doTagComplete(xmlDocument, position, completionSettings, NULL_CHECKER); } - public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position position, CancelChecker cancelChecker) { - return completions.doTagComplete(xmlDocument, position, cancelChecker); + public AutoCloseTagResponse doTagComplete(DOMDocument xmlDocument, Position position, XMLCompletionSettings completionSettings, CancelChecker cancelChecker) { + return completions.doTagComplete(xmlDocument, position, completionSettings, cancelChecker); } - public AutoCloseTagResponse doAutoClose(DOMDocument xmlDocument, Position position, CancelChecker cancelChecker) { + public AutoCloseTagResponse doAutoClose(DOMDocument xmlDocument, Position position, XMLCompletionSettings completionSettings, CancelChecker cancelChecker) { try { int offset = xmlDocument.offsetAt(position); String text = xmlDocument.getText(); if (offset > 0) { char c = text.charAt(offset - 1); if (c == '>' || c == '/') { - return doTagComplete(xmlDocument, position, cancelChecker); + return doTagComplete(xmlDocument, position, completionSettings, cancelChecker); } } return null; diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLCompletionSettings.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLCompletionSettings.java index 9d661596a..f816c5915 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLCompletionSettings.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLCompletionSettings.java @@ -24,12 +24,15 @@ public class XMLCompletionSettings { private boolean autoCloseTags; - public XMLCompletionSettings(boolean autoCloseTags) { + private boolean autoCloseRemovesContent; + + public XMLCompletionSettings(boolean autoCloseTags, boolean autoCloseRemovesContent) { this.autoCloseTags = autoCloseTags; + this.autoCloseRemovesContent = autoCloseRemovesContent; } public XMLCompletionSettings() { - this(true); + this(true, true); } public void setCapabilities(CompletionCapabilities completionCapabilities) { @@ -42,7 +45,7 @@ public CompletionCapabilities getCompletionCapabilities() { /** * Tag should be autoclosed with an end tag. - * + * * @param autoCloseTags */ public void setAutoCloseTags(boolean autoCloseTags) { @@ -51,17 +54,35 @@ public void setAutoCloseTags(boolean autoCloseTags) { /** * If tag should be autoclosed with an end tag. - * + * * @return */ public boolean isAutoCloseTags() { return autoCloseTags; } + /** + * If turning a start tag into a self closing tag should remove the content of the element + * + * @param autoCloseRemovesContent + */ + public void setAutoCloseRemovesContent(boolean autoCloseRemovesContent) { + this.autoCloseRemovesContent = autoCloseRemovesContent; + } + + /** + * Returns true if turning a start tag into a self closing tag should remove the content of the element and false otherwise + * + * @return true if turning a start tag into a self closing tag should remove the content of the element and false otherwise + */ + public boolean isAutoCloseRemovesContent() { + return autoCloseRemovesContent; + } + /** * Returns true if the client support snippet and * false otherwise. - * + * * @return true if the client support snippet and * false otherwise. */ @@ -74,10 +95,11 @@ public boolean isCompletionSnippetsSupported() { /** * Merge only the given completion settings (and not the capability) in the * settings. - * + * * @param newCompletion the new settings to merge. */ public void merge(XMLCompletionSettings newCompletion) { this.setAutoCloseTags(newCompletion.isAutoCloseTags()); + this.setAutoCloseRemovesContent(newCompletion.isAutoCloseRemovesContent()); } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java index 8127e00f2..6784c9ce8 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java @@ -16,6 +16,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertIterableEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -303,6 +304,10 @@ public static CompletionItem c(String label, String newText, Range range, String } public static void testTagCompletion(String value, String expected) throws BadLocationException { + testTagCompletion(value, expected, new SharedSettings()); + } + + public static void testTagCompletion(String value, String expected, SharedSettings settings) throws BadLocationException { int offset = value.indexOf('|'); value = value.substring(0, offset) + value.substring(offset + 1); @@ -312,7 +317,7 @@ public static void testTagCompletion(String value, String expected) throws BadLo Position position = document.positionAt(offset); DOMDocument htmlDoc = DOMParser.getInstance().parse(document, ls.getResolverExtensionManager()); - AutoCloseTagResponse response = ls.doTagComplete(htmlDoc, position); + AutoCloseTagResponse response = ls.doTagComplete(htmlDoc, settings.getCompletionSettings(), position); if (expected == null) { assertNull(response); return; @@ -321,6 +326,26 @@ public static void testTagCompletion(String value, String expected) throws BadLo assertEquals(expected, actual); } + public static void testTagCompletion(String value, AutoCloseTagResponse expected, SharedSettings settings) throws BadLocationException { + int offset = value.indexOf('|'); + value = value.substring(0, offset) + value.substring(offset + 1); + + XMLLanguageService ls = new XMLLanguageService(); + + TextDocument document = new TextDocument(value, "test://test/test.html"); + Position position = document.positionAt(offset); + DOMDocument htmlDoc = DOMParser.getInstance().parse(document, ls.getResolverExtensionManager()); + + AutoCloseTagResponse actual = ls.doTagComplete(htmlDoc, settings.getCompletionSettings(), position); + if (expected == null) { + assertNull(actual); + return; + } + assertNotNull(actual); + assertEquals(expected.snippet, actual.snippet); + assertEquals(expected.range, actual.range); + } + // ------------------- Diagnostics assert public static void testDiagnosticsFor(String xml, Diagnostic... expected) { diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLCompletionTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLCompletionTest.java index 2514751e3..e7e5865c1 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLCompletionTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLCompletionTest.java @@ -25,6 +25,7 @@ import java.util.Arrays; import java.util.List; +import org.eclipse.lemminx.XMLAssert; import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.customservice.AutoCloseTagResponse; import org.eclipse.lemminx.dom.DOMDocument; @@ -80,7 +81,7 @@ public void successfulEndTagCompletion() throws BadLocationException { @Test public void successfulEndTagCompletionWithIndent() throws BadLocationException { - + testCompletionFor("'", "/a>", r(0, 4, 0, 5), "/a>")); testCompletionFor(" \r\n" + // @@ -185,6 +186,51 @@ public void testAutoCloseEnabledDisabled() throws BadLocationException { assertAutoCloseEndTagCompletionWithRange("Text", null, null); } + @Test + public void testAutoCloseTagCompletionRemovesContent() throws BadLocationException { + SharedSettings settings = new SharedSettings(); + settings.getCompletionSettings().setAutoCloseTags(true); + settings.getCompletionSettings().setAutoCloseRemovesContent(true); + + + String value = // + "\n" + // + ""; + AutoCloseTagResponse closeTagResponse = new AutoCloseTagResponse(">$0", r(0, 3, 2, 4)); + + XMLAssert.testTagCompletion(value, closeTagResponse, settings); + } + + @Test + public void testAutoCloseTagCompletionDoesntRemoveContent() throws BadLocationException { + SharedSettings settings = new SharedSettings(); + settings.getCompletionSettings().setAutoCloseTags(true); + settings.getCompletionSettings().setAutoCloseRemovesContent(false); + + String value = // + "\n" + // + ""; + AutoCloseTagResponse closeTagResponse = new AutoCloseTagResponse(">$0"); + + XMLAssert.testTagCompletion(value, closeTagResponse, settings); + } + + @Test + public void testAutoCloseTagCompletionWithLeadingTextContent() throws BadLocationException { + SharedSettings settings = new SharedSettings(); + settings.getCompletionSettings().setAutoCloseTags(true); + settings.getCompletionSettings().setAutoCloseRemovesContent(true); + + String value = // + ""; + + XMLAssert.testTagCompletion(value, (AutoCloseTagResponse) null, settings); + } + @Test public void testnoCDATANPE() { try { @@ -229,6 +275,10 @@ public void assertAutoCloseEndTagCompletion(String xmlText, String expectedTextE } public void assertAutoCloseEndTagCompletionWithRange(String xmlText, String expectedTextEdit, Range range) { + assertAutoCloseEndTagCompletionWithRange(xmlText, expectedTextEdit, range, new SharedSettings()); + } + + public void assertAutoCloseEndTagCompletionWithRange(String xmlText, String expectedTextEdit, Range range, SharedSettings settings) { int offset = getOffset(xmlText); DOMDocument xmlDocument = initializeXMLDocument(xmlText, offset); Position position = null; @@ -237,7 +287,7 @@ public void assertAutoCloseEndTagCompletionWithRange(String xmlText, String expe } catch (Exception e) { fail("Couldn't get position at offset"); } - AutoCloseTagResponse response = languageService.doTagComplete(xmlDocument, position); + AutoCloseTagResponse response = languageService.doTagComplete(xmlDocument, settings.getCompletionSettings(), position); if (response == null) { assertNull(expectedTextEdit); assertNull(range);