From 3643937dec912c8da636e659c75b587b2bcd3d19 Mon Sep 17 00:00:00 2001 From: Jens Baumgart Date: Thu, 6 May 2010 17:28:23 +0200 Subject: [PATCH] Improve DiscardChangesAction Currently DiscardChangesAction (Trigger: Replace With->File in Git Index) runs in the UI thread. This might block the UI. Furthermore the discard operation is located in the action. Now DiscardChangesAction uses a Job to execute. The discard operation was moved in a new class DiscardChangesOperation. Change-Id: I06fe4efa096ae4ca6b1110c2f7259243e1d2c99d Signed-off-by: Jens Baumgart Signed-off-by: Chris Aniszczyk --- .../src/org/eclipse/egit/core/CoreText.java | 21 ++ .../src/org/eclipse/egit/core/coretext.properties | 7 + .../egit/core/internal/util/ProjectUtil.java | 29 ++- .../egit/core/op/DiscardChangesOperation.java | 212 +++++++++++++++++++++ .../src/org/eclipse/egit/ui/UIText.java | 3 + .../ui/internal/actions/DiscardChangesAction.java | 207 +++++++------------- .../src/org/eclipse/egit/ui/uitext.properties | 1 + 7 files changed, 343 insertions(+), 137 deletions(-) create mode 100644 org.eclipse.egit.core/src/org/eclipse/egit/core/op/DiscardChangesOperation.java rewrite org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesAction.java (65%) diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/CoreText.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/CoreText.java index 1624d60d..9de35c94 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/CoreText.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/CoreText.java @@ -47,6 +47,24 @@ public class CoreText extends NLS { public static String ConnectProviderOperation_ConnectingProject; /** */ + public static String DiscardChangesOperation_discardFailed; + + /** */ + public static String DiscardChangesOperation_discardFailedSeeLog; + + /** */ + public static String DiscardChangesOperation_discardingChanges; + + /** */ + public static String DiscardChangesOperation_refreshFailed; + + /** */ + public static String DiscardChangesOperation_repoNotFound; + + /** */ + public static String DiscardChangesOperation_writeIndexFailed; + + /** */ public static String DisconnectProviderOperation_disconnecting; /** */ @@ -191,6 +209,9 @@ public class CoreText extends NLS { public static String ProjectUtil_refreshingProjects; /** */ + public static String ProjectUtil_refreshing; + + /** */ public static String PushOperation_resultCancelled; /** */ diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/coretext.properties b/org.eclipse.egit.core/src/org/eclipse/egit/core/coretext.properties index 576d730b..1076c640 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/coretext.properties +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/coretext.properties @@ -12,6 +12,12 @@ CommitFileRevision_errorLookingUpPath=IO error looking up path {1} in {0}. ConnectProviderOperation_connecting=Connecting Git team provider. ConnectProviderOperation_ConnectingProject=Connecting project {0} +DiscardChangesOperation_discardFailed=Discarding changes of {0} failed +DiscardChangesOperation_discardFailedSeeLog=Discarding changes failed. See log for details +DiscardChangesOperation_discardingChanges=Discarding changes +DiscardChangesOperation_refreshFailed=Refreshing resources failed +DiscardChangesOperation_repoNotFound=Repository not found +DiscardChangesOperation_writeIndexFailed=Writing index failed DisconnectProviderOperation_disconnecting=Disconnecting Git team provider. UpdateJob_updatingIndex=Update index @@ -79,6 +85,7 @@ IndexFileRevision_indexEntryNotFound=Git index entry for path {1} not found ListRemoteOperation_title=Getting remote branches information ProjectUtil_refreshingProjects=Refreshing projects +ProjectUtil_refreshing=Refreshing PushOperation_resultCancelled=Operation was cancelled. PushOperation_resultNotSupported=Can't push to {0} PushOperation_resultTransportError=Transport error occured during push operation: {0} diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/util/ProjectUtil.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/util/ProjectUtil.java index 246b5040..8585f195 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/util/ProjectUtil.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/util/ProjectUtil.java @@ -21,7 +21,7 @@ import org.eclipse.jgit.lib.Repository; /** * This class contains utility methods related to projects - * + * TODO: rename to RefreshUtil or ResourceUtil? */ public class ProjectUtil { /** @@ -53,4 +53,31 @@ public class ProjectUtil { monitor.done(); } } + + /** + * The method refreshes resources + * + * @param resources + * resources to refresh + * @param monitor + * @throws CoreException + */ + public static void refreshResources(IResource[] resources, + IProgressMonitor monitor) throws CoreException { + try { + monitor.beginTask(CoreText.ProjectUtil_refreshing, + resources.length); + for (IResource resource : resources) { + if (monitor.isCanceled()) + break; + resource.refreshLocal(IResource.DEPTH_INFINITE, + new SubProgressMonitor(monitor, 1)); + monitor.worked(1); + } + } finally { + monitor.done(); + } + + } + } diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/DiscardChangesOperation.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/DiscardChangesOperation.java new file mode 100644 index 00000000..8dd576e1 --- /dev/null +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/DiscardChangesOperation.java @@ -0,0 +1,212 @@ +/******************************************************************************* + * Copyright (C) 2010, Jens Baumgart + * Copyright (C) 2010, Roland Grunberg + * + * 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 + * + * Code extracted from org.eclipse.egit.ui.internal.actions.DiscardChangesAction + * and reworked. + *******************************************************************************/ +package org.eclipse.egit.core.op; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.core.resources.IContainer; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.IResourceRuleFactory; +import org.eclipse.core.resources.IWorkspace; +import org.eclipse.core.resources.IWorkspaceRunnable; +import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.SubProgressMonitor; +import org.eclipse.core.runtime.jobs.ISchedulingRule; +import org.eclipse.core.runtime.jobs.MultiRule; +import org.eclipse.egit.core.Activator; +import org.eclipse.egit.core.CoreText; +import org.eclipse.egit.core.internal.util.ProjectUtil; +import org.eclipse.egit.core.project.RepositoryMapping; +import org.eclipse.jgit.lib.GitIndex; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.GitIndex.Entry; +import org.eclipse.osgi.util.NLS; + +/** + * The operation discards changes on a set of resources. In case of a folder + * resource all file resources in the sub tree are processed. + */ +public class DiscardChangesOperation implements IEGitOperation { + + IResource[] files; + + ISchedulingRule schedulingRule; + + /** + * Construct a {@link DiscardChangesOperation} object. + * + * @param files + */ + public DiscardChangesOperation(IResource[] files) { + this.files = files; + schedulingRule = calcRefreshRule(files); + } + + /** + * @return the rule needed to execute this operation + */ + public ISchedulingRule getSchedulingRule() { + return schedulingRule; + } + + private static ISchedulingRule calcRefreshRule(IResource[] resources) { + List rules = new ArrayList(); + IResourceRuleFactory ruleFactory = ResourcesPlugin.getWorkspace() + .getRuleFactory(); + for (IResource resource : resources) { + ISchedulingRule rule = ruleFactory.refreshRule(resource); + if (rule != null) + rules.add(rule); + } + if (rules.size() == 0) + return null; + else + return new MultiRule(rules.toArray(new IResource[rules.size()])); + } + + public void execute(IProgressMonitor monitor) throws CoreException { + IWorkspaceRunnable action = new IWorkspaceRunnable() { + public void run(IProgressMonitor monitor) throws CoreException { + discardChanges(monitor); + } + }; + ResourcesPlugin.getWorkspace().run(action, getSchedulingRule(), + IWorkspace.AVOID_UPDATE, monitor); + } + + private void discardChanges(IProgressMonitor monitor) throws CoreException { + monitor.beginTask(CoreText.DiscardChangesOperation_discardingChanges, 3); + boolean errorOccured = false; + List allFiles = new ArrayList(); + // find all files + for (IResource res : files) { + allFiles.addAll(getAllMembers(res)); + } + Set modifiedIndexes = new HashSet(); + for (IResource res : allFiles) { + Repository repo = getRepository(res); + if (repo == null) { + IStatus status = Activator.error( + CoreText.DiscardChangesOperation_repoNotFound, null); + throw new CoreException(status); + } + try { + discardChange(res, repo, modifiedIndexes); + } catch (IOException e) { + errorOccured = true; + String message = NLS.bind( + CoreText.DiscardChangesOperation_discardFailed, res + .getFullPath()); + Activator.logError(message, e); + } + } + monitor.worked(1); + for (GitIndex index : modifiedIndexes) { + try { + index.write(); + } catch (IOException e) { + errorOccured = true; + Activator.logError( + CoreText.DiscardChangesOperation_writeIndexFailed, e); + } + } + monitor.worked(1); + try { + ProjectUtil.refreshResources(files, new SubProgressMonitor(monitor, + 1)); + } catch (CoreException e) { + errorOccured = true; + Activator.logError(CoreText.DiscardChangesOperation_refreshFailed, + e); + } + monitor.worked(1); + monitor.done(); + if (errorOccured) { + IStatus status = Activator.error( + CoreText.DiscardChangesOperation_discardFailedSeeLog, null); + throw new CoreException(status); + } + } + + private static Repository getRepository(IResource resource) { + IProject project = resource.getProject(); + RepositoryMapping repositoryMapping = RepositoryMapping + .getMapping(project); + if (repositoryMapping != null) + return repositoryMapping.getRepository(); + else + return null; + } + + private void discardChange(IResource res, Repository repository, + Set modifiedIndexes) throws IOException { + String resRelPath = RepositoryMapping.getMapping(res) + .getRepoRelativePath(res); + + Entry e = repository.getIndex().getEntry(resRelPath); + // resource must exist in the index and be dirty + if (e != null && e.getStage() == 0 + && e.isModified(repository.getWorkDir())) { + GitIndex index = repository.getIndex(); + index.checkoutEntry(repository.getWorkDir(), e); + modifiedIndexes.add(index); + } + } + + /** + * @param res + * an IResource + * @return An ArrayList with all members of this IResource of arbitrary + * depth. This will return just the argument res if it is a file. + */ + private ArrayList getAllMembers(IResource res) { + ArrayList ret = new ArrayList(); + if (res.getLocation().toFile().isFile()) { + ret.add(res); + } else { + getAllMembersHelper(res, ret); + } + return ret; + } + + private void getAllMembersHelper(IResource res, ArrayList ret) { + if (res instanceof IContainer) { + ArrayList tmp = new ArrayList(); + IContainer cont = (IContainer) res; + try { + for (IResource r : cont.members()) { + if (r.getLocation().toFile().isFile()) { + tmp.add(r); + } else { + getAllMembersHelper(r, tmp); + } + } + } catch (CoreException e) { + // thrown by members() + // ignore children in case parent resource no longer accessible + return; + } + + ret.addAll(tmp); + } + } + +} diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java index 2ce6989a..1c209074 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/UIText.java @@ -1723,6 +1723,9 @@ public class UIText extends NLS { public static String DiscardChangesAction_confirmActionMessage; /** */ + public static String DiscardChangesAction_discardChanges; + + /** */ public static String DiscardChangesAction_unexpectedErrorTitle; /** */ diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesAction.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesAction.java dissimilarity index 65% index a169a7d5..c2ca63ef 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesAction.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/actions/DiscardChangesAction.java @@ -1,136 +1,71 @@ -/******************************************************************************* - * Copyright (C) 2010, Roland Grunberg - * - * 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.actions; - -import java.io.IOException; -import java.util.ArrayList; - -import org.eclipse.core.resources.IContainer; -import org.eclipse.core.resources.IProject; -import org.eclipse.core.resources.IResource; -import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.NullProgressMonitor; -import org.eclipse.egit.core.project.RepositoryMapping; -import org.eclipse.egit.ui.Activator; -import org.eclipse.egit.ui.UIText; -import org.eclipse.jface.action.IAction; -import org.eclipse.jface.dialogs.MessageDialog; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.RepositoryState; -import org.eclipse.jgit.lib.GitIndex.Entry; - -/** - * Checkout all selected dirty files. - */ -public class DiscardChangesAction extends RepositoryAction{ - - @Override - public void execute(IAction action) { - - boolean performAction = MessageDialog.openConfirm(getShell(), - UIText.DiscardChangesAction_confirmActionTitle, - UIText.DiscardChangesAction_confirmActionMessage); - if (performAction) { - performDiscardChanges(); - } - } - - private void performDiscardChanges() { - ArrayList allFiles = new ArrayList(); - - // find all files - for (IResource res : getSelectedResources()) { - allFiles.addAll(getAllMembers(res)); - } - - for (IResource res : allFiles) { - try { - discardChange(res); - } catch (IOException e1) { - Activator.handleError(UIText.DiscardChangesAction_unexpectedErrorMessage, e1, true); - }catch (RuntimeException e2) { - Activator.handleError(UIText.DiscardChangesAction_unexpectedIndexErrorMessage, e2, true); - } - } - - } - - private void discardChange(IResource res) throws IOException { - IProject[] proj = new IProject[] { res.getProject() }; - Repository repository = getRepositoriesFor(proj)[0]; - - String resRelPath = RepositoryMapping.getMapping(res).getRepoRelativePath(res); - Entry e = repository.getIndex().getEntry(resRelPath); - - // resource must exist in the index and be dirty - if (e != null && e.getStage() == 0 && e.isModified(repository.getWorkDir())) { - repository.getIndex().checkoutEntry(repository.getWorkDir(), e); - - try { - res.refreshLocal(0, new NullProgressMonitor()); - } catch (CoreException e1) { - Activator.handleError(UIText.DiscardChangesAction_refreshErrorMessage, e1, true); - } - - repository.getIndex().write(); - } - } - - @Override - public boolean isEnabled() { - for (IResource res : getSelectedResources()) { - IProject[] proj = new IProject[] { res.getProject() }; - Repository repository = getRepositoriesFor(proj)[0]; - if (! repository.getRepositoryState().equals(RepositoryState.SAFE)){ - return false; - } - } - return true; - } - - /** - * @param res an IResource - * @return An ArrayList with all members of this IResource - * of arbitrary depth. This will return just the argument - * res if it is a file. - */ - private ArrayList getAllMembers(IResource res) { - ArrayList ret = new ArrayList(); - if (res.getLocation().toFile().isFile()) { - ret.add(res); - } else { - getAllMembersHelper(res, ret); - } - return ret; - } - - - private void getAllMembersHelper(IResource res, ArrayList ret) { - ArrayList tmp = new ArrayList (); - if (res instanceof IContainer) { - IContainer cont = (IContainer) res; - try { - for (IResource r : cont.members()) { - if (r.getLocation().toFile().isFile()) { - tmp.add(r); - } else { - getAllMembersHelper(r, tmp); - } - } - } catch (CoreException e) { - // thrown by members() - // ignore children in case parent resource no longer accessible - return; - } - - ret.addAll(tmp); - } - } - -} +/******************************************************************************* + * Copyright (C) 2010, Roland Grunberg + * + * 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.actions; + +import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.IResource; +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.DiscardChangesOperation; +import org.eclipse.egit.ui.Activator; +import org.eclipse.egit.ui.UIText; +import org.eclipse.jface.action.IAction; +import org.eclipse.jface.dialogs.MessageDialog; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.RepositoryState; + +/** + * Checkout all selected dirty files. + */ +public class DiscardChangesAction extends RepositoryAction { + + @Override + public void execute(IAction action) { + + boolean performAction = MessageDialog.openConfirm(getShell(), + UIText.DiscardChangesAction_confirmActionTitle, + UIText.DiscardChangesAction_confirmActionMessage); + if (!performAction) + return; + final DiscardChangesOperation operation = new DiscardChangesOperation( + getSelectedResources()); + String jobname = UIText.DiscardChangesAction_discardChanges; + Job job = new Job(jobname) { + @Override + protected IStatus run(IProgressMonitor monitor) { + try { + operation.execute(monitor); + } catch (CoreException e) { + return Activator.createErrorStatus(e.getStatus() + .getMessage(), e); + } + return Status.OK_STATUS; + } + }; + job.setUser(true); + job.setRule(operation.getSchedulingRule()); + job.schedule(); + } + + @Override + public boolean isEnabled() { + for (IResource res : getSelectedResources()) { + IProject[] proj = new IProject[] { res.getProject() }; + Repository repository = getRepositoriesFor(proj)[0]; + if (!repository.getRepositoryState().equals(RepositoryState.SAFE)) { + return false; + } + } + return true; + } + +} diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties index f91a8481..6139155c 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/uitext.properties @@ -616,6 +616,7 @@ RepositoriesViewLabelProvider_TagsNodeText=Tags DiscardChangesAction_confirmActionTitle=Discard Local Changes DiscardChangesAction_confirmActionMessage=This will discard all local changes for the selected resources. Are you sure you want to do this ? +DiscardChangesAction_discardChanges=Discard Changes DiscardChangesAction_unexpectedErrorTitle=Unexpected Error DiscardChangesAction_unexpectedErrorMessage=An unexpected error occured while attempting to checkout resources. DiscardChangesAction_unexpectedIndexErrorMessage=An unexpected error occured while attempting to interact with the Git Index. -- 2.11.4.GIT