Prefer to change to existing local branch when fetching Gerrit change

When attempting to fetch a Gerrit patch set which the user has already
fetched they are now presented with an option to change to the already
fetched branch. This is the default option if a local branch exists. If
no local branch exists for a ref this option is not available.

Bug: 509181
Change-Id: I81af2906fe44dacb35ca9c23e4cd2583750f3b8a
Signed-off-by: Jaxsun McCarthy Huggan <jaxsun.mccarthy@tasktop.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

3 files changed, 111 insertions, 44 deletions

diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
index 7dc43ea5ad..f8914f36b9 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java
@@ -18,6 +18,7 @@
  * Copyright (C) 2015, Denis Zygann <d.zygann@web.de>
  * Copyright (C) 2016, Lars Vogel <Lars.Vogel@vogella.com>
  * Copyright (C) 2017, Wim Jongman <wim.jongman@remainsoftware.com> bug 358152
+ * Copyright (C) 2016, Jaxsun McCathy Huggan <jaxsun.mccarthy@tasktop.com>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -2834,6 +2835,9 @@ public class UIText extends NLS {
 	public static String FetchGerritChangePage_ChangeLabel;
 
 	/** */
+	public static String FetchGerritChangePage_ChangeToLocalBranchRadio;
+
+	/** */
 	public static String FetchGerritChangePage_CheckingOutTaskName;
 
 	/** */
@@ -2861,7 +2865,7 @@ public class UIText extends NLS {
 	public static String FetchGerritChangePage_GetChangeTaskName;
 
 	/** */
-	public static String FetchGerritChangePage_LocalBranchRadio;
+	public static String FetchGerritChangePage_CreateLocalBranchRadio;
 
 	/** */
 	public static String FetchGerritChangePage_LocalBranchCheckout;
@@ -2879,7 +2883,10 @@ public class UIText extends NLS {
 	public static String FetchGerritChangePage_RunInBackground;
 
 	/** */
-	public static String FetchGerritChangePage_SuggestedRefNamePattern;
+	public static String Change_FullRefNamePattern;
+
+	/** */
+	public static String Change_SuggestedBranchNamePattern;
 
 	/** */
 	public static String FetchGerritChangePage_TagNameText;
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java
index d5a2f8aa9d..7fbe9b365e 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java
@@ -9,6 +9,7 @@
  *    Mathias Kinzler (SAP AG) - initial implementation
  *    Marc Khouzam (Ericsson)  - Add an option not to checkout the new branch
  *    Thomas Wolf <thomas.wolf@paranor.ch> - Bug 493935, 495777
+ *    Jaxsun McCarthy Huggan <jaxsun.mccarthy@tasktop.com> - Bug 509181
  *******************************************************************************/
 package org.eclipse.egit.ui.internal.fetch;
 
@@ -61,6 +62,7 @@ import org.eclipse.jface.resource.JFaceResources;
 import org.eclipse.jface.window.Window;
 import org.eclipse.jface.wizard.IWizardContainer;
 import org.eclipse.jface.wizard.WizardPage;
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
@@ -113,6 +115,8 @@ public class FetchGerritChangePage extends WizardPage {
 
 	private Text refText;
 
+	private Button changeBranch;
+
 	private Button createBranch;
 
 	private Button createTag;
@@ -138,12 +142,15 @@ public class FetchGerritChangePage extends WizardPage {
 	private Button runInBackgroud;
 
 	private IInputValidator branchValidator;
+
 	private IInputValidator tagValidator;
 
 	private Button branchEditButton;
 
 	private Button branchCheckoutButton;
 
+	private Composite main;
+
 	/**
 	 * @param repository
 	 * @param refName initial value for the ref field
@@ -193,7 +200,7 @@ public class FetchGerritChangePage extends WizardPage {
 				candidateChange = determineChangeFromString(clipText.trim());
 			}
 		}
-		Composite main = new Composite(parent, SWT.NONE);
+		main = new Composite(parent, SWT.NONE);
 		main.setLayout(new GridLayout(2, false));
 		GridDataFactory.fillDefaults().grab(true, true).applyTo(main);
 		new Label(main, SWT.NONE)
@@ -225,10 +232,23 @@ public class FetchGerritChangePage extends WizardPage {
 				.applyTo(checkoutGroup);
 		checkoutGroup.setText(UIText.FetchGerritChangePage_AfterFetchGroup);
 
+		// radio: checkout local branch
+		changeBranch = new Button(checkoutGroup, SWT.RADIO);
+		GridDataFactory.fillDefaults().span(3, 1).applyTo(changeBranch);
+		changeBranch
+				.setText(UIText.FetchGerritChangePage_ChangeToLocalBranchRadio);
+		changeBranch.addSelectionListener(new SelectionAdapter() {
+
+			@Override
+			public void widgetSelected(SelectionEvent e) {
+				checkPage();
+			}
+		});
+
 		// radio: create local branch
 		createBranch = new Button(checkoutGroup, SWT.RADIO);
 		GridDataFactory.fillDefaults().span(1, 1).applyTo(createBranch);
-		createBranch.setText(UIText.FetchGerritChangePage_LocalBranchRadio);
+		createBranch.setText(UIText.FetchGerritChangePage_CreateLocalBranchRadio);
 		createBranch.addSelectionListener(new SelectionAdapter() {
 			@Override
 			public void widgetSelected(SelectionEvent e) {
@@ -326,10 +346,13 @@ public class FetchGerritChangePage extends WizardPage {
 			}
 		});
 
-		if ("checkout".equals(defaultCommand)) //$NON-NLS-1$
+		if ("checkout".equals(defaultCommand)) { //$NON-NLS-1$
 			checkout.setSelection(true);
-		else
+		} else if (getLocalRef(refName) != null) {
+			changeBranch.setSelection(true);
+		} else {
 			createBranch.setSelection(true);
+		}
 
 		warningAdditionalRefNotActive = new Composite(main, SWT.NONE);
 		GridDataFactory.fillDefaults().span(2, 1).grab(true, false)
@@ -349,15 +372,18 @@ public class FetchGerritChangePage extends WizardPage {
 			public void modifyText(ModifyEvent e) {
 				Change change = Change.fromRef(refText.getText());
 				if (change != null) {
-					branchText.setText(NLS
-							.bind(UIText.FetchGerritChangePage_SuggestedRefNamePattern,
-									change.getChangeNumber(),
-									change.getPatchSetNumber()));
+					branchText.setText(change.suggestBranchName());
 					tagText.setText(branchText.getText());
 				} else {
 					branchText.setText(""); //$NON-NLS-1$
 					tagText.setText(""); //$NON-NLS-1$
 				}
+				if (getLocalRef(refText.getText()) == null
+						&& changeBranch.getSelection()) {
+					createBranch.setSelection(true);
+				} else {
+					changeBranch.setSelection(true);
+				}
 				checkPage();
 			}
 		});
@@ -519,13 +545,19 @@ public class FetchGerritChangePage extends WizardPage {
 	}
 
 	private void checkPage() {
+		boolean hasLocalBranch = getLocalRef(refText.getText()) != null;
+		changeBranch.setEnabled(hasLocalBranch);
+		changeBranch.setVisible(hasLocalBranch);
+		GridData gd = (GridData) changeBranch.getLayoutData();
+		gd.exclude = !hasLocalBranch;
+
 		boolean createBranchSelected = createBranch.getSelection();
 		branchText.setEnabled(createBranchSelected);
 		branchText.setVisible(createBranchSelected);
 		branchTextlabel.setVisible(createBranchSelected);
 		branchEditButton.setVisible(createBranchSelected);
 		branchCheckoutButton.setVisible(createBranchSelected);
-		GridData gd = (GridData) branchText.getLayoutData();
+		gd = (GridData) branchText.getLayoutData();
 		gd.exclude = !createBranchSelected;
 		gd = (GridData) branchTextlabel.getLayoutData();
 		gd.exclude = !createBranchSelected;
@@ -542,7 +574,6 @@ public class FetchGerritChangePage extends WizardPage {
 		gd.exclude = !createTagSelected;
 		gd = (GridData) tagTextlabel.getLayoutData();
 		gd.exclude = !createTagSelected;
-		branchText.getParent().layout(true);
 
 		boolean showActivateAdditionalRefs = false;
 		showActivateAdditionalRefs = (checkout.getSelection() || dontCheckout
@@ -556,7 +587,8 @@ public class FetchGerritChangePage extends WizardPage {
 		gd = (GridData) warningAdditionalRefNotActive.getLayoutData();
 		gd.exclude = !showActivateAdditionalRefs;
 		warningAdditionalRefNotActive.setVisible(showActivateAdditionalRefs);
-		warningAdditionalRefNotActive.getParent().layout(true);
+
+		main.getParent().layout(true);
 
 		setErrorMessage(null);
 		try {
@@ -580,6 +612,18 @@ public class FetchGerritChangePage extends WizardPage {
 		}
 	}
 
+	private Ref getLocalRef(String ref) {
+		Change change = Change.fromRef(ref);
+		if (change != null) {
+			try {
+				return repository.findRef(change.computeFullRefName());
+			} catch (IOException e) {
+				// ignore
+			}
+		}
+		return null;
+	}
+
 	private List<Change> getRefsForContentAssist()
 			throws InvocationTargetException, InterruptedException {
 		if (changeRefs == null) {
@@ -638,6 +682,7 @@ public class FetchGerritChangePage extends WizardPage {
 		final String uri = uriCombo.getText();
 		final boolean doCheckout = checkout.getSelection();
 		final boolean doCreateTag = createTag.getSelection();
+		final boolean doChangeBranch = changeBranch.getSelection();
 		final boolean doCreateBranch = createBranch.getSelection();
 		final boolean doCheckoutNewBranch = branchCheckoutButton.getSelection();
 		final boolean doActivateAdditionalRefs = (checkout.getSelection() || dontCheckout
@@ -655,9 +700,9 @@ public class FetchGerritChangePage extends WizardPage {
 				public IStatus runInWorkspace(IProgressMonitor monitor) {
 					try {
 						internalDoFetch(spec, uri, doCheckout, doCreateTag,
-								doCreateBranch, doCheckoutNewBranch,
-								doActivateAdditionalRefs, textForTag,
-								textForBranch, monitor);
+								doChangeBranch, doCreateBranch,
+								doCheckoutNewBranch, doActivateAdditionalRefs,
+								textForTag, textForBranch, monitor);
 					} catch (CoreException ce) {
 						return ce.getStatus();
 					} catch (Exception e) {
@@ -686,10 +731,10 @@ public class FetchGerritChangePage extends WizardPage {
 								InterruptedException {
 							try {
 								internalDoFetch(spec, uri, doCheckout,
-											doCreateTag, doCreateBranch,
-											doCheckoutNewBranch,
-										doActivateAdditionalRefs, textForTag,
-										textForBranch, monitor);
+											doCreateTag, doChangeBranch,
+											doCreateBranch,
+										doCheckoutNewBranch, doActivateAdditionalRefs,
+										textForTag, textForBranch, monitor);
 							} catch (RuntimeException e) {
 								throw e;
 							} catch (Exception e) {
@@ -711,9 +756,9 @@ public class FetchGerritChangePage extends WizardPage {
 	}
 
 	private void internalDoFetch(RefSpec spec, String uri, boolean doCheckout,
-			boolean doCreateTag, boolean doCreateBranch,
-			boolean doCheckoutNewBranch, boolean doActivateAdditionalRefs,
-			String textForTag, String textForBranch, IProgressMonitor monitor)
+			boolean doCreateTag, boolean doChangeBranch,
+			boolean doCreateBranch, boolean doCheckoutNewBranch,
+			boolean doActivateAdditionalRefs, String textForTag, String textForBranch, IProgressMonitor monitor)
 			throws IOException, CoreException, URISyntaxException {
 
 		int totalWork = 1;
@@ -725,26 +770,31 @@ public class FetchGerritChangePage extends WizardPage {
 				UIText.FetchGerritChangePage_GetChangeTaskName,
 				totalWork);
 
-		try {
-			RevCommit commit = fetchChange(uri, spec, monitor);
+		if (doChangeBranch) {
+			Ref localRef = getLocalRef(spec.getSource());
+			checkout(localRef.getName(), monitor);
+		} else {
+			try {
+				RevCommit commit = fetchChange(uri, spec, monitor);
 
-			if (doCreateTag)
-				createTag(spec, textForTag, commit, monitor);
+				if (doCreateTag)
+					createTag(spec, textForTag, commit, monitor);
 
-			if (doCreateBranch)
-				createBranch(textForBranch, doCheckoutNewBranch, commit,
-						monitor);
+				if (doCreateBranch)
+					createBranch(textForBranch, doCheckoutNewBranch, commit,
+							monitor);
 
-			if (doCheckout || doCreateTag)
-				checkout(commit.name(), monitor);
+				if (doCheckout || doCreateTag)
+					checkout(commit.name(), monitor);
 
-			if (doActivateAdditionalRefs)
-				activateAdditionalRefs();
+				if (doActivateAdditionalRefs)
+					activateAdditionalRefs();
 
-			storeLastUsedUri(uri);
+				storeLastUsedUri(uri);
 
-		} finally {
-			monitor.done();
+			} finally {
+				monitor.done();
+			}
 		}
 	}
 
@@ -896,9 +946,9 @@ public class FetchGerritChangePage extends WizardPage {
 
 		private final Integer patchSetNumber;
 
-		static Change fromRef(String refName) {
+		static @Nullable Change fromRef(@Nullable String refName) {
 			try {
-				if (!refName.startsWith("refs/changes/")) //$NON-NLS-1$
+				if (refName == null || !refName.startsWith("refs/changes/")) //$NON-NLS-1$
 					return null;
 				String[] tokens = refName.substring(13).split("/"); //$NON-NLS-1$
 				if (tokens.length != 3)
@@ -934,9 +984,16 @@ public class FetchGerritChangePage extends WizardPage {
 			return patchSetNumber;
 		}
 
-		/* (non-Javadoc)
-		 * @see java.lang.Object#toString()
-		 */
+		public String suggestBranchName() {
+			return NLS.bind(UIText.Change_SuggestedBranchNamePattern,
+					changeNumber, patchSetNumber);
+		}
+
+		public String computeFullRefName() {
+			return NLS.bind(UIText.Change_FullRefNamePattern, changeNumber,
+					patchSetNumber);
+		}
+
 		@Override
 		public String toString() {
 			return refName;
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
index fc28db2b6c..0eaa45762d 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties
@@ -20,6 +20,7 @@
 #    Christian Georgi <christian.georgi@sap.com> - Check for empty second line is configurable
 #    Denis Zygann <d.zygann@web.de>           - 473919 Add tooltip text for sorting files action
 #    Wim Jongman <wim.jongman@remainsoftware.com - 358152
+#    Jaxsun McCarthy Huggan <jaxsun.mccarthy@tasktop.com> - 509181 add change to local branch radio button
 ###############################################################################
 AbortRebaseCommand_CancelDialogMessage=The abort operation was canceled
 AbortRebaseCommand_JobName=Aborting Rebase
@@ -952,6 +953,7 @@ FetchGerritChangePage_AfterFetchGroup=Action to perform after fetch
 FetchGerritChangePage_BranchEditButton=Edit Branches...
 FetchGerritChangePage_BranchNameText=Branch &name:
 FetchGerritChangePage_ChangeLabel=&Change:
+FetchGerritChangePage_ChangeToLocalBranchRadio=Checkout local branch already existing for this change
 FetchGerritChangePage_CheckingOutTaskName=Checking out change
 FetchGerritChangePage_CheckoutRadio=Check&out FETCH_HEAD
 FetchGerritChangePage_ContentAssistDescription=Patch set {0} of change {1}
@@ -961,13 +963,14 @@ FetchGerritChangePage_CreatingTagTaskName=Creating tag
 FetchGerritChangePage_FetchingTaskName=Fetching change {0}
 FetchGerritChangePage_GeneratedTagMessage=Generated for Gerrit change {0}
 FetchGerritChangePage_GetChangeTaskName=Get change from Gerrit
-FetchGerritChangePage_LocalBranchRadio=Create a &local branch
+FetchGerritChangePage_CreateLocalBranchRadio=Create a &local branch
 FetchGerritChangePage_LocalBranchCheckout=Checkout new branch
 FetchGerritChangePage_MissingChangeMessage=Please provide a change
 FetchGerritChangePage_PageMessage=Please select a Gerrit URI and change to fetch
 FetchGerritChangePage_PageTitle=Fetch a change from Gerrit into repository {0}
 FetchGerritChangePage_RunInBackground=&Run in background
-FetchGerritChangePage_SuggestedRefNamePattern=change/{0}/{1}
+Change_FullRefNamePattern=refs/heads/change/{0}/{1}
+Change_SuggestedBranchNamePattern=change/{0}/{1}
 FetchGerritChangePage_TagNameText=Tag &name:
 FetchGerritChangePage_TagRadio=Create and checkout a &tag
 FetchGerritChangePage_UpdateRadio=U&pdate FETCH_HEAD only