Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2016-07-24 20:18:29 +0000
committerThomas Wolf2016-08-27 12:32:01 +0000
commitbece49f2f67eaac2b91e89848723d85180c6eb9b (patch)
tree02eaea34a085215b84c1383ccb0d9a4bd226c146
parent7069243c32afd4af29cf5218976bf03f85e562cd (diff)
downloadegit-bece49f2f67eaac2b91e89848723d85180c6eb9b.tar.gz
egit-bece49f2f67eaac2b91e89848723d85180c6eb9b.tar.xz
egit-bece49f2f67eaac2b91e89848723d85180c6eb9b.zip
Immediate user feedback on failed push background jobs
With commit 09e0d369 we've eliminated asynchronous dialogs from background jobs for pushing and fetching. Instead, we associate an action to open the result dialog with the job; the user can invoke that action from the progress view. This works nicely, but makes it easy to overlook a failed push. Try to improve that by making the job return an ERROR status when a push failure has occurred. This makes the job/status framework notify the user; the notification dialog will show the failure message and additionally a button giving access to the job's action, which will open the full push result dialog. Bug: 498176 Change-Id: Ia12c8586a0bf3827fe8916d1fa5853cbfb1e7604 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java8
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/jobs/RepositoryJob.java23
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushJob.java131
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java57
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushWizard.java78
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/ShowPushResultAction.java12
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties4
7 files changed, 191 insertions, 122 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 77122751a..05c1c0d93 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
@@ -2176,7 +2176,10 @@ public class UIText extends NLS {
public static String ShowPushResultAction_name;
/** */
- public static String PushWizard_cantConnectToAny;
+ public static String PushJob_cantConnectToAny;
+
+ /** */
+ public static String PushJob_unexpectedError;
/** */
public static String PushWizard_cantPrepareUpdatesMessage;
@@ -2200,9 +2203,6 @@ public class UIText extends NLS {
public static String PushWizard_missingRefsTitle;
/** */
- public static String PushWizard_unexpectedError;
-
- /** */
public static String PushWizard_windowTitleDefault;
/** */
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/jobs/RepositoryJob.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/jobs/RepositoryJob.java
index bd3bf3ad8..4830ca5e2 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/jobs/RepositoryJob.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/jobs/RepositoryJob.java
@@ -16,6 +16,7 @@ import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.egit.ui.Activator;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.jface.action.IAction;
+import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.PlatformUI;
import org.eclipse.ui.progress.IProgressConstants;
@@ -59,8 +60,14 @@ public abstract class RepositoryJob extends Job {
} else {
setProperty(IProgressConstants.KEEP_PROPERTY, Boolean.TRUE);
setProperty(IProgressConstants.ACTION_PROPERTY, action);
- return new Status(IStatus.OK, Activator.getPluginId(),
- IStatus.OK, action.getText(), null);
+ IStatus finalStatus = getDeferredStatus();
+ String msg = finalStatus.getMessage();
+ if (msg == null || msg.isEmpty()) {
+ return new Status(finalStatus.getSeverity(),
+ finalStatus.getPlugin(), finalStatus.getCode(),
+ action.getText(), finalStatus.getException());
+ }
+ return finalStatus;
}
}
return status;
@@ -87,6 +94,18 @@ public abstract class RepositoryJob extends Job {
*/
abstract protected IAction getAction();
+ /**
+ * Obtains an {@link IStatus} describing the final outcome of the operation.
+ * This default implementation returns an {@link IStatus#OK OK} status.
+ *
+ * @return an {@link IStatus} describing the outcome of the job
+ */
+ @NonNull
+ protected IStatus getDeferredStatus() {
+ return new Status(IStatus.OK, Activator.getPluginId(), IStatus.OK, "", //$NON-NLS-1$
+ null);
+ }
+
private boolean isModal() {
Boolean modal = (Boolean) getProperty(
IProgressConstants.PROPERTY_IN_DIALOG);
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushJob.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushJob.java
new file mode 100644
index 000000000..0f034a114
--- /dev/null
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushJob.java
@@ -0,0 +1,131 @@
+/*******************************************************************************
+ * Copyright (C) 2016, Thomas Wolf <thomas.wolf@paranor.ch>
+ *
+ * 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
+ *******************************************************************************/
+package org.eclipse.egit.ui.internal.push;
+
+import java.lang.reflect.InvocationTargetException;
+
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.egit.core.op.PushOperation;
+import org.eclipse.egit.core.op.PushOperationResult;
+import org.eclipse.egit.ui.Activator;
+import org.eclipse.egit.ui.JobFamilies;
+import org.eclipse.egit.ui.internal.UIText;
+import org.eclipse.egit.ui.internal.jobs.RepositoryJob;
+import org.eclipse.jface.action.IAction;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+import org.eclipse.jgit.transport.URIish;
+import org.eclipse.osgi.util.NLS;
+
+/**
+ * Background job for pushing to repositories.
+ */
+public class PushJob extends RepositoryJob {
+
+ private final PushOperation operation;
+
+ private final PushOperationResult resultToCompare;
+
+ private final String destinationString;
+
+ private final Repository localDb;
+
+ private final boolean showConfigureButton;
+
+ private PushOperationResult operationResult;
+
+ /**
+ * Creates a new {@link PushJob} that performs the given
+ * {@link PushOperation} on the given {@link Repository}.
+ *
+ * @param name
+ * of the job
+ * @param repository
+ * to push to
+ * @param operation
+ * to perform
+ * @param expectedResult
+ * of the operation
+ * @param destinationString
+ * describing where to push to
+ * @param showConfigureButton
+ * whether the result dialog should have a configuration button
+ */
+ public PushJob(String name, Repository repository, PushOperation operation,
+ PushOperationResult expectedResult, String destinationString,
+ boolean showConfigureButton) {
+ super(name);
+ this.operation = operation;
+ this.resultToCompare = expectedResult;
+ this.destinationString = destinationString;
+ this.localDb = repository;
+ this.showConfigureButton = showConfigureButton;
+ }
+
+ @Override
+ protected IStatus performJob(final IProgressMonitor monitor) {
+ try {
+ operation.run(monitor);
+ } catch (final InvocationTargetException e) {
+ return new Status(IStatus.ERROR, Activator.getPluginId(),
+ UIText.PushJob_unexpectedError, e.getCause());
+ }
+
+ operationResult = operation.getOperationResult();
+ if (!operationResult.isSuccessfulConnectionForAnyURI()) {
+ return new Status(IStatus.ERROR, Activator.getPluginId(),
+ NLS.bind(UIText.PushJob_cantConnectToAny,
+ operationResult.getErrorStringForAllURis()));
+ }
+
+ return Status.OK_STATUS;
+ }
+
+ @Override
+ protected IAction getAction() {
+ Repository repo = localDb;
+ if (repo != null && (resultToCompare == null
+ || !resultToCompare.equals(operationResult))) {
+ return new ShowPushResultAction(repo, operationResult,
+ destinationString, showConfigureButton);
+ }
+ return null;
+ }
+
+ @Override
+ protected IStatus getDeferredStatus() {
+ for (URIish uri : operationResult.getURIs()) {
+ PushResult outcome = operationResult.getPushResult(uri);
+ for (RemoteRefUpdate update : outcome.getRemoteUpdates()) {
+ switch (update.getStatus()) {
+ case NOT_ATTEMPTED:
+ case UP_TO_DATE:
+ case OK:
+ continue;
+ default:
+ return new Status(IStatus.ERROR, Activator.getPluginId(),
+ IStatus.ERROR, update.getMessage(), null);
+ }
+ }
+ }
+ return super.getDeferredStatus();
+ }
+
+ @Override
+ public boolean belongsTo(Object family) {
+ if (JobFamilies.PUSH.equals(family)) {
+ return true;
+ }
+ return super.belongsTo(family);
+ }
+
+}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
index c948725f0..f859481db 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushOperationUI.java
@@ -19,19 +19,14 @@ import java.util.List;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.egit.core.op.PushOperation;
import org.eclipse.egit.core.op.PushOperationResult;
import org.eclipse.egit.core.op.PushOperationSpecification;
import org.eclipse.egit.ui.Activator;
-import org.eclipse.egit.ui.JobFamilies;
import org.eclipse.egit.ui.UIPreferences;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.egit.ui.internal.credentials.EGitCredentialsProvider;
-import org.eclipse.egit.ui.internal.jobs.RepositoryJob;
-import org.eclipse.jface.action.IAction;
import org.eclipse.jgit.errors.NotSupportedException;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.CredentialsProvider;
@@ -218,48 +213,28 @@ public class PushOperationUI {
}
/**
- * Starts the operation asynchronously showing a confirmation dialog after
- * completion
+ * Starts the operation asynchronously.
*/
public void start() {
final Repository repo = repository;
if (repo == null) {
return;
}
- Job job = new RepositoryJob(NLS.bind(UIText.PushOperationUI_PushJobName,
- destinationString)) {
-
- private PushOperationResult result;
-
- @Override
- protected IStatus performJob(IProgressMonitor monitor) {
- try {
- result = execute(monitor);
- } catch (CoreException e) {
- return Activator.createErrorStatus(e.getStatus()
- .getMessage(), e);
- }
- return Status.OK_STATUS;
- }
-
- @Override
- protected IAction getAction() {
- if (expectedResult == null || !expectedResult.equals(result)) {
- return new ShowPushResultAction(repo, result,
- destinationString, showConfigureButton);
- }
- return null;
- }
-
- @Override
- public boolean belongsTo(Object family) {
- if (JobFamilies.PUSH.equals(family)) {
- return true;
- }
- return super.belongsTo(family);
- }
-
- };
+ try {
+ createPushOperation();
+ } catch (CoreException e) {
+ Activator.showErrorStatus(e.getLocalizedMessage(), e.getStatus());
+ return;
+ }
+ if (credentialsProvider != null) {
+ op.setCredentialsProvider(credentialsProvider);
+ } else {
+ op.setCredentialsProvider(new EGitCredentialsProvider());
+ }
+ Job job = new PushJob(
+ NLS.bind(UIText.PushOperationUI_PushJobName, destinationString),
+ repo, op, expectedResult, destinationString,
+ showConfigureButton);
job.setUser(true);
job.schedule();
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushWizard.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushWizard.java
index ebf1cd5b9..dcb5a8cdd 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushWizard.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/PushWizard.java
@@ -12,13 +12,11 @@
package org.eclipse.egit.ui.internal.push;
import java.io.IOException;
-import java.lang.reflect.InvocationTargetException;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
-import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.Job;
@@ -27,7 +25,6 @@ import org.eclipse.egit.core.op.PushOperationResult;
import org.eclipse.egit.core.op.PushOperationSpecification;
import org.eclipse.egit.core.securestorage.UserPasswordCredentials;
import org.eclipse.egit.ui.Activator;
-import org.eclipse.egit.ui.JobFamilies;
import org.eclipse.egit.ui.UIPreferences;
import org.eclipse.egit.ui.internal.SecureStoreUtils;
import org.eclipse.egit.ui.internal.UIIcons;
@@ -36,8 +33,6 @@ import org.eclipse.egit.ui.internal.components.RefSpecPage;
import org.eclipse.egit.ui.internal.components.RepositorySelection;
import org.eclipse.egit.ui.internal.components.RepositorySelectionPage;
import org.eclipse.egit.ui.internal.credentials.EGitCredentialsProvider;
-import org.eclipse.egit.ui.internal.jobs.RepositoryJob;
-import org.eclipse.jface.action.IAction;
import org.eclipse.jface.dialogs.ErrorDialog;
import org.eclipse.jface.wizard.IWizardPage;
import org.eclipse.jface.wizard.Wizard;
@@ -165,12 +160,16 @@ public class PushWizard extends Wizard {
operation.setCredentialsProvider(new EGitCredentialsProvider(
credentials.getUser(), credentials.getPassword()));
final PushOperationResult resultToCompare;
- if (confirmPage.isShowOnlyIfChangedSelected())
+ if (confirmPage.isShowOnlyIfChangedSelected()) {
resultToCompare = confirmPage.getConfirmedResult();
- else
+ } else {
resultToCompare = null;
- final Job job = new PushJob(localDb, operation, resultToCompare,
- getDestinationString(repoPage.getSelection()));
+ }
+ final Job job = new PushJob(
+ NLS.bind(UIText.PushWizard_jobName,
+ getURIsString(operation.getSpecification().getURIs())),
+ localDb, operation, resultToCompare,
+ getDestinationString(repoPage.getSelection()), true);
job.setUser(true);
job.schedule();
@@ -272,65 +271,4 @@ public class PushWizard extends Wizard {
return destination;
}
- static class PushJob extends RepositoryJob {
- private final PushOperation operation;
-
- private final PushOperationResult resultToCompare;
-
- private final String destinationString;
-
- private Repository localDb;
-
- private PushOperationResult operationResult;
-
- public PushJob(final Repository localDb, final PushOperation operation,
- final PushOperationResult resultToCompare,
- final String destinationString) {
- super(NLS.bind(UIText.PushWizard_jobName, getURIsString(operation
- .getSpecification().getURIs())));
- this.operation = operation;
- this.resultToCompare = resultToCompare;
- this.destinationString = destinationString;
- this.localDb = localDb;
- }
-
- @Override
- protected IStatus performJob(final IProgressMonitor monitor) {
- try {
- operation.run(monitor);
- } catch (final InvocationTargetException e) {
- return new Status(IStatus.ERROR, Activator.getPluginId(),
- UIText.PushWizard_unexpectedError, e.getCause());
- }
-
- operationResult = operation.getOperationResult();
- if (!operationResult.isSuccessfulConnectionForAnyURI()) {
- return new Status(IStatus.ERROR, Activator.getPluginId(),
- NLS.bind(UIText.PushWizard_cantConnectToAny,
- operationResult.getErrorStringForAllURis()));
- }
-
- return Status.OK_STATUS;
- }
-
- @Override
- protected IAction getAction() {
- Repository repo = localDb;
- if (repo != null && (resultToCompare == null
- || !resultToCompare.equals(operationResult))) {
- return new ShowPushResultAction(repo, operationResult,
- destinationString, true);
- }
- return null;
- }
-
- @Override
- public boolean belongsTo(Object family) {
- if (JobFamilies.PUSH.equals(family)) {
- return true;
- }
- return super.belongsTo(family);
- }
-
- }
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/ShowPushResultAction.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/ShowPushResultAction.java
index b5e101cc4..d103d2fe3 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/ShowPushResultAction.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/push/ShowPushResultAction.java
@@ -12,6 +12,7 @@ import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.egit.ui.internal.jobs.RepositoryJobResultAction;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.swt.SWT;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.ui.PlatformUI;
@@ -49,12 +50,17 @@ public class ShowPushResultAction extends RepositoryJobResultAction {
this.showConfigure = showConfigureButton;
}
+ private boolean isModal(Shell shell) {
+ return (shell.getStyle() & (SWT.APPLICATION_MODAL | SWT.PRIMARY_MODAL
+ | SWT.SYSTEM_MODAL)) != 0;
+ }
+
@Override
- protected void showResult(Repository repository) {
- Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow()
+ protected void showResult(@NonNull Repository repository) {
+ Shell shell = PlatformUI.getWorkbench().getModalDialogShellProvider()
.getShell();
PushResultDialog dialog = new PushResultDialog(shell, repository,
- operationResult, destination, false);
+ operationResult, destination, isModal(shell));
dialog.showConfigureButton(showConfigure);
dialog.open();
}
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 8dc34c313..b4ba05a3d 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
@@ -756,7 +756,8 @@ RepositoryJob_NullStatus=Internal error: job ''{0}'' returned a null status.
RepositoryJobResultAction_RepositoryGone=Cannot find git repository ''{0}'' anymore: result cannot be shown.
ShowPushResultAction_name=Show Push Result...
-PushWizard_cantConnectToAny=Can''t connect to any repository: {0}
+PushJob_cantConnectToAny=Can''t connect to any repository: {0}
+PushJob_unexpectedError=Unexpected error occurred.
PushWizard_cantPrepareUpdatesMessage=Can't resolve ref specifications locally (local refs changed?) or create tracking ref update.
PushWizard_cantPrepareUpdatesTitle=Preparing Ref Updates Error
PushWizard_cantSaveMessage=Couldn't save specified specifications in configuration file.
@@ -764,7 +765,6 @@ PushWizard_cantSaveTitle=Configuration Storage Warning
PushWizard_jobName=Pushing to {0}
PushWizard_missingRefsMessage=Ref specifications don't match any source ref (local refs changed?).
PushWizard_missingRefsTitle=Missing Refs Error
-PushWizard_unexpectedError=Unexpected error occurred.
PushWizard_windowTitleDefault=Push to Another Repository
PushWizard_windowTitleWithDestination=Push to: {0}

Back to the top