Handles setting a space before a self closing tag end bracket.

Fixes eclipse-lemminx#197 Also fixed an unknown issue where the new settings would replace existing and not update the current ones. In the case of a setting not being sent from the client, the default value set on the server side would be lost in XMLFormattingOptions anymore. Signed-off-by: Nikolas Komonen <nikolaskomonen@gmail.com>
NikolasKomonen · Nov 14, 2018 · 86644a6 · 86644a6
1 parent 5e7b595
commit 86644a6
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 23 deletions.
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/XMLLanguageServer.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/XMLLanguageServer.java
@@ -123,7 +123,7 @@ public void updateSettings(Object initializationOptionsSettings) {
 			// Update format settings
 			XMLFormattingOptions formatterSettings = clientSettings.getFormat();
 			if (formatterSettings != null) {
-				xmlTextDocumentService.setSharedFormattingOptions(formatterSettings);
+				xmlTextDocumentService.getSharedFormattingOptions().merge(formatterSettings);
 			}
 
 			CompletionSettings newCompletions = clientSettings.getCompletion();

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DocumentType.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DocumentType.java
@@ -44,10 +44,8 @@ public int getEndContent() {
 		return endContent;
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * 
-	 * @see org.w3c.dom.DocumentType#getName()
+	/**
+	 * The text immediately after DOCTYPE, "<!DOCTYPE this_is_the_Name ..."
 	 */
 	@Override
 	public String getName() {

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DocumentTypeElement.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DocumentTypeElement.java
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/settings/XMLFormattingOptions.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/settings/XMLFormattingOptions.java
@@ -19,42 +19,64 @@
  * All defaults should be set here to eventually be overridden if needed.
  */
 public class XMLFormattingOptions extends FormattingOptions {
-
 	// All possible keys
 	private static final String SPLIT_ATTRIBUTES = "splitAttributes";
 	private static final String JOIN_CDATA_LINES = "joinCDATALines";
 	private static final String FORMAT_COMMENTS = "formatComments";
 	private static final String JOIN_COMMENT_LINES = "joinCommentLines";
 	private static final String JOIN_CONTENT_LINES = "joinContentLines";
 	private static final String ENABLED = "enabled";
+	private static final String SPACE_BEFORE_EMPTY_CLOSE_TAG = "spaceBeforeEmptyCloseTag";
 
 	public XMLFormattingOptions() {
 		this(false);
 	}
 
-	public XMLFormattingOptions(boolean defaultValue) {
-		if (defaultValue) {
+	/**
+	 * Create an XMLFormattingOptions instance with the option to initialize
+	 * default values for all supported settings. 
+	 */
+	public XMLFormattingOptions(boolean initializeDefaults) {
+		if (initializeDefaults) {
 			initializeDefaultSettings();
 		}
 	}
 
-	private void initializeDefaultSettings() {
+	/** 
+	 * Necessary: Initialize default values in case client does not provide one 
+	 */
+	public void initializeDefaultSettings() {
 		this.setSplitAttributes(false);
 		this.setJoinCDATALines(false);
 		this.setFormatComments(true);
 		this.setJoinCommentLines(false);
 		this.setJoinContentLines(false);
 		this.setEnabled(true);
+		this.setSpaceBeforeEmptyCloseTag(true);
 	}
 
-	public XMLFormattingOptions(int tabSize, boolean insertSpaces) {
+	public XMLFormattingOptions(int tabSize, boolean insertSpaces, boolean initializeDefaultSettings) {
 		super(tabSize, insertSpaces);
+		if(initializeDefaultSettings) {
+			initializeDefaultSettings();
+		}
 	}
 
-	public XMLFormattingOptions(FormattingOptions options) {
+	public XMLFormattingOptions(int tabSize, boolean insertSpaces) {
+		this(tabSize, insertSpaces, true);
+	}
+
+	public XMLFormattingOptions(FormattingOptions options, boolean initializeDefaultSettings) {
+		if(initializeDefaultSettings) {
+			initializeDefaultSettings();
+		}
 		merge(options);
 	}
 
+	public XMLFormattingOptions(FormattingOptions options) {
+		this(options, true);
+	}
+
 	public boolean isSplitAttributes() {
 		final Boolean value = this.getBoolean(XMLFormattingOptions.SPLIT_ATTRIBUTES);
 		if ((value != null)) {
@@ -133,9 +155,29 @@ public void setEnabled(final boolean enabled) {
 		this.putBoolean(XMLFormattingOptions.ENABLED, Boolean.valueOf(enabled));
 	}
 
+	public void setSpaceBeforeEmptyCloseTag(final boolean spaceBeforeEmptyCloseTag) {
+		this.putBoolean(XMLFormattingOptions.SPACE_BEFORE_EMPTY_CLOSE_TAG, Boolean.valueOf(spaceBeforeEmptyCloseTag));
+	}
+
+	public boolean isSpaceBeforeEmptyCloseTag() {
+		final Boolean value = this.getBoolean(XMLFormattingOptions.SPACE_BEFORE_EMPTY_CLOSE_TAG);
+		if ((value != null)) {
+			return (value).booleanValue();
+		} else {
+			return true;
+		}
+	}
+
 	public XMLFormattingOptions merge(FormattingOptions formattingOptions) {
-		formattingOptions.entrySet().stream().forEach(entry -> //
-		this.putIfAbsent(entry.getKey(), entry.getValue()) //
+		formattingOptions.entrySet().stream().forEach(entry -> {
+			String key = entry.getKey();
+			if(!key.equals("tabSize") && !key.equals("insertSpaces")) {
+				this.put(entry.getKey(), entry.getValue());	
+			} 
+			else {
+				this.putIfAbsent(entry.getKey(), entry.getValue());
+			}
+		}
 		);
 		return this;
 	}

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java
@@ -72,7 +72,10 @@ public XMLBuilder closeStartElement() {
 	}
 
 	public XMLBuilder endElement() {
-		xml.append(" />");
+		if(formattingOptions.isSpaceBeforeEmptyCloseTag()) {
+			xml.append(" ");
+		}
+		xml.append("/>");
 		return this;
 	}
 

diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java
@@ -596,6 +596,36 @@ public void testContentFormatting6() throws BadLocationException {
 		format(content, expected, formattingOptions);
 	}
 
+	@Test public void testSelfCloseTagSpace() throws BadLocationException {
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		formattingOptions.setSpaceBeforeEmptyCloseTag(true);
+
+		String content = 
+				"<a>\r" + //
+				" <b/>\r" + //
+				"</a>";
+		String expected = 
+				"<a>\r" + //
+				"  <b />\r" + //
+				"</a>";
+		format(content, expected, formattingOptions);
+	}
+
+	@Test public void testSelfCloseTagSpaceFalse() throws BadLocationException {
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		formattingOptions.setSpaceBeforeEmptyCloseTag(false);
+
+		String content = 
+				"<a>\r" + //
+				" <b/>\r" + //
+				"</a>";
+		String expected = 
+				"<a>\r" + //
+				"  <b/>\r" + //
+				"</a>";
+		format(content, expected, formattingOptions);
+	}
+
 
 
 	//-------------------------Tools-----------------------------------------

diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/settings/SettingsTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/settings/SettingsTest.java
@@ -50,7 +50,9 @@ public void initializationOptionsSettings() {
 				"			\"joinCDATALines\": true,\r\n" + //
 				"			\"formatComments\": true,\r\n" + //
 				"			\"joinCommentLines\": true\r\n" + //
-				"		}\r\n" + "	}\r\n" + "}";
+				"		}\r\n" + 
+				"	}\r\n" + 
+				"}";
 		// Emulate InitializeParams#getInitializationOptions() object created as
 		// JSONObject when XMLLanguageServer#initialize(InitializeParams params) is
 		// called
@@ -85,30 +87,31 @@ private static InitializeParams createInitializeParams(String json) {
 
 	@Test
 	public void formatSettings() {
-		XMLFormattingOptions sharedXMLFormattingOptions = new XMLFormattingOptions();
+		XMLFormattingOptions sharedXMLFormattingOptions = new XMLFormattingOptions(true);
 		sharedXMLFormattingOptions.setTabSize(10);
 		sharedXMLFormattingOptions.setInsertSpaces(true);
 		sharedXMLFormattingOptions.setJoinCommentLines(true);
 
 		// formatting options coming from request
 		FormattingOptions formattingOptions = new FormattingOptions();
 		formattingOptions.setTabSize(5);
+		formattingOptions.setInsertSpaces(false);
 
 		XMLFormattingOptions xmlFormattingOptions = new XMLFormattingOptions(formattingOptions);
 		Assert.assertEquals(5, xmlFormattingOptions.getTabSize()); // value coming from the request formattingOptions
 		Assert.assertFalse(xmlFormattingOptions.isInsertSpaces()); // formattingOptions doesn't defines insert spaces
 																	// flag
 
-		Assert.assertFalse(xmlFormattingOptions.isJoinCommentLines());
+		Assert.assertFalse(xmlFormattingOptions.isJoinCommentLines());//Since default for JoinCommentLines is False
 
 		// merge with shared sharedXMLFormattingOptions (formatting settings created in
 		// the InitializeParams
 		xmlFormattingOptions.merge(sharedXMLFormattingOptions);
-		Assert.assertEquals(5, xmlFormattingOptions.getTabSize()); // tab size is kept to 5 (and not updated with
-																	// shared value 10), because request
-																	// formattingOptions defines it.
-		Assert.assertTrue(xmlFormattingOptions.isInsertSpaces()); // insert spaces is to true (shared value), because
-																	// request formatting options doesn't define it.
+		Assert.assertEquals(5, xmlFormattingOptions.getTabSize()); // tab size is kept as 5 (and not updated with
+																   // shared value 10), because only the request's
+																   // formattingOptions object is allowed to define it.
+		Assert.assertFalse(xmlFormattingOptions.isInsertSpaces()); // insert spaces is kept as false because only the request's
+																   // formattingOptions object is allowed to define it.
 		Assert.assertTrue(xmlFormattingOptions.isJoinCommentLines());
 	}
 }
diff --git a/....lsp4xml/src/test/java/org/eclipse/lsp4xml/settings/capabilities/XMLCapabilitiesTest.java b/....lsp4xml/src/test/java/org/eclipse/lsp4xml/settings/capabilities/XMLCapabilitiesTest.java
@@ -57,7 +57,7 @@ public class XMLCapabilitiesTest {
 
 	@Before
 	public void startup() {
-		XMLFormattingOptions formattingOptions = new XMLFormattingOptions();
+		XMLFormattingOptions formattingOptions = new XMLFormattingOptions(true);
 		formattingOptions.setEnabled(true);
 
 		textDocumentService = new XMLTextDocumentService(null);