(500, 503) Improves formatting of array and hash.

There were several glitches in the formatting of array and hash
in certain corner cases:
* first element in manifest
* last element in manifest
* being empty, or having one element
* not breaking after end comma (only on separating commas)

This also fixes formatting of separator expression ';' which was handled
as a statement (triggering extra linebreak).
closes #500
closes #503

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/geppetto/pp/dsl/formatting/AbstractListLayout.java

@@ -17,6 +17,8 @@
 import org.cloudsmith.xtext.dommodel.formatter.ILayoutManager.ILayoutContext;
 import org.cloudsmith.xtext.dommodel.formatter.LayoutUtils;
 import org.cloudsmith.xtext.textflow.ITextFlow;
+import org.eclipse.emf.ecore.EObject;
+import org.eclipse.xtext.AbstractElement;
 
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -43,17 +45,20 @@ protected boolean format(IDomNode node, ITextFlow flow, ILayoutContext context)
 		final int clusterWidth = advisor.clusterSize();
 
 		boolean breakAndAlign = false;
-		switch(advice) {
-			case Never:
-				break;
-			case Always:
-				breakAndAlign = true;
-				break;
-			case OnOverflow:
-				if(!layoutUtils.fitsOnSameLine(node, flow, context))
+		// Do not break and align unless there is more than one element in the list/hash
+		if(hasMoreThanOneElement(node.getSemanticObject()))
+			switch(advice) {
+				case Never:
+					break;
+				case Always:
 					breakAndAlign = true;
-				break;
-		}
+					break;
+				case OnOverflow:
+					// Measure fit from start to final significant element (not counting trailing WS/breaks)
+					if(!layoutUtils.fitsOnSameLine(node, getLastSignificantGrammarElement(), flow, context))
+						breakAndAlign = true;
+					break;
+			}
 		markup(node, breakAndAlign, clusterWidth, flow, context); // that was easy
 		return false;
 	}
@@ -62,7 +67,10 @@ protected IBreakAndAlignAdvice getAlignAdvice() {
 		return breakAlignAdviceProvider.get();
 	}
 
+	protected abstract AbstractElement getLastSignificantGrammarElement();
+
+	protected abstract boolean hasMoreThanOneElement(EObject semantic);
+
 	protected abstract void markup(IDomNode node, final boolean breakAndAlign, final int clusterWidth, ITextFlow flow,
 			ILayoutContext context);
-
 }

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/geppetto/pp/dsl/formatting/LiteralHashLayout.java

@@ -17,6 +17,7 @@
 import java.util.Map.Entry;
 
 import org.cloudsmith.geppetto.common.stats.IntegerCluster;
+import org.cloudsmith.geppetto.pp.LiteralHash;
 import org.cloudsmith.geppetto.pp.dsl.services.PPGrammarAccess;
 import org.cloudsmith.geppetto.pp.dsl.services.PPGrammarAccess.HashEntryElements;
 import org.cloudsmith.geppetto.pp.dsl.services.PPGrammarAccess.LiteralHashElements;
@@ -31,6 +32,7 @@
 import org.cloudsmith.xtext.textflow.ITextFlow;
 import org.cloudsmith.xtext.textflow.TextFlow;
 import org.eclipse.emf.ecore.EObject;
+import org.eclipse.xtext.AbstractElement;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -73,6 +75,16 @@ protected IBreakAndAlignAdvice getAlignAdvice() {
 		return breakAlignAdviceProvider.get();
 	}
 
+	@Override
+	protected AbstractElement getLastSignificantGrammarElement() {
+		return grammarAccess.getLiteralHashAccess().getRightCurlyBracketKeyword_4();
+	}
+
+	@Override
+	protected boolean hasMoreThanOneElement(EObject semantic) {
+		return ((LiteralHash) semantic).getElements().size() > 1;
+	}
+
 	@Override
 	protected void markup(IDomNode node, final boolean breakAndAlign, final int clusterWidth, ITextFlow flow,
 			ILayoutContext context) {

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/geppetto/pp/dsl/formatting/LiteralListLayout.java

@@ -13,6 +13,7 @@
 
 import java.util.Iterator;
 
+import org.cloudsmith.geppetto.pp.LiteralList;
 import org.cloudsmith.geppetto.pp.dsl.services.PPGrammarAccess;
 import org.cloudsmith.geppetto.pp.dsl.services.PPGrammarAccess.LiteralListElements;
 import org.cloudsmith.xtext.dommodel.DomModelUtils;
@@ -22,6 +23,7 @@
 import org.cloudsmith.xtext.dommodel.formatter.css.StyleSet;
 import org.cloudsmith.xtext.textflow.ITextFlow;
 import org.eclipse.emf.ecore.EObject;
+import org.eclipse.xtext.AbstractElement;
 
 import com.google.inject.Inject;
 
@@ -44,6 +46,16 @@ public class LiteralListLayout extends AbstractListLayout {
 	@Inject
 	private PPGrammarAccess grammarAccess;
 
+	@Override
+	protected AbstractElement getLastSignificantGrammarElement() {
+		return grammarAccess.getLiteralListAccess().getRightSquareBracketKeyword_3();
+	}
+
+	@Override
+	protected boolean hasMoreThanOneElement(EObject semantic) {
+		return ((LiteralList) semantic).getElements().size() > 1;
+	}
+
 	@Override
 	protected void markup(IDomNode node, final boolean breakAndAlign, final int clusterWidth, ITextFlow flow,
 			ILayoutContext context) {
@@ -64,6 +76,11 @@ else if(breakAndAlign && ge == access.getCommaKeyword_2_1_0()) {
 				if(DomModelUtils.isWhitespace(nextLeaf))
 					nextLeaf.getStyles().add(StyleSet.withStyles(styles.oneLineBreak()));
 			}
+			else if(breakAndAlign && ge == access.getCommaKeyword_2_2()) {
+				IDomNode nextLeaf = DomModelUtils.nextWhitespace(n);
+				if(DomModelUtils.isWhitespace(nextLeaf))
+					nextLeaf.getStyles().add(StyleSet.withStyles(styles.oneLineBreak()));
+			}
 		}
 
 	}

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/geppetto/pp/dsl/formatting/PPSemanticLayout.java

@@ -34,6 +34,7 @@
 import org.cloudsmith.geppetto.pp.ResourceBody;
 import org.cloudsmith.geppetto.pp.ResourceExpression;
 import org.cloudsmith.geppetto.pp.SelectorExpression;
+import org.cloudsmith.geppetto.pp.SeparatorExpression;
 import org.cloudsmith.geppetto.pp.SingleQuotedString;
 import org.cloudsmith.geppetto.pp.UnlessExpression;
 import org.cloudsmith.geppetto.pp.VerbatimTE;
@@ -394,6 +395,8 @@ protected Pair<Integer, Integer> internalFormatStatementList(IDomNode node, EObj
 					firstToken.getStyleClassifiers().add(StatementStyle.FIRST);
 					first = false;
 				}
+				if(semantic instanceof SeparatorExpression)
+					continue; // skip marking this
 				// mark all (except func args) as being a STATEMENT
 				firstToken.getStyleClassifiers().add(StatementStyle.STATEMENT);
 				statementCount++;

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/geppetto/pp/dsl/formatting/PPStylesheetProvider.java

@@ -315,7 +315,14 @@ public DomCSS get() {
 			Select.whitespaceBefore(Select.grammar(grammarAccess.getRubyLambdaAccess().getVerticalLineKeyword_4())) //
 			.withStyle(styles.noSpace()).withRuleName("WsBeforeRightRPipe"),
 			Select.whitespaceAfter(Select.grammar(grammarAccess.getRubyLambdaAccess().getVerticalLineKeyword_4())) //
-			.withStyle(styles.oneSpace()).withRuleName("WsAfterRightRPipe")
+			.withStyle(styles.oneSpace()).withRuleName("WsAfterRightRPipe"),
+
+			// Separator
+			Select.whitespaceBefore(
+				Select.grammar(grammarAccess.getSeparatorExpressionAccess().getSemicolonKeyword_1())) //
+			.withStyles(styles.noSpace(), styles.noLineBreak()).withRuleName("WsBeforeSeparator"),
+			Select.whitespaceAfter(Select.grammar(grammarAccess.getSeparatorExpressionAccess().getSemicolonKeyword_1())) //
+			.withStyles(styles.oneSpace(), styles.lineBreaks(0, 1, 1, true, true)).withRuleName("WsAfterSeparator")
 
 		// ,
 

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/xtext/dommodel/DomModelUtils.java

@@ -62,7 +62,10 @@ private static void appendEffectiveStyle(Appendable result, IDomNode node, Strin
 		if(effective instanceof StyleSetWithTracking) {
 			StyleSetWithTracking trackingSet = (StyleSetWithTracking) effective;
 			for(IStyle<?> style : effective.getStyles()) {
-				ruleToStyle.put(trackingSet.getStyleSource(style).getRuleName(), style);
+				String n = trackingSet.getStyleSource(style).getRuleName();
+				if(n == null || n.length() < 1)
+					throw new RuntimeException("Rule without name");
+				ruleToStyle.put(n, style);
 			}
 		}
 		else {

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/xtext/dommodel/formatter/LayoutUtils.java

@@ -84,21 +84,43 @@ public static void unifyWidthAndAlign(IDomNode dom, AbstractElement grammarEleme
 	 * 
 	 * @param node
 	 *            - the node to measure
+	 * @param untilGrammarElement
+	 *            - the last element to measure (inclusive)
 	 * @param flow
 	 *            - the flow where the node is to be written (at some later point in time)
 	 * @param context
 	 *            - the context used to write to the given flow
 	 * @return true if the formatted node fits on the same line
 	 */
-	public boolean fitsOnSameLine(IDomNode node, ITextFlow flow, ILayoutContext context) {
+	public boolean fitsOnSameLine(IDomNode node, AbstractElement untilGrammarElement, ITextFlow flow,
+			ILayoutContext context) {
 		DelegatingLayoutContext dlc = new DelegatingLayoutContext(context);
 		MeasuredTextFlow continuedFlow = new MeasuredTextFlow((MeasuredTextFlow) flow);
-		int h0 = continuedFlow.getHeight();
+		// if flow is empty the first output char will give it height 1
+		int h0 = Math.max(1, continuedFlow.getHeight());
 		for(IDomNode n : node.getChildren()) {
 			feeder.sequence(n, continuedFlow, dlc);
+			if(untilGrammarElement != null && n.getGrammarElement() == untilGrammarElement)
+				break;
 		}
 		int h1 = continuedFlow.getHeight();
 		// if output causes break (height increases), or at edge (the '{' will not fit).
 		return h1 <= h0 && continuedFlow.getWidthOfLastLine() < continuedFlow.getPreferredMaxWidth();
 	}
+
+	/**
+	 * Returns true if the node, when formatted and written to the given flow will fit on what remains
+	 * on the line. The flow and contexts are not affected by this operation.
+	 * 
+	 * @param node
+	 *            - the node to measure
+	 * @param flow
+	 *            - the flow where the node is to be written (at some later point in time)
+	 * @param context
+	 *            - the context used to write to the given flow
+	 * @return true if the formatted node fits on the same line
+	 */
+	public boolean fitsOnSameLine(IDomNode node, ITextFlow flow, ILayoutContext context) {
+		return fitsOnSameLine(node, null, flow, context);
+	}
 }

org.cloudsmith.geppetto.pp.dsl/src/org/cloudsmith/xtext/dommodel/formatter/css/DomCSS.java

@@ -147,7 +147,7 @@ public List<Rule> collectRules(IDomNode node) {
 
 		// if element has a style map, add a (matched) rule for it
 		if(node.getStyles() != null)
-			matches.add(new Rule(new Select.Instance(node), node.getStyles()));
+			matches.add(new Rule(new Select.Instance(node), node.getStyles()).withRuleName("StyleInNode"));
 		for(Rule r : cssRules)
 			if(r.matches(node))
 				matches.add(r);