Attachment 169285 Details for Bug 313524 – New proposed patch + doc

[patch] New proposed patch + doc

v02.txt (text/plain), 11.67 KB, created by Frederic Fusier

on 2010-05-20 04:16:35 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Frederic Fusier

Created: 2010-05-20 04:16:35 EDT

Size: 11.67 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: buildnotes_jdt-core.html
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/buildnotes_jdt-core.html,v
>retrieving revision 1.7461
>diff -u -r1.7461 buildnotes_jdt-core.html
>--- buildnotes_jdt-core.html	18 May 2010 18:12:12 -0000	1.7461
>+++ buildnotes_jdt-core.html	20 May 2010 08:13:23 -0000
>@@ -48,9 +48,60 @@
> Project org.eclipse.jdt.core v_A54
> (<a href="http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.jdt.core/?only_with_tag=v_A54">cvs</a>).
> <h2>What's new in this drop</h2>
>+<ul>
>+<li>
>+Added a new preference to force the formatter to try to keep nested expressions on one line.
>+
>+This new preference is controlled with the option:
>+<code>DefaultCodeFormatterConstants.FORMATTER_KEEP_NESTED_EXPRESSIONS_ON_ONE_LINE</code>
>+<pre>
>+/**
>+ * FORMATTER / Option to wrap outer expressions first in nested expressions
>+ * - option id: "org.eclipse.jdt.core.formatter.wrap_outer_expressions_first_when_nested"
>+ * - possible values: { TRUE, FALSE }
>+ * - default: TRUE
>+ *
>+ * This option changes the formatter behavior when nested method calls are encountered.
>+ * Since 3.6, the formatter tries to wrap outermost method calls first to have a better output.
>+ * For example, let's say we are using the Eclipse built-in profile with a max line width=40+space for tab policy.
>+ * Then consider the following snippet:
>+ *
>+ * public class X01 {
>+ * void test() {
>+ * foo(bar(1, 2, 3, 4), bar(5, 6, 7, 8));
>+ * }
>+ * }
>+ *
>+ * With this new strategy, the formatter will wrap the line earlier, between the arguments of the message call
>+ * for this example, and then it will allow to keep each nested call on a single line.
>+ * Hence, the output will be:
>+ *
>+ * public class X01 {
>+ * void test() {
>+ * foo(bar(1, 2, 3, 4),
>+ * bar(5, 6, 7, 8));
>+ * }
>+ * }
>+ *
>+ * Important notes:
>+ * 1. This new behavior is automatically activated (ie. the default value for this preference is {@link #TRUE}).
>+ * If the backward compatibility regarding previous versions formatter behavior (ie. before 3.6 version) is necessary,
>+ * then the preference needs to be set to the {@link #FALSE} value to retrieve the previous formatter behavior.
>+ * 2. The new strategy currently only applies to nested method calls, but might be extended to other nested expressions in future versions
>+ * 
>+ * @see #TRUE
>+ * @see #FALSE
>+ * @since 3.6
>+ */
>+</pre>
>+See bug <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=313524">313524</a> for more details.
>+</li>
>+</ul>
> 
> <h3>Problem Reports Fixed</h3>
>-<a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=313109">313109</a>
>+<a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=313524">313524</a>
>+[formatter] Add preference for improved lines wrapping in nested method calls
>+ <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=313109">313109</a>
> @SuppressWarnings on multiple locals is marked unnecessary if any local is never used
> 
> <a name="v_A53"></a>
>Index: formatter/org/eclipse/jdt/core/formatter/DefaultCodeFormatterConstants.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/core/formatter/DefaultCodeFormatterConstants.java,v
>retrieving revision 1.112
>diff -u -r1.112 DefaultCodeFormatterConstants.java
>--- formatter/org/eclipse/jdt/core/formatter/DefaultCodeFormatterConstants.java	6 May 2010 11:11:26 -0000	1.112
>+++ formatter/org/eclipse/jdt/core/formatter/DefaultCodeFormatterConstants.java	20 May 2010 08:13:25 -0000
>@@ -3500,6 +3500,51 @@
> 	public static final String FORMATTER_WRAP_BEFORE_BINARY_OPERATOR = JavaCore.PLUGIN_ID + ".formatter.wrap_before_binary_operator"; //$NON-NLS-1$
> 	/**
> 	 * <pre>
>+	 * FORMATTER / Option to wrap outer expressions first in nested expressions
>+	 * - option id: "org.eclipse.jdt.core.formatter.wrap_outer_expressions_first_when_nested"
>+	 * - possible values: { TRUE, FALSE }
>+	 * - default: TRUE
>+	 * </pre>
>+	 * 
>+	 * This option changes the formatter behavior when nested method calls are encountered.
>+	 * Since 3.6, the formatter tries to wrap outermost method calls first to have a better output.
>+	 * For example, let's say we are using the Eclipse built-in profile with a max line width=40+space for tab policy.
>+	 * Then consider the following snippet:
>+	 * <pre>
>+	 * public class X01 {
>+	 * void test() {
>+	 * foo(bar(1, 2, 3, 4), bar(5, 6, 7, 8));
>+	 * }
>+	 * }
>+	 * </pre>
>+	 * With this new strategy, the formatter will wrap the line earlier, between the arguments of the message call
>+	 * for this example, and then it will allow to keep each nested call on a single line.
>+	 * Hence, the output will be:
>+	 * <pre>
>+	 * public class X01 {
>+	 * void test() {
>+	 * foo(bar(1, 2, 3, 4),
>+	 * bar(5, 6, 7, 8));
>+	 * }
>+	 * }
>+	 * </pre>
>+	 * 
>+	 * 
>+	 * Important notes:
>+	 * <ol>
>+	 * <li>This new behavior is automatically activated (ie. the default value for this preference is {@link #TRUE}).
>+	 * If the backward compatibility regarding previous versions formatter behavior (ie. before 3.6 version) is necessary,
>+	 * then the preference needs to be set to the {@link #FALSE} value to retrieve the previous formatter behavior.</li>
>+	 * <li>The new strategy currently only applies to nested method calls, but might be extended to other nested expressions in future versions</li>
>+	 * </ol>
>+	 * 
>+	 * @see #TRUE
>+	 * @see #FALSE
>+	 * @since 3.6
>+	 */
>+	public static final String FORMATTER_WRAP_OUTER_EXPRESSIONS_FIRST_WHEN_NESTED = JavaCore.PLUGIN_ID + ".formatter.wrap_outer_expressions_first_when_nested"; //$NON-NLS-1$
>+	/**
>+	 * <pre>
> 	 * FORMATTER / The wrapping is done by indenting by one compare to the current indentation.
> 	 * </pre>
> 	 * @since 3.0
>Index: formatter/org/eclipse/jdt/internal/formatter/CodeFormatterVisitor.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/CodeFormatterVisitor.java,v
>retrieving revision 1.234
>diff -u -r1.234 CodeFormatterVisitor.java
>--- formatter/org/eclipse/jdt/internal/formatter/CodeFormatterVisitor.java	6 May 2010 16:29:32 -0000	1.234
>+++ formatter/org/eclipse/jdt/internal/formatter/CodeFormatterVisitor.java	20 May 2010 08:13:27 -0000
>@@ -1364,7 +1364,9 @@
> 			}
> 			startingPositionInCascade = 2;
> 		}
>-		int tieBreakRule = size-startingPositionInCascade > 2 ? Alignment.R_OUTERMOST : Alignment.R_INNERMOST;
>+		int tieBreakRule = this.preferences.wrap_outer_expressions_first_when_nested && size-startingPositionInCascade > 2
>+			? Alignment.R_OUTERMOST
>+			: Alignment.R_INNERMOST;
> 		Alignment cascadingMessageSendAlignment =
> 			this.scribe.createAlignment(
> 				Alignment.CASCADING_MESSAGE_SEND,
>@@ -1676,7 +1678,7 @@
> 		Alignment messageAlignment) {
> 
> 		if (messageAlignment != null) {
>-			if (messageAlignment.canAlign()) {
>+			if (!this.preferences.wrap_outer_expressions_first_when_nested || messageAlignment.canAlign()) {
> 				this.scribe.alignFragment(messageAlignment, 0);
> 			}
> 			this.scribe.printNextToken(TerminalTokens.TokenNameDOT);
>Index: formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatterOptions.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatterOptions.java,v
>retrieving revision 1.107
>diff -u -r1.107 DefaultCodeFormatterOptions.java
>--- formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatterOptions.java	6 May 2010 11:11:27 -0000	1.107
>+++ formatter/org/eclipse/jdt/internal/formatter/DefaultCodeFormatterOptions.java	20 May 2010 08:13:30 -0000
>@@ -325,6 +325,7 @@
> 	public int tab_char;
> 	public boolean use_tabs_only_for_leading_indentations;
> 	public boolean wrap_before_binary_operator;
>+	public boolean wrap_outer_expressions_first_when_nested;
> 
> 	public int initial_indentation_level;
> 	public String line_separator;
>@@ -622,6 +623,7 @@
> 		options.put(DefaultCodeFormatterConstants.FORMATTER_DISABLING_TAG, this.disabling_tag == null ? Util.EMPTY_STRING : new String(this.disabling_tag));
> 		options.put(DefaultCodeFormatterConstants.FORMATTER_ENABLING_TAG, this.enabling_tag == null ? Util.EMPTY_STRING : new String(this.enabling_tag));
> 		options.put(DefaultCodeFormatterConstants.FORMATTER_USE_ON_OFF_TAGS, this.use_tags ? DefaultCodeFormatterConstants.TRUE : DefaultCodeFormatterConstants.FALSE);
>+		options.put(DefaultCodeFormatterConstants.FORMATTER_WRAP_OUTER_EXPRESSIONS_FIRST_WHEN_NESTED, this.wrap_outer_expressions_first_when_nested ? DefaultCodeFormatterConstants.TRUE : DefaultCodeFormatterConstants.FALSE);
> 		return options;
> 	}
> 
>@@ -1997,6 +1999,10 @@
> 				}
> 			}
> 		}
>+		final Object wrapWrapOuterExpressionsFirstWhenNestedOption = settings.get(DefaultCodeFormatterConstants.FORMATTER_WRAP_OUTER_EXPRESSIONS_FIRST_WHEN_NESTED);
>+		if (wrapWrapOuterExpressionsFirstWhenNestedOption != null) {
>+			this.wrap_outer_expressions_first_when_nested = DefaultCodeFormatterConstants.TRUE.equals(wrapWrapOuterExpressionsFirstWhenNestedOption);
>+		}
> 	}
> 
> 	/**
>@@ -2310,6 +2316,7 @@
> 		this.use_tags = false;
> 		this.disabling_tag = DEFAULT_DISABLING_TAG;
> 		this.enabling_tag = DEFAULT_ENABLING_TAG;
>+		this.wrap_outer_expressions_first_when_nested = true;
> 	}
> 
> 	public void setEclipseDefaultSettings() {
>@@ -2584,5 +2591,6 @@
> 		this.use_tags = false;
> 		this.disabling_tag = DEFAULT_DISABLING_TAG;
> 		this.enabling_tag = DEFAULT_ENABLING_TAG;
>+		this.wrap_outer_expressions_first_when_nested = true;
> 	}
> }
>Index: formatter/org/eclipse/jdt/internal/formatter/Scribe.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/formatter/org/eclipse/jdt/internal/formatter/Scribe.java,v
>retrieving revision 1.209
>diff -u -r1.209 Scribe.java
>--- formatter/org/eclipse/jdt/internal/formatter/Scribe.java	6 May 2010 15:22:52 -0000	1.209
>+++ formatter/org/eclipse/jdt/internal/formatter/Scribe.java	20 May 2010 08:13:41 -0000
>@@ -1258,6 +1258,38 @@
> 	}
> 
> 	public void handleLineTooLong() {
>+		if (this.formatter.preferences.wrap_outer_expressions_first_when_nested) {
>+			handleLineTooLongSmartly();
>+			return;
>+		}
>+		// search for closest breakable alignment, using tiebreak rules
>+		// look for outermost breakable one
>+		int relativeDepth = 0, outerMostDepth = -1;
>+		Alignment targetAlignment = this.currentAlignment;
>+		while (targetAlignment != null){
>+			if (targetAlignment.tieBreakRule == Alignment.R_OUTERMOST && targetAlignment.couldBreak()){
>+				outerMostDepth = relativeDepth;
>+			}
>+			targetAlignment = targetAlignment.enclosing;
>+			relativeDepth++;
>+		}
>+		if (outerMostDepth >= 0) {
>+			throw new AlignmentException(AlignmentException.LINE_TOO_LONG, outerMostDepth);
>+		}
>+		// look for innermost breakable one
>+		relativeDepth = 0;
>+		targetAlignment = this.currentAlignment;
>+		while (targetAlignment != null){
>+			if (targetAlignment.couldBreak()){
>+				throw new AlignmentException(AlignmentException.LINE_TOO_LONG, relativeDepth);
>+			}
>+			targetAlignment = targetAlignment.enclosing;
>+			relativeDepth++;
>+		}
>+		// did not find any breakable location - proceed
>+	}
>+
>+	private void handleLineTooLongSmartly() {
> 		// search for closest breakable alignment, using tiebreak rules
> 		// look for outermost breakable one
> 		int relativeDepth = 0, outerMostDepth = -1;

Flags:

daniel_megert: review+

Actions: View | Diff

Attachments on bug 313524: 169154 | 169285 | 169295 | 169296