[203252] [formatting] JSP formatter makes code less readable

4 files changed, 186 insertions, 11 deletions

diff --git a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormatter.java b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormatter.java
index 3642881354..99384cd3f8 100644
--- a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormatter.java
+++ b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormatter.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2007 IBM Corporation and others.
+ * Copyright (c) 2004, 2010 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  * 
  * Contributors:
  *     IBM Corporation - initial API and implementation
+ *     Genuitec - fix for bug #203252
  *******************************************************************************/
 package org.eclipse.wst.html.core.internal.format;
 
@@ -37,10 +38,17 @@ import org.w3c.dom.Text;
 public class HTMLFormatter implements IStructuredFormatter {
 
 	private static final String HTML_NAME = "html";//$NON-NLS-1$
+	private static final String HEAD_NAME = "head"; //$NON-NLS-1$
 	private static final String BODY_NAME = "BODY";//$NON-NLS-1$
 	// hidden jsp logic that should be removed when jsp formatter is created
 	private static final String JSP = "jsp";//$NON-NLS-1$
 
+	private HTMLFormattingUtil formattingUtil;
+
+	public HTMLFormatter() {
+		formattingUtil = new HTMLFormattingUtil();
+	}
+
 	/**
 	 */
 	protected void addWidth(HTMLFormatContraints contraints, int width) {
@@ -164,6 +172,11 @@ public class HTMLFormatter implements IStructuredFormatter {
 			if (decl != null) {
 				if (decl.getContentType() == CMElementDeclaration.ELEMENT)
 					return true;
+                // causes all closing tags to wrap to a new line
+                boolean allowsText = decl.getContentType() == CMElementDeclaration.MIXED
+                        || decl.getContentType() == CMElementDeclaration.PCDATA;
+                if (allowsNewlineAfter(allowsText, node, element))
+                    return true;
 				String tagName = element.getTagName();
 				// special for direct children under BODY
 				if (tagName != null && tagName.equalsIgnoreCase(BODY_NAME))
@@ -271,19 +284,12 @@ public class HTMLFormatter implements IStructuredFormatter {
 
 			CMElementDeclaration decl = getElementDeclaration(element);
 			if (decl != null) {
-				if (decl.getContentType() == CMElementDeclaration.ELEMENT)
-					return true;
-				String tagName = element.getTagName();
-				// special for direct children under BODY
-				if (tagName != null && tagName.equalsIgnoreCase(BODY_NAME))
-					return true;
+			    return allowNewlineBefore(node, element);
 			}
 		}
 
 		if (node.getNodeType() == Node.ELEMENT_NODE) {
-			CMElementDeclaration decl = getElementDeclaration((Element) node);
-			if (canInsertBreakBefore(decl))
-				return true;
+			return true;
 		}
 		if (prev != null && prev.getNodeType() == Node.ELEMENT_NODE) {
 			CMElementDeclaration decl = getElementDeclaration((Element) prev);
@@ -429,10 +435,15 @@ public class HTMLFormatter implements IStructuredFormatter {
 					}
 					continue;
 				}
+				else {
+					String localName = element.getLocalName();
+					if (HTML_NAME.equalsIgnoreCase(localName) || HEAD_NAME.equalsIgnoreCase(localName))
+						break;
+				}
 
 				CMElementDeclaration decl = getElementDeclaration(element);
 				if (decl != null && decl.supports(HTMLCMProperties.SHOULD_INDENT_CHILD_SOURCE)) {
-					boolean shouldIndent = ((Boolean) decl.getProperty(HTMLCMProperties.SHOULD_INDENT_CHILD_SOURCE)).booleanValue();
+					boolean shouldIndent = isIdentable(node, parent); 
 					if (shouldIndent)
 						buffer.append(indent);
 				}
@@ -746,4 +757,68 @@ public class HTMLFormatter implements IStructuredFormatter {
 	public void setProgressMonitor(IProgressMonitor progressMonitor) {
 		fProgressMonitor = progressMonitor;
 	}
+
+    /* Check to see if current text Node is a child of an inline element. */
+    public boolean isInlinableTextNode(Node theNode, Element theParentElement) {
+        return formattingUtil.isInline(theParentElement) && 
+               theNode.getNodeType() == Node.TEXT_NODE;
+    }
+    
+    public boolean allowsNewlineAfter(boolean theBool, Node theNode, Element theParentElement) {
+        boolean result = theBool;
+        if ((theNode.getNodeType() == Node.TEXT_NODE) && formattingUtil.isInline(theParentElement)) {
+            result = false;
+        } else if (theNode.getNodeType() == Node.ELEMENT_NODE
+                && formattingUtil.isInline(theNode.getNextSibling())) {
+            result = false;
+        }
+        else if (theNode.getNodeType() == Node.TEXT_NODE) {
+        	Node next = theNode.getNextSibling();
+        	if (next != null && formattingUtil.isInline(next)) {
+        		result = false;
+        	}
+        }
+        return result;
+    }
+
+    public boolean allowNewlineBefore(Node theNode) {
+        if (theNode.getNodeType() != Node.TEXT_NODE &&
+            theNode.getNodeType() != Node.ELEMENT_NODE) return false;
+        return (formattingUtil.isInline(theNode.getParentNode()) ||
+                        formattingUtil.isInline(theNode.getPreviousSibling()));        
+    }
+    
+    public boolean allowNewlineBefore(Node theNode, Element theParentElement) {
+        boolean result = true;
+        /* The calling method canInsertBreakBefore is checking if you can 
+         * insert a line break after the text node in the parentElement.  We 
+         * need to check for the case with inline element because we don't want to 
+         * break before the closing </tag> */
+        if (isInlinableTextNode(theNode, theParentElement)) {
+            result = false;
+        /* Check to see if we need to not break the line because we are
+         * a child of a inline element or a next sibling to an inline element*/
+        } else if (allowNewlineBefore(theNode)) {
+            result = false;
+        }
+        return result;
+    }
+
+    public boolean isIdentable(Node theNode, Node theParent) {
+        boolean result = true;
+        /* The first 2 cases where we don't want to break/indent or if the
+         * node is a inlineText ELement or if we should skip it before its parent
+         * is an inlineText element.  
+         * The last check is to make sure that the parent is actually the parent
+         * of the node.  This method is called when the formatter is formatting
+         * the startTag and the wrap margin causes attributes to be indents on
+         * mulitple lines.  In this case where the parentNode doesn't match
+         * theParent argument, we can allow the indent. */
+        if (formattingUtil.isInline(theNode) && 
+        		formattingUtil.shouldSkipIndentForNode(theNode) &&
+                theParent == theNode.getParentNode()) {
+            result = false;
+        }
+        return result;
+    }
 }
\ No newline at end of file
diff --git a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormattingUtil.java b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormattingUtil.java
new file mode 100644
index 0000000000..9a2b000632
--- /dev/null
+++ b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/format/HTMLFormattingUtil.java
@@ -0,0 +1,90 @@
+/*******************************************************************************
+ * Copyright (c) 2010 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ * 
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.wst.html.core.internal.format;
+
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
+import org.eclipse.wst.html.core.internal.HTMLCorePlugin;
+import org.eclipse.wst.html.core.internal.preferences.HTMLCorePreferenceNames;
+import org.w3c.dom.Node;
+
+import com.ibm.icu.util.StringTokenizer;
+
+public class HTMLFormattingUtil {
+
+	private Set fInlineElements;
+
+	public HTMLFormattingUtil() {
+		fInlineElements = getInlineSet();
+	}
+
+	public boolean isInline(Node node) {
+		return node != null && fInlineElements.contains(node.getNodeName().toLowerCase(Locale.US));
+	}
+
+	public boolean shouldSkipIndentForNode(Node node) {
+		return isInline(node.getParentNode());
+	}
+
+	/**
+	 * Returns an array of the element names considered as "inline" for the purposes of formatting.
+	 * This list represents those stored in the preference store when invoked.
+	 * 
+	 * @return An array of element names considered to be "inline"
+	 */
+	public static Object[] getInlineElements() {
+		return getInlineSet().toArray();
+	}
+
+	/**
+	 * Stores the element names to the preference store to be considered as "inline"
+	 * 
+	 * @param elements The element names considered to be "inline"
+	 */
+	public static void exportToPreferences(Object[] elements) {
+		if (elements != null) {
+			StringBuffer buffer = new StringBuffer();
+			for (int i = 0; i < elements.length; i++) {
+				if (i > 0)
+					buffer.append(',');
+				buffer.append(elements[i]);
+			}
+			HTMLCorePlugin.getDefault().getPluginPreferences().setValue(HTMLCorePreferenceNames.INLINE_ELEMENTS, buffer.toString());
+		}
+	}
+
+	/**
+	 * Returns an array of the default element names considered as "inline" for the purposes of formatting.
+	 * 
+	 * @return An array of the default element names considered to be "inline"
+	 */
+	public static Object[] getDefaultInlineElements() {
+		String inline = HTMLCorePlugin.getDefault().getPluginPreferences().getDefaultString(HTMLCorePreferenceNames.INLINE_ELEMENTS);
+		Set defaults = new HashSet();
+		StringTokenizer tokenizer = new StringTokenizer(inline, ",");
+		while (tokenizer.hasMoreTokens()) {
+			defaults.add(tokenizer.nextToken());
+		}
+		return defaults.toArray();
+	}
+
+	private static Set getInlineSet() {
+		String inline = HTMLCorePlugin.getDefault().getPluginPreferences().getString(HTMLCorePreferenceNames.INLINE_ELEMENTS);
+		Set elements = new HashSet();
+		StringTokenizer tokenizer = new StringTokenizer(inline, ",");
+		while (tokenizer.hasMoreTokens()) {
+			elements.add(tokenizer.nextToken());
+		}
+		return elements;
+	}
+}
diff --git a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceInitializer.java b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceInitializer.java
index e7b5e5b2ed..3130e1663b 100644
--- a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceInitializer.java
+++ b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceInitializer.java
@@ -61,6 +61,8 @@ public class HTMLCorePreferenceInitializer extends AbstractPreferenceInitializer
 		// this could be made smarter by actually looking up the content
 		// type's valid extensions
 		node.put(HTMLCorePreferenceNames.DEFAULT_EXTENSION, "html"); //$NON-NLS-1$
+
+		node.put(HTMLCorePreferenceNames.INLINE_ELEMENTS, "a,abbr,acronym,b,basefont,big,br,cite,em,font,i,img,input,label,li,q,s,select,small,span,strike,strong,sub,sup,td,th,title,u");
 		
 		initializeValidationPreferences(node);
 	}
diff --git a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceNames.java b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceNames.java
index 5c501e3e5a..af1e90fcec 100644
--- a/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceNames.java
+++ b/bundles/org.eclipse.wst.html.core/src/org/eclipse/wst/html/core/internal/preferences/HTMLCorePreferenceNames.java
@@ -198,6 +198,14 @@ public class HTMLCorePreferenceNames {
 	public static final String ATTR_NAME_CASE = "attrNameCase";//$NON-NLS-1$
 
 	/**
+	 * Preferred elements to be considered as inline for the purposes of formatting
+	 * <p>
+	 * Value is a comma-separated list of element names
+	 * </p>
+	 */
+	public static final String INLINE_ELEMENTS = "inlineElements"; //$NON-NLS-1$
+
+	/**
 	 * Possible value for the preference #TAG_NAME_CASE or #ATTR_NAME_CASE.
 	 * Indicates to leave case as is.
 	 *