git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[EGIT PATCH 26/31] Make Clone's SourceBranchPage more user-friendly

From
MZMarek Zawirski <marek.zawirski@gmail.com>
Date
Aug 17, 2008, 20:44 UTC
Message-ID
<1219005852-21496-27-git-send-email-marek.zawirski@gmail.com>
In-Reply-To
<1219005852-21496-26-git-send-email-marek.zawirski@gmail.com>

-Show page description and progress monitor so user get some feedback what's happening when we're listing remote refs (which may take some time... sometimes). -Don't display error during connection to remote repo. -Set page error before showing error dialog (avoid flashing over Our Dear User eyers). -Enable/disable select all and unselect all buttons in response to current selections. -Use ListRemoteOperation instead of our own anonymous class. This introduces some progress information. It also remove unnecessary (wrong?) code for cancellation handling and always close Transport (even if fetch is not supported by Transport).

Perhaps-making-too-big-commit...
Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
---
 .../src/org/spearce/egit/ui/UIText.java            |    9 +-
 .../egit/ui/internal/clone/GitCloneWizard.java     |    1 +
 .../egit/ui/internal/clone/SourceBranchPage.java   |  134 +++++++++-----------
 .../src/org/spearce/egit/ui/uitext.properties      |    3 +-
 4 files changed, 67 insertions(+), 80 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
index 189b769..9b290f3 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java
@@ -119,6 +119,9 @@ public class UIText extends NLS {
 	public static String SourceBranchPage_title;
 
 	/** */
+	public static String SourceBranchPage_description;
+
+	/** */
 	public static String SourceBranchPage_branchList;
 
 	/** */
@@ -134,15 +137,15 @@ public class UIText extends NLS {
 	public static String SourceBranchPage_transportError;
 
 	/** */
-	public static String SourceBranchPage_interrupted;
-
-	/** */
 	public static String SourceBranchPage_cannotListBranches;
 
 	/** */
 	public static String SourceBranchPage_remoteListingCancelled;
 
 	/** */
+	public static String SourceBranchPage_cannotCreateTemp;
+
+	/** */
 	public static String CloneDestinationPage_title;
 
 	/** */
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/GitCloneWizard.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/GitCloneWizard.java
index 10b2cd4..a69dc52 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/GitCloneWizard.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/GitCloneWizard.java
@@ -44,6 +44,7 @@ public class GitCloneWizard extends Wizard implements IImportWizard {
 	public void init(IWorkbench arg0, IStructuredSelection arg1) {
 		setWindowTitle(UIText.GitCloneWizard_title);
 		setDefaultPageImageDescriptor(UIIcons.WIZBAN_IMPORT_REPO);
+		setNeedsProgressMonitor(true);
 		cloneSource = new RepositorySelectionPage(true);
 		validSource = new SourceBranchPage(cloneSource);
 		cloneDestination = new CloneDestinationPage(cloneSource, validSource);
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
index 7710612..030419d 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java
@@ -10,20 +10,16 @@
 package org.spearce.egit.ui.internal.clone;
 
 import java.io.File;
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
-import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.NullProgressMonitor;
-import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.jface.dialogs.ErrorDialog;
-import org.eclipse.jface.operation.IRunnableWithProgress;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.events.SelectionAdapter;
@@ -36,6 +32,7 @@ import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Label;
 import org.eclipse.swt.widgets.Table;
 import org.eclipse.swt.widgets.TableItem;
+import org.spearce.egit.core.op.ListRemoteOperation;
 import org.spearce.egit.ui.Activator;
 import org.spearce.egit.ui.UIText;
 import org.spearce.egit.ui.internal.components.BaseWizardPage;
@@ -45,8 +42,7 @@ import org.spearce.egit.ui.internal.components.SelectionChangeListener;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.Ref;
 import org.spearce.jgit.lib.Repository;
-import org.spearce.jgit.transport.FetchConnection;
-import org.spearce.jgit.transport.Transport;
+import org.spearce.jgit.transport.URIish;
 
 class SourceBranchPage extends BaseWizardPage {
 	private final RepositorySelectionPage sourcePage;
@@ -69,7 +65,8 @@ class SourceBranchPage extends BaseWizardPage {
 		super(SourceBranchPage.class.getName());
 		sourcePage = sp;
 		setTitle(UIText.SourceBranchPage_title);
-
+		setDescription(UIText.SourceBranchPage_description);
+		
 		sourcePage.addSelectionListener(new SelectionChangeListener() {
 			public void selectionChanged() {
 				if (!sourcePage.selectionEquals(validatedRepoSelection))
@@ -134,10 +131,10 @@ class SourceBranchPage extends BaseWizardPage {
 
 		final Composite bPanel = new Composite(panel, SWT.NONE);
 		bPanel.setLayout(new RowLayout());
-		Button b;
-		b = new Button(bPanel, SWT.PUSH);
-		b.setText(UIText.SourceBranchPage_selectAll);
-		b.addSelectionListener(new SelectionAdapter() {
+		final Button selectB;
+		selectB = new Button(bPanel, SWT.PUSH);
+		selectB.setText(UIText.SourceBranchPage_selectAll);
+		selectB.addSelectionListener(new SelectionAdapter() {
 			public void widgetSelected(final SelectionEvent e) {
 				for (int i = 0; i < refsTable.getItemCount(); i++)
 					refsTable.getItem(i).setChecked(true);
@@ -147,9 +144,9 @@ class SourceBranchPage extends BaseWizardPage {
 				checkPage();
 			}
 		});
-		b = new Button(bPanel, SWT.PUSH);
-		b.setText(UIText.SourceBranchPage_selectNone);
-		b.addSelectionListener(new SelectionAdapter() {
+		final Button unselectB = new Button(bPanel, SWT.PUSH);
+		unselectB.setText(UIText.SourceBranchPage_selectNone);
+		unselectB.addSelectionListener(new SelectionAdapter() {
 			public void widgetSelected(final SelectionEvent e) {
 				for (int i = 0; i < refsTable.getItemCount(); i++)
 					refsTable.getItem(i).setChecked(false);
@@ -160,6 +157,13 @@ class SourceBranchPage extends BaseWizardPage {
 		});
 		bPanel.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false));
 
+		addSelectionListener(new SelectionChangeListener() {
+			public void selectionChanged() {
+				selectB.setEnabled(selectedRefs.size() != availableRefs.size());
+				unselectB.setEnabled(selectedRefs.size() != 0);
+			}
+		});
+
 		setControl(panel);
 		checkPage();
 	}
@@ -210,7 +214,8 @@ class SourceBranchPage extends BaseWizardPage {
 		availableRefs.clear();
 		selectedRefs.clear();
 		refsTable.removeAll();
-		checkPage();
+		setPageComplete(false);
+		setErrorMessage(null);
 		label.getDisplay().asyncExec(new Runnable() {
 			public void run() {
 				revalidateImpl(newRepoSelection);
@@ -222,73 +227,50 @@ class SourceBranchPage extends BaseWizardPage {
 		if (label.isDisposed() || !isCurrentPage())
 			return;
 
+		final ListRemoteOperation listRemoteOp; 
 		try {
-			getContainer().run(true, true, new IRunnableWithProgress() {
-				public void run(final IProgressMonitor pm)
-						throws InvocationTargetException, InterruptedException {
-					final IProgressMonitor monitor;
-					if (pm == null)
-						monitor = new NullProgressMonitor();
-					else
-						monitor = pm;
-					try {
-						final Repository db = new Repository(new File("/tmp"));
-						final Transport tn = Transport.open(db,
-								newRepoSelection.getURI());
-						final Collection<Ref> adv;
-						final FetchConnection fn = tn.openFetch();
-						try {
-							adv = fn.getRefs();
-						} finally {
-							fn.close();
-							tn.close();
-						}
-
-						final Ref idHEAD = fn.getRef(Constants.HEAD);
-						head = null;
-						for (final Ref r : adv) {
-							final String n = r.getName();
-							if (!n.startsWith(Constants.HEADS_PREFIX + "/"))
-								continue;
-							availableRefs.add(r);
-							if (idHEAD == null || head != null)
-								continue;
-							if (r.getObjectId().equals(idHEAD.getObjectId()))
-								head = r;
-						}
-						Collections.sort(availableRefs, new Comparator<Ref>() {
-							public int compare(final Ref o1, final Ref o2) {
-								return o1.getName().compareTo(o2.getName());
-							}
-						});
-						if (idHEAD != null && head == null) {
-							head = idHEAD;
-							availableRefs.add(0, idHEAD);
-						}
-					} catch (Exception err) {
-						throw new InvocationTargetException(err);
-					}
-					monitor.done();
-				}
-			});
+			final URIish uri = newRepoSelection.getURI();
+			final Repository db = new Repository(new File("/tmp"));
+			listRemoteOp = new ListRemoteOperation(db, uri);
+			getContainer().run(true, true, listRemoteOp);
 		} catch (InvocationTargetException e) {
 			Throwable why = e.getCause();
-			if ((why instanceof OperationCanceledException)) {
-				transportError(UIText.SourceBranchPage_remoteListingCancelled);
-				return;
-			} else {
-				ErrorDialog.openError(getShell(),
-						UIText.SourceBranchPage_transportError,
-						UIText.SourceBranchPage_cannotListBranches, new Status(
-								IStatus.ERROR, Activator.getPluginId(), 0, why
-										.getMessage(), why.getCause()));
-				transportError(why.getMessage());
-			}
+			transportError(why.getMessage());
+			ErrorDialog.openError(getShell(),
+					UIText.SourceBranchPage_transportError,
+					UIText.SourceBranchPage_cannotListBranches, new Status(
+							IStatus.ERROR, Activator.getPluginId(), 0, why
+									.getMessage(), why.getCause()));
+			return;
+		} catch (IOException e) {
+			transportError(UIText.SourceBranchPage_cannotCreateTemp);
 			return;
 		} catch (InterruptedException e) {
-			transportError(UIText.SourceBranchPage_interrupted);
+			transportError(UIText.SourceBranchPage_remoteListingCancelled);
 			return;
 		}
+		
+		final Ref idHEAD = listRemoteOp.getRemoteRef(Constants.HEAD);
+		head = null;
+		for (final Ref r : listRemoteOp.getRemoteRefs()) {
+			final String n = r.getName();
+			if (!n.startsWith(Constants.HEADS_PREFIX + "/"))
+				continue;
+			availableRefs.add(r);
+			if (idHEAD == null || head != null)
+				continue;
+			if (r.getObjectId().equals(idHEAD.getObjectId()))
+				head = r;
+		}
+		Collections.sort(availableRefs, new Comparator<Ref>() {
+			public int compare(final Ref o1, final Ref o2) {
+				return o1.getName().compareTo(o2.getName());
+			}
+		});
+		if (idHEAD != null && head == null) {
+			head = idHEAD;
+			availableRefs.add(0, idHEAD);
+		}
 
 		validatedRepoSelection = newRepoSelection;
 		for (final Ref r : availableRefs) {
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
index f46e183..55f6c87 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties
@@ -54,14 +54,15 @@ RepositorySelectionPage_fileNotFound={0} does not exist.
 RepositorySelectionPage_internalError=Internal error; consult Eclipse error log.
 
 SourceBranchPage_title=Source Git Repository
+SourceBranchPage_description=Select branches to clone from remote repository.
 SourceBranchPage_branchList=Branches of {0}:
 SourceBranchPage_selectAll=Select All
 SourceBranchPage_selectNone=Deselect All
 SourceBranchPage_errorBranchRequired=At least one branch must be selected.
 SourceBranchPage_transportError=Transport Error
 SourceBranchPage_cannotListBranches=Cannot list the available branches.
-SourceBranchPage_interrupted=Connection attempt interrupted.
 SourceBranchPage_remoteListingCancelled=Operation cancelled
+SourceBranchPage_cannotCreateTemp=Couldn't create temporary repository.
 
 
 CloneDestinationPage_title=Local Destination

-- 
1.5.6.3
Previous: Marek ZawirskiNext: Marek Zawirski
Message 27 of 62 in “Push GUI, GUI improvements, various jgit stuff”
  1. 00/31 Push GUI, GUI improvements, various jgit stuffMarek Zawirski, Aug 17, 2008
  2. 01/31 Fix Repository.mapObject() for missing objectsMarek Zawirski, Aug 17, 2008
  3. 02/31 Fix Repository isValidRefName() for empty namesMarek Zawirski, Aug 17, 2008
  4. 03/31 Fix Repository.resolve() to not throw runtime exceptionsMarek Zawirski, Aug 17, 2008
  5. 04/31 Document/fix Transport open method for specific caseMarek Zawirski, Aug 17, 2008
  6. 05/31 Fix RefSpec javadoc regarding spec expandingMarek Zawirski, Aug 17, 2008
  7. 06/31 Make wildcard checking public in RefSpecMarek Zawirski, Aug 17, 2008
  8. 07/31 Add openAll() and applyConfig() methods to TransportMarek Zawirski, Aug 17, 2008
  9. 08/31 Add dryRun option to Transport and console pushMarek Zawirski, Aug 17, 2008
  10. 09/31 Extract Transport findRemoteRefUpdatesFor() as static methodMarek Zawirski, Aug 17, 2008
  11. 10/31 Improve javadoc of Transport push()Marek Zawirski, Aug 17, 2008
  12. 11/31 Clean up exception issues in RemoteRefUpdateMarek Zawirski, Aug 17, 2008
  13. 12/31 Add another RemoteRefUpdate constructor, useful for 2-stage pushMarek Zawirski, Aug 17, 2008
  14. 13/31 Add getAllRemoteConfigs() to RemoteConfigMarek Zawirski, Aug 17, 2008
  15. 14/31 Add setFetchRefSpecs and setPushRefSpecs to RemoteConfigMarek Zawirski, Aug 17, 2008
  16. 15/31 Add simple abbreviate() method to ObjectIdMarek Zawirski, Aug 17, 2008
  17. 16/31 Remove debug/test console output from GitIndexMarek Zawirski, Aug 17, 2008
  18. 17/31 Fix typo in uitext.properties messageMarek Zawirski, Aug 17, 2008
  19. 18/31 Refactor/rewrite CloneSourcePage to universal RepositorySelectionPageMarek Zawirski, Aug 17, 2008
  20. 19/31 Clone wizard and related: refactor, clean-up, fixes or improvementsMarek Zawirski, Aug 17, 2008
  21. 20/31 Move clone logic away from GitCloneWizard to CloneOperationMarek Zawirski, Aug 17, 2008
  22. 21/31 Add canCreateSubdir() heuristic in CloneDestinationPageMarek Zawirski, Aug 17, 2008
  23. 22/31 Set FileDialog selection appropriately in clone wizardMarek Zawirski, Aug 17, 2008
  24. 23/31 Allow selecting empty dir in clone wizardMarek Zawirski, Aug 17, 2008
  25. 24/31 Clone wizard: force dir to suggested path only if repo selection changeMarek Zawirski, Aug 17, 2008
  26. 25/31 Create ListRemoteOperation for listing remote repo branchesMarek Zawirski, Aug 17, 2008
  27. 26/31 Make Clone's SourceBranchPage more user-friendlyMarek Zawirski, Aug 17, 2008
  28. 27/31 Add few EPL Eclipse iconsMarek Zawirski, Aug 17, 2008
  29. 28/31 Checkbox images/screenshotsMarek Zawirski, Aug 17, 2008
  30. 29/31 Universal GUI for specifications edition: RefSpecPanel and relatedMarek Zawirski, Aug 17, 2008
  31. 30/31 Add PushOperation to pluginMarek Zawirski, Aug 17, 2008
  32. 31/31 Push GUIMarek Zawirski, Aug 17, 2008
  33. Robin RosenbergAug 19, 2008
  34. Shawn O. PearceAug 19, 2008
  35. Shawn O. PearceAug 19, 2008
  36. Shawn O. PearceAug 19, 2008
  37. Robin RosenbergAug 19, 2008
  38. Marek ZawirskiAug 20, 2008
  39. 1/6 Remove DEFAULT_DRY_RUN constant from TransportMarek Zawirski, Aug 20, 2008
  40. 2/6 Emphasize that db is a local one in RemoteRefUpdateMarek Zawirski, Aug 20, 2008
  41. 3/6 Handle URIs parsing errors in PushAction betterMarek Zawirski, Aug 20, 2008
  42. 4/6 Fix proposal provider for fetch in RefSpecPanelMarek Zawirski, Aug 20, 2008
  43. 5/6 Fix disappearing "save configuration" label in RefSpecPageMarek Zawirski, Aug 20, 2008
  44. 6/6 Fix RefSpecPage formattingMarek Zawirski, Aug 20, 2008
  45. Shawn O. PearceAug 20, 2008
  46. Git-aware Issue Tracking?Petr Baudis, Aug 20, 2008
  47. Shawn O. PearceAug 20, 2008
  48. Robin RosenbergAug 20, 2008
  49. Martin LanghoffAug 21, 2008
  50. Jakub NarebskiAug 20, 2008
  51. Pierre HabouzitAug 21, 2008
  52. Jakub NarebskiAug 21, 2008
  53. Mike DalessioAug 20, 2008
  54. Imran M YousufAug 21, 2008
  55. Marek ZawirskiAug 25, 2008
  56. Shawn O. PearceAug 25, 2008
  57. Robin RosenbergAug 21, 2008
  58. Shawn O. PearceAug 21, 2008
  59. Robin RosenbergAug 21, 2008
  60. Shawn O. PearceAug 22, 2008
  61. Marek ZawirskiAug 25, 2008
  62. Shawn O. PearceAug 25, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.