Fix layout bugs in PullResultDialog

- result group should not grab vertical space if it just contains a
label
- dialog was huge e.g. for simple FAST_FORWARD rebase result
- always persist dialog width; only persist height for nontrivial
content

Change-Id: I1af2e26cfb3a86028579436804a3ff16ac38759a
Also-by: Robin Stocker <robin@nibor.org>
Signed-off-by: Markus Keller <markus_keller@ch.ibm.com>
Signed-off-by: Robin Stocker <robin@nibor.org>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 38 insertions, 7 deletions

diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/pull/PullResultDialog.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/pull/PullResultDialog.java
index ce42b7a780..9c7805e11c 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/pull/PullResultDialog.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/pull/PullResultDialog.java
@@ -30,6 +30,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.FetchResult;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
@@ -50,7 +51,7 @@ public class PullResultDialog extends Dialog {
 
 	private final PullResult result;
 
-	private final boolean hasUpdates;
+	private boolean persistSize;
 
 	/**
 	 * @param shell
@@ -63,8 +64,7 @@ public class PullResultDialog extends Dialog {
 		setBlockOnOpen(false);
 		this.repo = repo;
 		this.result = result;
-		hasUpdates = hasFetchResults() || hasMergeResults()
-				|| hasRebaseResults();
+		persistSize = hasFetchResults() || hasMergeResults();
 	}
 
 	private boolean hasFetchResults() {
@@ -123,16 +123,18 @@ public class PullResultDialog extends Dialog {
 		Group mergeResultGroup = new Group(main, SWT.SHADOW_ETCHED_IN);
 		mergeResultGroup
 				.setText(UIText.PullResultDialog_MergeResultGroupHeader);
-		GridDataFactory.fillDefaults().grab(true, true).applyTo(
-				mergeResultGroup);
 		if (hasMergeResults()) {
+			GridDataFactory.fillDefaults().grab(true, true).applyTo(
+					mergeResultGroup);
 			GridLayoutFactory.fillDefaults().applyTo(mergeResultGroup);
 			MergeResultDialog dlg = new MergeResultDialog(getParentShell(),
 					repo, result.getMergeResult());
 			dlg.createDialogArea(mergeResultGroup);
 		} else if (hasRebaseResults()) {
+			GridDataFactory.fillDefaults().grab(true, false).applyTo(
+					mergeResultGroup);
+			GridLayoutFactory.swtDefaults().applyTo(mergeResultGroup);
 			Status status = result.getRebaseResult().getStatus();
-			GridLayoutFactory.fillDefaults().applyTo(mergeResultGroup);
 			switch (status) {
 			case OK:
 				// fall through
@@ -157,6 +159,8 @@ public class PullResultDialog extends Dialog {
 			Text statusText = new Text(mergeResultGroup, SWT.READ_ONLY);
 			statusText.setText(status.name());
 		} else {
+			GridDataFactory.fillDefaults().grab(true, false).applyTo(
+					mergeResultGroup);
 			GridLayoutFactory.swtDefaults().applyTo(mergeResultGroup);
 			Label noResult = new Label(mergeResultGroup, SWT.NONE);
 			noResult
@@ -187,6 +191,33 @@ public class PullResultDialog extends Dialog {
 
 	@Override
 	protected int getDialogBoundsStrategy() {
-		return hasUpdates ? DIALOG_PERSISTLOCATION | DIALOG_PERSISTSIZE : DIALOG_PERSISTLOCATION;
+		int strategy = DIALOG_PERSISTLOCATION;
+		if (persistSize)
+			strategy |= DIALOG_PERSISTSIZE;
+		return strategy;
+	}
+
+	@Override
+	protected Point getInitialSize() {
+		if (!persistSize) {
+			// For "small" dialogs with label-only results, use the default
+			// height and the persisted width
+			Point size = super.getInitialSize();
+			size.x = getPersistedSize().x;
+			return size;
+		}
+		return super.getInitialSize();
+	}
+
+	private Point getPersistedSize() {
+		boolean oldPersistSize = persistSize;
+		// This affects getDialogBoundsStrategy
+		persistSize = true;
+		try {
+			Point persistedSize = super.getInitialSize();
+			return persistedSize;
+		} finally {
+			persistSize = oldPersistSize;
+		}
 	}
 }