Bug 508787 - Rework launch configuration selection dialog for groupsI20170124-0800I20170123-2000I20170123-0830I20170123-0800

According to discussion in the bug 508787, a better layout for the
selection dialog has been implemented. This also removed quite some
(invisible) widgets from the dialog and should improve the overal
experience with it.

The most user visible change is that the launch mode combo is now below
the launch configuration selection tree.

Change-Id: I25296f5a4c9315af87d817c164a3d33b858919c2
Signed-off-by: Markus Duft <markus.duft@ssi-schaefer.com>

5 files changed, 90 insertions, 243 deletions

diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.java
index c1020bc05..aeae3b518 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.java
@@ -243,7 +243,7 @@ public class DebugUIMessages extends NLS {
 	public static String GroupLaunch_Cycle;
 	public static String GroupLaunch_Error;
 	public static String GroupLaunchConfigurationSelectionDialog_0;
-
+	public static String GroupLaunchConfigurationSelectionDialog_1;
 	public static String GroupLaunchConfigurationSelectionDialog_10;
 	public static String GroupLaunchConfigurationSelectionDialog_11;
 	public static String GroupLaunchConfigurationSelectionDialog_12;
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.properties b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.properties
index e082ca5a5..af3d12e82 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.properties
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/DebugUIMessages.properties
@@ -122,13 +122,14 @@ GroupLaunchConfigurationSelectionDialog_5=Use default mode when launching
 GroupLaunchConfigurationSelectionDialog_7=Select a launch configuration
 GroupLaunchConfigurationSelectionDialog_8=&Post launch action:
 GroupLaunchConfigurationSelectionDialog_9=&Seconds:
+GroupLaunchConfigurationSelectionDialog_1=Selected launch mode is not supported for selected launch configuration(s)
 GroupLaunchConfigurationSelectionDialog_10=Enter valid number of seconds
 GroupLaunchConfigurationSelectionDialog_11=Select only one launch configuration
 GroupLaunchConfigurationSelectionDialog_12=Add Launch Configuration
 GroupLaunchConfigurationSelectionDialog_13=Edit Launch Configuration
 GroupLaunchConfigurationSelectionDialog_14=Add one or more launch configurations to the launch group
 GroupLaunchConfigurationSelectionDialog_15=Edit an existing entry in the launch group
-GroupLaunchConfigurationSelectionDialog_adoptText=Adopt launch if already running
+GroupLaunchConfigurationSelectionDialog_adoptText=&Adopt launch if already running
 GroupLaunchConfigurationSelectionDialog_adoptTooltip=Instead of launching a new process, adds the running launch to the group.
 GroupLaunchConfigurationSelectionDialog_errorNoRegexp=Enter a regular expression to wait for
 GroupLaunchConfigurationSelectionDialog_regexp=Regular Expression:
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/ComboControlledStackComposite.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/ComboControlledStackComposite.java
deleted file mode 100644
index e1b90b2a3..000000000
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/ComboControlledStackComposite.java
+++ /dev/null
@@ -1,156 +0,0 @@
-/*******************************************************************************
- *  Copyright (c) 2009, 2016 QNX Software Systems 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:
- *      QNX Software Systems - initial API and implementation
- *      Freescale Semiconductor
- *******************************************************************************/
-package org.eclipse.debug.internal.ui.groups;
-
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.TreeMap;
-
-import org.eclipse.swt.SWT;
-import org.eclipse.swt.custom.StackLayout;
-import org.eclipse.swt.events.SelectionAdapter;
-import org.eclipse.swt.events.SelectionEvent;
-import org.eclipse.swt.layout.GridData;
-import org.eclipse.swt.layout.GridLayout;
-import org.eclipse.swt.widgets.Combo;
-import org.eclipse.swt.widgets.Composite;
-import org.eclipse.swt.widgets.Control;
-import org.eclipse.swt.widgets.Label;
-
-/**
- * Stack Composite - Switch between panes controlled by combo box
- * <p>
- * Copied from CDT (org.eclipse.cdt.launch)
- */
-class ComboControlledStackComposite extends Composite {
-	private Composite fArea;
-	private Combo fCombo;
-	private Map<String, Composite> tabMap; // label ==> tab
-	private Map<String, String> capMap = new TreeMap<>();
-	private StackLayout layout;
-	private Label fLabel;
-
-	public ComboControlledStackComposite(Composite parent, int style) {
-		super(parent, style);
-		tabMap = new LinkedHashMap<String, Composite>();
-		setLayout(new GridLayout(2, false));
-		createContents(this);
-	}
-
-	public void setLabelText(String label) {
-		fLabel.setText(label);
-	}
-
-	private static String capitalize(String l) {
-		return l.substring(0, 1).toUpperCase() + l.substring(1);
-	}
-
-	public void addItem(String label, Composite tab) {
-		tabMap.put(label, tab);
-		String cap = capitalize(label);
-		fCombo.add(cap);
-		capMap.put(cap, label);
-		if (layout.topControl==null) {
-			layout.topControl = tab;
-			fCombo.setText(cap);
-		}
-	}
-
-	public void deleteItem(String label) {
-		if (capMap.get(fCombo.getText()).equals(label)) {
-			setSelection(fCombo.getItem(0));
-		}
-		Composite tab = tabMap.get(label);
-		if (tab != null) {
-			tab.dispose();
-			tabMap.remove(label);
-			capMap.remove(capitalize(label));
-		}
-	}
-
-	public void setSelection(String label) {
-		fCombo.setText(capitalize(label));
-		setPage(label);
-	}
-
-	protected void createContents(Composite parent) {
-		fLabel = createLabel(this);
-		fCombo = createCombo(this);
-		GridData cgd = new GridData(GridData.FILL_HORIZONTAL);
-
-		fCombo.setLayoutData(cgd);
-		fArea = createTabArea(this);
-		GridData agd = new GridData(GridData.FILL_BOTH);
-		agd.horizontalSpan = 2;
-		fArea.setLayoutData(agd);
-	}
-
-
-	public Composite getStackParent() {
-		return fArea;
-	}
-
-	public Label getLabel() {
-		return fLabel;
-	}
-
-	/**
-	 * @return the underlying combo, should NOT be used to get the actual text,
-	 *         use {@link #getSelection()} instead.
-	 */
-	public Combo getCombo() {
-		return fCombo;
-	}
-
-	public String getSelection() {
-		return capMap.get(fCombo.getText());
-	}
-
-	protected Composite createTabArea(Composite parent) {
-		Composite comp = new Composite(parent, SWT.NONE);
-		layout = new StackLayout();
-		comp.setLayout(layout);
-
-		return comp;
-	}
-
-
-	protected Label createLabel(Composite parent) {
-		Label label = new Label(parent, SWT.WRAP);
-	    return label;
-    }
-
-	protected Combo createCombo(Composite parent) {
-		Combo box = new Combo(parent, SWT.READ_ONLY);
-		box.addSelectionListener(new SelectionAdapter() {
-			@Override
-			public void widgetSelected(SelectionEvent e) {
-				String name = fCombo.getText();
-				comboSelected(name);
-			}
-		});
-		return box;
-	}
-
-	protected void comboSelected(String label) {
-		setPage(capMap.get(label));
-	}
-
-	protected void setPage(String label) {
-		layout.topControl = tabMap.get(label);
-		getStackParent().layout();
-	}
-
-	public Control getTopControl() {
-		return layout != null ? layout.topControl : null;
-	}
-}
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationSelectionDialog.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationSelectionDialog.java
index 487355228..181d1b549 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationSelectionDialog.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationSelectionDialog.java
@@ -40,13 +40,13 @@ import org.eclipse.debug.ui.ILaunchGroup;
 import org.eclipse.jface.dialogs.IDialogConstants;
 import org.eclipse.jface.dialogs.TitleAreaDialog;
 import org.eclipse.jface.layout.GridDataFactory;
+import org.eclipse.jface.layout.GridLayoutFactory;
 import org.eclipse.jface.resource.ImageDescriptor;
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.ISelectionChangedListener;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.jface.viewers.SelectionChangedEvent;
 import org.eclipse.jface.viewers.StructuredSelection;
-import org.eclipse.jface.viewers.TreeViewer;
 import org.eclipse.jface.viewers.Viewer;
 import org.eclipse.jface.viewers.ViewerFilter;
 import org.eclipse.swt.SWT;
@@ -54,7 +54,7 @@ import org.eclipse.swt.events.ModifyEvent;
 import org.eclipse.swt.events.ModifyListener;
 import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
-import org.eclipse.swt.graphics.Rectangle;
+import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.layout.GridData;
 import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Button;
@@ -64,8 +64,6 @@ import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.Text;
-import org.eclipse.swt.widgets.Tree;
-import org.eclipse.ui.dialogs.FilteredTree;
 import org.eclipse.ui.dialogs.PatternFilter;
 
 /**
@@ -81,7 +79,6 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 	private boolean adoptIfRunning;
 	private ViewerFilter emptyTypeFilter;
 	private IStructuredSelection fInitialSelection;
-	private ComboControlledStackComposite fStackComposite;
 	private Label fActionParamLabel;
 	private Text fActionParamWidget; // in seconds
 	private boolean fForEditing; // true if dialog was opened to edit an entry,
@@ -143,8 +140,14 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 	}
 
 	@Override
+	protected Point getInitialSize() {
+		return new Point(750, 550);
+	}
+
+	@Override
 	protected Control createDialogArea(Composite parent2) {
 		Composite comp = (Composite) super.createDialogArea(parent2);
+		GridLayoutFactory.fillDefaults().margins(10, 10).applyTo(comp);
 
 		// title bar
 		getShell().setText(fForEditing ? DebugUIMessages.GroupLaunchConfigurationSelectionDialog_13 : DebugUIMessages.GroupLaunchConfigurationSelectionDialog_12);
@@ -152,8 +155,6 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 		// dialog message area (not title bar)
 		setTitle(fForEditing ? DebugUIMessages.GroupLaunchConfigurationSelectionDialog_15 : DebugUIMessages.GroupLaunchConfigurationSelectionDialog_14);
 
-		fStackComposite = new ComboControlledStackComposite(comp, SWT.NONE);
-
 		Map<String, ILaunchGroup> modes = new LinkedHashMap<>();
 		modes.put(GroupLaunchElement.MODE_INHERIT, new InheritModeGroup());
 		Set<ILaunchGroup> sortedGroups = new TreeSet<>((a, b) -> {
@@ -173,46 +174,37 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 			}
 		}
 
-		for (Map.Entry<String, ILaunchGroup> entry : modes.entrySet()) {
-			ILaunchGroup launchGroup = entry.getValue();
-			LaunchConfigurationFilteredTree fTree = new LaunchConfigurationFilteredTree(fStackComposite.getStackParent(), SWT.MULTI | SWT.H_SCROLL | SWT.V_SCROLL | SWT.BORDER, new PatternFilter(), launchGroup, null);
-			String lgm = entry.getKey();
-			fStackComposite.addItem(lgm, fTree);
-			fTree.createViewControl();
-			ViewerFilter[] filters = fTree.getViewer().getFilters();
-			for (ViewerFilter viewerFilter : filters) {
-				if (viewerFilter instanceof LaunchGroupFilter) {
-					fTree.getViewer().removeFilter(viewerFilter);
-				}
-			}
-			fTree.getViewer().addFilter(emptyTypeFilter);
-			fTree.getViewer().addSelectionChangedListener(this);
-			if (lgm.equals(this.mode)) {
-				fStackComposite.setSelection(lgm);
-			}
-			if (fInitialSelection != null) {
-				fTree.getViewer().setSelection(fInitialSelection, true);
+		// the tree requires a non-null group. use inherit as dummy as this will
+		// not cause filtering.
+		ILaunchGroup launchGroup = modes.get(GroupLaunchElement.MODE_INHERIT);
+		LaunchConfigurationFilteredTree fTree = new LaunchConfigurationFilteredTree(comp, SWT.MULTI | SWT.H_SCROLL | SWT.V_SCROLL | SWT.BORDER, new PatternFilter(), launchGroup, null);
+		fTree.createViewControl();
+		ViewerFilter[] filters = fTree.getViewer().getFilters();
+		for (ViewerFilter viewerFilter : filters) {
+			if (viewerFilter instanceof LaunchGroupFilter) {
+				fTree.getViewer().removeFilter(viewerFilter);
 			}
 		}
-		fStackComposite.setLabelText(DebugUIMessages.GroupLaunchConfigurationSelectionDialog_4);
-		fStackComposite.pack();
-		Rectangle bounds = fStackComposite.getBounds();
-		// adjust size
-		GridData data = ((GridData) fStackComposite.getLayoutData());
-		if (data == null) {
-			data = new GridData(GridData.FILL_BOTH);
-			fStackComposite.setLayoutData(data);
+		fTree.getViewer().addFilter(emptyTypeFilter);
+		fTree.getViewer().addSelectionChangedListener(this);
+		if (fInitialSelection != null) {
+			fTree.getViewer().setSelection(fInitialSelection, true);
 		}
-		data.heightHint = Math.max(convertHeightInCharsToPixels(15), bounds.height);
-		data.widthHint = Math.max(convertWidthInCharsToPixels(40), bounds.width);
-		fStackComposite.getCombo().addSelectionListener(new SelectionAdapter() {
-			@Override
-			public void widgetSelected(SelectionEvent e) {
-				mode = fStackComposite.getSelection();
-			}
-		});
+		GridDataFactory.fillDefaults().grab(true, true).minSize(400, 150).applyTo(fTree.getViewer().getControl());
 
-		Button chkAdopt = new Button(comp, SWT.CHECK);
+		Composite additionalSettings = new Composite(comp, SWT.NONE);
+		additionalSettings.setLayout(new GridLayout(4, false));
+		additionalSettings.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
+
+		createModeSelectionControl(modes, additionalSettings);
+		createPostLaunchControl(additionalSettings);
+
+		// skip the first cell and put the checkbox in the second one
+		Composite c = new Composite(additionalSettings, SWT.NONE);
+		GridLayoutFactory.fillDefaults().applyTo(c);
+		GridDataFactory.fillDefaults().applyTo(c);
+
+		Button chkAdopt = new Button(additionalSettings, SWT.CHECK);
 		chkAdopt.setText(DebugUIMessages.GroupLaunchConfigurationSelectionDialog_adoptText);
 		chkAdopt.setToolTipText(DebugUIMessages.GroupLaunchConfigurationSelectionDialog_adoptTooltip);
 		chkAdopt.setSelection(adoptIfRunning);
@@ -222,15 +214,47 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 				adoptIfRunning = chkAdopt.getSelection();
 			}
 		});
+		GridDataFactory.fillDefaults().grab(true, false).span(3, 1).applyTo(chkAdopt);
 
-		createPostLaunchControl(comp);
 		return comp;
 	}
 
-	private void createPostLaunchControl(Composite parent) {
-		Composite comp = new Composite(parent, SWT.NONE);
-		comp.setLayout(new GridLayout(4, false));
-		comp.setLayoutData(new GridData(GridData.FILL_HORIZONTAL));
+	private void createModeSelectionControl(Map<String, ILaunchGroup> modes, Composite comp) {
+		Label label = new Label(comp, SWT.NONE);
+		label.setText(DebugUIMessages.GroupLaunchConfigurationSelectionDialog_4);
+
+		Map<String, String> capitalized = new LinkedHashMap<>();
+		modes.keySet().forEach(m -> capitalized.put(m.substring(0, 1).toUpperCase() + m.substring(1), m));
+
+		Combo cvMode = new Combo(comp, SWT.READ_ONLY);
+		GridDataFactory.fillDefaults().grab(true, false).applyTo(cvMode);
+		cvMode.setItems(capitalized.keySet().toArray(new String[capitalized.size()]));
+
+		// initial selection to the current mode.
+		int index = 0;
+		for (String m : modes.keySet()) {
+			if (m.equals(mode)) {
+				cvMode.select(index);
+				break;
+			}
+			index++;
+		}
+
+		cvMode.addSelectionListener(new SelectionAdapter() {
+			@Override
+			public void widgetSelected(SelectionEvent e) {
+				mode = capitalized.get(cvMode.getText());
+				validate();
+			}
+		});
+
+		// fill up the remaining two cells in the parent layout
+		Composite c = new Composite(comp, SWT.NONE);
+		GridLayoutFactory.fillDefaults().applyTo(c);
+		GridDataFactory.fillDefaults().span(2, 1).applyTo(c);
+	}
+
+	private void createPostLaunchControl(Composite comp) {
 		Label label = new Label(comp, SWT.NONE);
 		label.setText(DebugUIMessages.GroupLaunchConfigurationSelectionDialog_8);
 		Combo combo = new Combo(comp, SWT.READ_ONLY);
@@ -248,10 +272,11 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 			}
 		});
 		combo.setText(action.getDescription());
+		GridDataFactory.fillDefaults().grab(true, false).minSize(250, SWT.DEFAULT).applyTo(combo);
 
 		fActionParamLabel = new Label(comp, SWT.NONE);
 		fActionParamWidget = new Text(comp, SWT.SINGLE | SWT.BORDER);
-		GridDataFactory.fillDefaults().grab(true, false).applyTo(fActionParamWidget);
+		GridDataFactory.fillDefaults().grab(true, false).minSize(150, SWT.DEFAULT).applyTo(fActionParamWidget);
 		fActionParamWidget.addModifyListener(new ModifyListener() {
 			@Override
 			public void modifyText(ModifyEvent e) {
@@ -326,40 +351,6 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 
 	@Override
 	public void selectionChanged(SelectionChangedEvent event) {
-
-		// This listener gets called for a selection change in the launch
-		// configuration viewer embedded in the dialog. Problem is, there are
-		// numerous viewers--one for each platform debug ILaunchGroup (run,
-		// debug, profile). These viewers are stacked, so only one is ever
-		// visible to the user. During initialization, we get a selection change
-		// notification for every viewer. We need to ignore all but the one that
-		// matters--the visible one.
-
-		Tree topTree = null;
-		final Control topControl = fStackComposite.getTopControl();
-		if (topControl instanceof FilteredTree) {
-			final TreeViewer viewer = ((FilteredTree) topControl).getViewer();
-			if (viewer != null) {
-				topTree = viewer.getTree();
-			}
-		}
-		if (topTree == null) {
-			return;
-		}
-
-		boolean selectionIsForVisibleViewer = false;
-		final Object src = event.getSource();
-		if (src instanceof Viewer) {
-			final Control viewerControl = ((Viewer) src).getControl();
-			if (viewerControl == topTree) {
-				selectionIsForVisibleViewer = true;
-			}
-		}
-
-		if (!selectionIsForVisibleViewer) {
-			return;
-		}
-
 		fSelection = event.getSelection();
 		validate();
 	}
@@ -391,11 +382,22 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 					isValid = !hasSelfRecursive(sel);
 					setErrorMessage(isValid ? null : DebugUIMessages.GroupLaunchConfigurationSelectionDialog_0);
 				}
+				if (isValid && !GroupLaunchElement.MODE_INHERIT.equals(mode)) {
+					if (!sel.supportsMode(mode)) {
+						isValid = false;
+					}
+					setErrorMessage(isValid ? null : DebugUIMessages.GroupLaunchConfigurationSelectionDialog_1);
+				}
+
+				if (!isValid) {
+					break;
+				}
 			}
 		} catch (CoreException e) {
 			DebugUIPlugin.log(e);
 		}
 
+
 		if (isValid) {
 			if (action == GroupElementPostLaunchAction.DELAY) {
 				isValid = (actionParam instanceof Integer) && ((Integer) actionParam > 0);
@@ -418,7 +420,7 @@ class GroupLaunchConfigurationSelectionDialog extends TitleAreaDialog implements
 			return false;
 		}
 
-		if(c.getType().equals(groupType)) {
+		if (c.getType().equals(groupType)) {
 			// it's a launch group
 			if (c.getName().equals(selfRef.getName())) {
 				return true;
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationTabGroup.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationTabGroup.java
index 2aa494741..285909910 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationTabGroup.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/groups/GroupLaunchConfigurationTabGroup.java
@@ -275,7 +275,7 @@ public class GroupLaunchConfigurationTabGroup extends AbstractLaunchConfiguratio
 			setControl(comp);
 			//comp.setBackground(PlatformUI.getWorkbench().getDisplay().getSystemColor(SWT.COLOR_DARK_GREEN));
 			comp.setLayout(new GridLayout(2, false));
-			treeViewer = new CheckboxTreeViewer(comp, SWT.BORDER | SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL);
+			treeViewer = new CheckboxTreeViewer(comp, SWT.BORDER | SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL | SWT.FULL_SELECTION);
 			Tree table = treeViewer.getTree();
 			table.setFont(parent.getFont());
 			treeViewer.setContentProvider(new ContentProvider());