Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2018-06-26 16:44:45 -0400
committerThomas Wolf2018-07-15 09:46:53 -0400
commit0da9aa76e5fb1f2af437d9b21f87bb66f0e15c96 (patch)
tree0a9c57dd51cb7211e39b11bdfcbd2ffe812002f3 /org.eclipse.egit.ui/src/org
parent70d01ef871f64de26390f243613beb5ad6ac8978 (diff)
downloadegit-0da9aa76e5fb1f2af437d9b21f87bb66f0e15c96.tar.gz
egit-0da9aa76e5fb1f2af437d9b21f87bb66f0e15c96.tar.xz
egit-0da9aa76e5fb1f2af437d9b21f87bb66f0e15c96.zip
Git history page: move expensive operations in the background
There were still two potentially expensive operations occurring in the UI thread: * Computing the file diffs can take quite a while for large commits. * Preparing to load the history can take quite a while if there are many refs. Because of the highly asynchronous loading already going on for other things (commit messages, unified file diffs, actually loading the history) and the somewhat hacky way the various viewers were re-set when something changed, it was not possible to do this in several commits and still have everything working correctly. The old code stored the repository to use before the new history was loaded, which sometimes could lead to cases where EGit would end up looking for an object in the wrong repository when the repository had changed. Concerning file diffs: we exploit the fact that since commit 9df32ed a FileDiff carries its repository, and we wrap all items needed to compute the file diffs in a FileDiffInput object (tree walk, repo, interesting paths, and so on). The diff calculation is done newly in a background job, and only the actual update of the viewer happens in the UI thread. Because the diff computation is now asynchronous, revealing and selecting interesting elements had to be moved to this UI update triggered by the background job. Incidentally this part of the code became much simpler. One problem here is that computing diffs is not cancelable in JGit. The background jobs must be serialized because they may re-use the same TreeWalk instance. This may lead to longer delays than desired until the file diff viewer updates, but at least he UI will not be blocked. To remedy this fully, DiffEntry.scan() and RenameDetector.compute() in JGit should be made cancelable. See bug 536323 and bug 536324. Concerning the history preparation: marking the start points for the RevWalk can be a rather expensive operation if there are many refs. Use a customized RevWalk for the history that does this on the very first call to next(). That way it gets executed in the GenerateHistoryJob instead of in the UI thread. For the commit message viewer, a similar problem with trying to look up a commit in the wrong repository became apparent. This was resolved by not storing the repo in the viewer itself but relying on the fact that an SWTCommit also carries its repository along. Finally, asynchronously loading the history meant that there was some time during which some parts of the git history page had already been prepared for a new repository now being loaded, but the table would still show the old entries. During that time a user could select one of these old commits and try to invoke commands on it, which then also could result in looking up a commit in the wrong repository. This is resolved now by clearing the table when the repository changes. Bug: 440588 Bug: 485743 Change-Id: I5c3cf9a7a0ae536000a9bf4542407fea0b741e22 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Diffstat (limited to 'org.eclipse.egit.ui/src/org')
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java6
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitEditorPage.java1
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/RepositoryCommit.java4
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/StashEditorPage.java1
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitFileDiffViewer.java164
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitMessageViewer.java38
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiff.java89
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffContentProvider.java213
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffInput.java88
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java182
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryWalk.java145
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/SWTCommit.java2
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties2
13 files changed, 598 insertions, 337 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 78abd8041..f0ceb6a3d 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
@@ -3046,9 +3046,15 @@ public class UIText extends NLS {
public static String FetchWizard_windowTitleWithSource;
/** */
+ public static String FileDiffContentProvider_computingFileDiffs;
+
+ /** */
public static String FileDiffContentProvider_errorGettingDifference;
/** */
+ public static String FileDiffContentProvider_updatingFileDiffs;
+
+ /** */
public static String FileDiffLabelProvider_RenamedFromToolTip;
/** */
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitEditorPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitEditorPage.java
index bc5812cd5..89461a462 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitEditorPage.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/CommitEditorPage.java
@@ -522,7 +522,6 @@ public class CommitEditorPage extends FormPage
.grab(true, true).applyTo(control);
addToFocusTracking(control);
diffViewer.setContentProvider(ArrayContentProvider.getInstance());
- diffViewer.setTreeWalk(getCommit().getRepository(), null);
updateSectionClient(diffSection, filesArea, toolkit);
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/RepositoryCommit.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/RepositoryCommit.java
index a09400db5..5370dea5a 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/RepositoryCommit.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/RepositoryCommit.java
@@ -153,7 +153,7 @@ public class RepositoryCommit extends WorkbenchAdapter
for (RevCommit parent : commit.getParents())
revWalk.parseBody(parent);
diffs = FileDiff.compute(repository, treewalk, commit, parents,
- TreeFilter.ALL);
+ null, TreeFilter.ALL);
} catch (IOException e) {
diffs = new FileDiff[0];
}
@@ -177,7 +177,7 @@ public class RepositoryCommit extends WorkbenchAdapter
treewalk.setFilter(TreeFilter.ANY_DIFF);
loadParents();
diffsResult = FileDiff.compute(repository, treewalk, commit,
- parents, TreeFilter.ALL);
+ parents, null, TreeFilter.ALL);
} catch (IOException e) {
diffsResult = new FileDiff[0];
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/StashEditorPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/StashEditorPage.java
index 0017b316a..b0cba74ca 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/StashEditorPage.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/commit/StashEditorPage.java
@@ -110,7 +110,6 @@ public class StashEditorPage extends CommitEditorPage {
addToFocusTracking(control);
stagedDiffViewer.setContentProvider(ArrayContentProvider
.getInstance());
- stagedDiffViewer.setTreeWalk(getCommit().getRepository(), null);
updateSectionClient(stagedDiffSection, unstagedChangesArea, toolkit);
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitFileDiffViewer.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitFileDiffViewer.java
index 7c7ac6129..2b73ac854 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitFileDiffViewer.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitFileDiffViewer.java
@@ -4,7 +4,7 @@
* Copyright (C) 2012, 2013 Robin Stocker <robin@nibor.org>
* Copyright (C) 2012, Gunnar Wagenknecht <gunnar@wagenknecht.org>
* Copyright (C) 2013, Laurent Goubet <laurent.goubet@obeo.fr>
- * Copyright (C) 2016, Thomas Wolf <thomas.wolf@paranor.ch>
+ * Copyright (C) 2016, 2018 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 2.0
@@ -18,9 +18,9 @@ package org.eclipse.egit.ui.internal.history;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Iterator;
import java.util.List;
-import java.util.Set;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IResource;
@@ -56,13 +56,11 @@ import org.eclipse.jface.viewers.StructuredSelection;
import org.eclipse.jface.viewers.TableViewer;
import org.eclipse.jface.viewers.Viewer;
import org.eclipse.jface.viewers.ViewerComparator;
-import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.osgi.util.NLS;
import org.eclipse.swt.SWT;
import org.eclipse.swt.dnd.Clipboard;
@@ -95,10 +93,6 @@ import org.eclipse.ui.themes.ColorUtil;
public class CommitFileDiffViewer extends TableViewer {
private static final String LINESEP = System.getProperty("line.separator"); //$NON-NLS-1$
- private Repository db;
-
- private TreeWalk walker;
-
private Clipboard clipboard;
private IAction selectAll;
@@ -259,9 +253,11 @@ public class CommitFileDiffViewer extends TableViewer {
return;
final IStructuredSelection iss = (IStructuredSelection) s;
for (Iterator<FileDiff> it = iss.iterator(); it.hasNext();) {
- String relativePath = it.next().getPath();
+ FileDiff diff = it.next();
+ String relativePath = diff.getPath();
File file = new Path(
- getRepository().getWorkTree().getAbsolutePath())
+ diff.getRepository().getWorkTree()
+ .getAbsolutePath())
.append(relativePath).toFile();
DiffViewer.openFileInEditor(file, -1);
}
@@ -352,13 +348,14 @@ public class CommitFileDiffViewer extends TableViewer {
boolean deleteSelected = false;
for (Object item : sel.toList()) {
FileDiff fileDiff = (FileDiff) item;
- if (fileDiff.isSubmodule())
+ if (fileDiff.isSubmodule()) {
submoduleSelected = true;
-
- if (fileDiff.getChange() == ChangeType.ADD)
+ }
+ if (fileDiff.getChange() == ChangeType.ADD) {
addSelected = true;
- else if (fileDiff.getChange() == ChangeType.DELETE)
+ } else if (fileDiff.getChange() == ChangeType.DELETE) {
deleteSelected = true;
+ }
}
selectAll.setEnabled(!allSelected);
@@ -371,12 +368,15 @@ public class CommitFileDiffViewer extends TableViewer {
openPreviousVersion.setEnabled(oneOrMoreSelected && !addSelected);
compare.setEnabled(sel.size() == 1);
blame.setEnabled(oneOrMoreSelected);
- if (sel.size() == 1 && !db.isBare()) {
+ if (sel.size() == 1) {
FileDiff diff = (FileDiff) sel.getFirstElement();
- String path = new Path(
- getRepository().getWorkTree().getAbsolutePath())
- .append(diff.getPath()).toOSString();
- boolean workTreeFileExists = new File(path).exists();
+ Repository repo = diff.getRepository();
+ boolean workTreeFileExists = false;
+ if (!repo.isBare()) {
+ String path = new Path(repo.getWorkTree().getAbsolutePath())
+ .append(diff.getPath()).toOSString();
+ workTreeFileExists = new File(path).exists();
+ }
compareWorkingTreeVersion.setEnabled(workTreeFileExists);
openWorkingTreeVersion.setEnabled(workTreeFileExists);
} else {
@@ -393,27 +393,25 @@ public class CommitFileDiffViewer extends TableViewer {
}
}
- @Override
- protected void inputChanged(final Object input, final Object oldInput) {
- if (oldInput == null && input == null)
- return;
- super.inputChanged(input, oldInput);
- revealFirstInterestingElement();
- }
-
/**
* @return the show in context or null
* @see IShowInSource#getShowInContext()
*/
public ShowInContext getShowInContext() {
- if (db.isBare())
- return null;
- IPath workTreePath = new Path(db.getWorkTree().getAbsolutePath());
IStructuredSelection selection = (IStructuredSelection) getSelection();
List<Object> elements = new ArrayList<>();
List<File> files = new ArrayList<>();
+ Repository repo = null;
+ IPath workTreePath = null;
for (Object selectedElement : selection.toList()) {
FileDiff fileDiff = (FileDiff) selectedElement;
+ if (repo == null || workTreePath == null) {
+ repo = fileDiff.getRepository();
+ if (repo == null || repo.isBare()) {
+ return null;
+ }
+ workTreePath = new Path(repo.getWorkTree().getAbsolutePath());
+ }
IPath path = workTreePath.append(fileDiff.getPath());
IFile file = ResourceUtil.getFileForLocation(path, false);
if (file != null)
@@ -424,7 +422,7 @@ public class CommitFileDiffViewer extends TableViewer {
}
HistoryPageInput historyPageInput = null;
if (!files.isEmpty()) {
- historyPageInput = new HistoryPageInput(db,
+ historyPageInput = new HistoryPageInput(repo,
files.toArray(new File[files.size()]));
}
return new ShowInContext(historyPageInput,
@@ -432,11 +430,11 @@ public class CommitFileDiffViewer extends TableViewer {
}
private void openThisVersionInEditor(FileDiff d) {
- DiffViewer.openInEditor(getRepository(), d, DiffEntry.Side.NEW, -1);
+ DiffViewer.openInEditor(d.getRepository(), d, DiffEntry.Side.NEW, -1);
}
private void openPreviousVersionInEditor(FileDiff d) {
- DiffViewer.openInEditor(getRepository(), d, DiffEntry.Side.OLD, -1);
+ DiffViewer.openInEditor(d.getRepository(), d, DiffEntry.Side.OLD, -1);
}
private void showAnnotations(FileDiff d) {
@@ -448,7 +446,7 @@ public class CommitFileDiffViewer extends TableViewer {
? d.getCommit().getParent(0) : d.getCommit();
String path = d.getPath();
IFileRevision rev = CompareUtils.getFileRevision(path, commit,
- getRepository(),
+ d.getRepository(),
d.getChange().equals(ChangeType.DELETE) ? d.getBlobs()[0]
: d.getBlobs()[d.getBlobs().length - 1]);
if (rev instanceof CommitFileRevision) {
@@ -470,7 +468,7 @@ public class CommitFileDiffViewer extends TableViewer {
void showTwoWayFileDiff(final FileDiff d) {
if (d.getBlobs().length <= 2) {
- DiffViewer.showTwoWayFileDiff(getRepository(), d);
+ DiffViewer.showTwoWayFileDiff(d.getRepository(), d);
} else {
MessageDialog.openInformation(
PlatformUI.getWorkbench().getActiveWorkbenchWindow()
@@ -481,60 +479,34 @@ public class CommitFileDiffViewer extends TableViewer {
}
void showWorkingDirectoryFileDiff(final FileDiff d) {
- final String p = d.getPath();
- final RevCommit commit = d.getCommit();
+ String p = d.getPath();
+ RevCommit commit = d.getCommit();
+ Repository repo = d.getRepository();
- if (commit == null) {
+ if (commit == null || repo == null) {
Activator.showError(UIText.GitHistoryPage_openFailed, null);
return;
}
IWorkbenchPage activePage = site.getWorkbenchWindow().getActivePage();
- IFile file = ResourceUtil.getFileForLocation(getRepository(), p, false);
+ IFile file = ResourceUtil.getFileForLocation(repo, p, false);
try {
if (file != null) {
final IResource[] resources = new IResource[] { file, };
- CompareUtils.compare(resources, getRepository(), Constants.HEAD,
+ CompareUtils.compare(resources, repo, Constants.HEAD,
commit.getName(), true, activePage);
} else {
- IPath path = new Path(
- getRepository().getWorkTree().getAbsolutePath())
- .append(p);
+ IPath path = new Path(repo.getWorkTree().getAbsolutePath())
+ .append(p);
File ioFile = path.toFile();
- if (ioFile.exists())
- CompareUtils.compare(path, getRepository(), Constants.HEAD,
+ if (ioFile.exists()) {
+ CompareUtils.compare(path, repo, Constants.HEAD,
commit.getName(), true, activePage);
+ }
}
} catch (IOException e) {
- Activator.logError(UIText.GitHistoryPage_openFailed, e);
- Activator.showError(UIText.GitHistoryPage_openFailed, null);
- }
- }
-
- TreeWalk getTreeWalk() {
- if (walker == null)
- throw new IllegalStateException("TreeWalk has not been set"); //$NON-NLS-1$
- return walker;
- }
-
- @NonNull
- Repository getRepository() {
- Repository repo = db;
- if (repo == null) {
- throw new IllegalStateException("Repository has not been set"); //$NON-NLS-1$
+ Activator.handleError(UIText.GitHistoryPage_openFailed, e, true);
}
- return repo;
- }
-
- /**
- * Set repository and tree walk
- *
- * @param repository
- * @param walk
- */
- public void setTreeWalk(Repository repository, TreeWalk walk) {
- db = repository;
- walker = walk;
}
private void doSelectAll() {
@@ -569,53 +541,11 @@ public class CommitFileDiffViewer extends TableViewer {
}
/**
- * @see FileDiffContentProvider#setInterestingPaths(Set)
+ * @see FileDiffContentProvider#setInterestingPaths(Collection)
* @param interestingPaths
*/
- void setInterestingPaths(Set<String> interestingPaths) {
+ void setInterestingPaths(Collection<String> interestingPaths) {
((FileDiffContentProvider) getContentProvider())
.setInterestingPaths(interestingPaths);
}
-
- void selectFirstInterestingElement() {
- IStructuredContentProvider contentProvider = ((IStructuredContentProvider) getContentProvider());
- Object[] elements = contentProvider.getElements(getInput());
- for (final Object element : elements) {
- if (element instanceof FileDiff) {
- FileDiff fileDiff = (FileDiff) element;
- boolean marked = fileDiff.isMarked(
- FileDiffContentProvider.INTERESTING_MARK_TREE_FILTER_INDEX);
- if (marked) {
- setSelection(new StructuredSelection(fileDiff));
- return;
- }
- }
- }
- }
-
- private void revealFirstInterestingElement() {
- IStructuredContentProvider contentProvider = ((IStructuredContentProvider) getContentProvider());
- Object[] elements = contentProvider.getElements(getInput());
- if (elements.length <= 1)
- return;
-
- for (final Object element : elements) {
- if (element instanceof FileDiff) {
- FileDiff fileDiff = (FileDiff) element;
- boolean marked = fileDiff.isMarked(
- FileDiffContentProvider.INTERESTING_MARK_TREE_FILTER_INDEX);
- if (marked) {
- // Does not yet work reliably, see comment on bug 393610.
- getTable().getDisplay().asyncExec(new Runnable() {
- @Override
- public void run() {
- reveal(element);
- }
- });
- // Only reveal first
- return;
- }
- }
- }
- }
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitMessageViewer.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitMessageViewer.java
index 1c3ebb41c..cf528b3b1 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitMessageViewer.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/CommitMessageViewer.java
@@ -6,7 +6,7 @@
* Copyright (C) 2011, Stefan Lay <stefan.lay@sap.com>
* Copyright (C) 2014, Marc-Andre Laperle <marc-andre.laperle@ericsson.com>
* Copyright (C) 2015, IBM Corporation (Dani Megert <daniel_megert@ch.ibm.com>)
- * Copyright (C) 2015, 2016 Thomas Wolf <thomas.wolf@paranor.ch>
+ * Copyright (C) 2015, 2018 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 2.0
@@ -63,7 +63,6 @@ import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revplot.PlotCommit;
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.StyledText;
import org.eclipse.swt.graphics.Font;
@@ -90,11 +89,8 @@ class CommitMessageViewer extends HyperlinkSourceViewer {
// Detects theme font changes
private final IPropertyChangeListener themeListener;
- // the current repository
- private Repository db;
-
// the "input" (set by setInput())
- private PlotCommit<?> commit;
+ private SWTCommit commit;
// formatting option to fill the lines
private boolean fill;
@@ -286,12 +282,12 @@ class CommitMessageViewer extends HyperlinkSourceViewer {
// so we only rebuild this when the commit did in fact change
if (input == commit)
return;
- commit = (PlotCommit<?>) input;
+ commit = (SWTCommit) input;
if (refsChangedListener != null) {
refsChangedListener.remove();
refsChangedListener = null;
}
-
+ Repository db = commit == null ? null : commit.getRepository();
if (db != null) {
allRefs = getBranches(db);
refsChangedListener = db.getListenerList().addRefsChangedListener(
@@ -311,10 +307,6 @@ class CommitMessageViewer extends HyperlinkSourceViewer {
return commit;
}
- void setRepository(final Repository repository) {
- this.db = repository;
- }
-
private static List<Ref> getBranches(Repository repo) {
List<Ref> ref = new ArrayList<>();
try {
@@ -327,28 +319,24 @@ class CommitMessageViewer extends HyperlinkSourceViewer {
return ref;
}
- private Repository getRepository() {
- if (db == null)
- throw new IllegalStateException("Repository has not been set"); //$NON-NLS-1$
- return db;
- }
-
private void format() {
- if (db == null || commit == null) {
+ if (formatJob != null && formatJob.getState() != Job.NONE) {
+ formatJob.cancel();
+ }
+ if (commit == null) {
setDocument(new Document("")); //$NON-NLS-1$
- return;
+ } else {
+ scheduleFormatJob();
}
- if (formatJob != null && formatJob.getState() != Job.NONE)
- formatJob.cancel();
- scheduleFormatJob();
}
private void scheduleFormatJob() {
IWorkbenchSiteProgressService siteService = AdapterUtils.adapt(partSite, IWorkbenchSiteProgressService.class);
- if (siteService == null)
+ if (siteService == null) {
return;
+ }
FormatJob.FormatRequest formatRequest = new FormatJob.FormatRequest(
- getRepository(), commit, fill, allRefs);
+ commit.getRepository(), commit, fill, allRefs);
formatJob = new FormatJob(formatRequest);
addDoneListenerToFormatJob();
siteService.schedule(formatJob, 0 /* now */, true /*
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiff.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiff.java
index 4a1830d48..fd8e848c3 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiff.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiff.java
@@ -21,12 +21,16 @@ import java.util.LinkedList;
import java.util.List;
import org.eclipse.core.resources.IResource;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.SubMonitor;
+import org.eclipse.egit.core.EclipseGitProgressTransformer;
import org.eclipse.egit.core.internal.util.ResourceUtil;
import org.eclipse.egit.ui.UIUtils;
import org.eclipse.egit.ui.internal.DecorationOverlayDescriptor;
import org.eclipse.egit.ui.internal.UIIcons;
import org.eclipse.jface.resource.ImageDescriptor;
import org.eclipse.jface.viewers.IDecoration;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.diff.DiffFormatter;
@@ -82,9 +86,9 @@ public class FileDiff extends WorkbenchAdapter {
private final RevCommit commit;
- private DiffEntry diffEntry;
+ private final DiffEntry diffEntry;
- private Repository repository;
+ private final Repository repository;
static ObjectId[] trees(final RevCommit commit, final RevCommit[] parents) {
final ObjectId[] r = new ObjectId[parents.length + 1];
@@ -100,6 +104,8 @@ public class FileDiff extends WorkbenchAdapter {
* @param repository
* @param walk
* @param commit
+ * @param monitor
+ * for progress reporting an cancellation; may be {@code null}
* @param markTreeFilters
* optional filters for marking entries, see
* {@link #isMarked(int)}
@@ -109,11 +115,11 @@ public class FileDiff extends WorkbenchAdapter {
* @throws CorruptObjectException
* @throws IOException
*/
- public static FileDiff[] compute(final Repository repository,
- final TreeWalk walk, final RevCommit commit,
- final TreeFilter... markTreeFilters) throws MissingObjectException,
+ public static FileDiff[] compute(Repository repository, TreeWalk walk,
+ RevCommit commit, @Nullable IProgressMonitor monitor,
+ TreeFilter... markTreeFilters) throws MissingObjectException,
IncorrectObjectTypeException, CorruptObjectException, IOException {
- return compute(repository, walk, commit, commit.getParents(),
+ return compute(repository, walk, commit, commit.getParents(), monitor,
markTreeFilters);
}
@@ -124,6 +130,8 @@ public class FileDiff extends WorkbenchAdapter {
* @param walk
* @param commit
* @param parents
+ * @param monitor
+ * for progress reporting an cancellation; may be {@code null}
* @param markTreeFilters
* optional filters for marking entries, see
* {@link #isMarked(int)}
@@ -133,11 +141,12 @@ public class FileDiff extends WorkbenchAdapter {
* @throws CorruptObjectException
* @throws IOException
*/
- public static FileDiff[] compute(final Repository repository,
- final TreeWalk walk, final RevCommit commit,
- final RevCommit[] parents,
- final TreeFilter... markTreeFilters) throws MissingObjectException,
- IncorrectObjectTypeException, CorruptObjectException, IOException {
+ public static FileDiff[] compute(Repository repository, TreeWalk walk,
+ RevCommit commit, RevCommit[] parents,
+ @Nullable IProgressMonitor monitor, TreeFilter... markTreeFilters)
+ throws MissingObjectException, IncorrectObjectTypeException,
+ CorruptObjectException, IOException {
+
final ArrayList<FileDiff> r = new ArrayList<>();
if (parents.length > 0) {
@@ -149,21 +158,36 @@ public class FileDiff extends WorkbenchAdapter {
}
if (walk.getTreeCount() <= 2) {
- List<DiffEntry> entries = DiffEntry.scan(walk, false, markTreeFilters);
+ // TODO: make JGit DiffEntry.scan and RenameDetector.compute
+ // cancelable
+ SubMonitor progress = SubMonitor.convert(monitor, 3);
+ List<DiffEntry> entries = DiffEntry.scan(walk, false,
+ markTreeFilters);
+ if (progress.isCanceled()) {
+ return new FileDiff[0];
+ }
+ progress.worked(1);
List<DiffEntry> xentries = new LinkedList<>(entries);
RenameDetector detector = new RenameDetector(repository);
detector.addAll(entries);
+ EclipseGitProgressTransformer jgitProgress = new EclipseGitProgressTransformer(
+ progress.newChild(1));
List<DiffEntry> renames = detector.compute(walk.getObjectReader(),
- org.eclipse.jgit.lib.NullProgressMonitor.INSTANCE);
- for (DiffEntry m : renames) {
- final FileDiff d = new FileDiff(repository, commit, m);
- r.add(d);
- for (Iterator<DiffEntry> i = xentries.iterator(); i.hasNext();) {
- DiffEntry n = i.next();
- if (m.getOldPath().equals(n.getOldPath()))
- i.remove();
- else if (m.getNewPath().equals(n.getNewPath()))
- i.remove();
+ jgitProgress);
+ if (!progress.isCanceled()) {
+ progress.setWorkRemaining(renames.size());
+ for (DiffEntry m : renames) {
+ final FileDiff d = new FileDiff(repository, commit, m);
+ r.add(d);
+ for (Iterator<DiffEntry> i = xentries.iterator(); i
+ .hasNext();) {
+ DiffEntry n = i.next();
+ if (m.getOldPath().equals(n.getOldPath()))
+ i.remove();
+ else if (m.getNewPath().equals(n.getNewPath()))
+ i.remove();
+ }
+ progress.worked(1);
}
}
for (DiffEntry m : xentries) {
@@ -172,6 +196,7 @@ public class FileDiff extends WorkbenchAdapter {
}
}
else { // DiffEntry does not support walks with more than two trees
+ SubMonitor progress = SubMonitor.convert(monitor, 1);
final int nTree = walk.getTreeCount();
final int myTree = nTree - 1;
@@ -179,9 +204,13 @@ public class FileDiff extends WorkbenchAdapter {
markTreeFilters);
while (walk.next()) {
- if (matchAnyParent(walk, myTree))
+ if (progress.isCanceled()) {
+ break;
+ }
+ progress.setWorkRemaining(100).worked(1);
+ if (matchAnyParent(walk, myTree)) {
continue;
-
+ }
int treeFilterMarks = treeFilterMarker.getMarks(walk);
final FileDiffForMerges d = new FileDiffForMerges(repository,
@@ -206,7 +235,6 @@ public class FileDiff extends WorkbenchAdapter {
d.modes[i] = walk.getFileMode(i);
}
-
r.add(d);
}
@@ -332,6 +360,15 @@ public class FileDiff extends WorkbenchAdapter {
}
/**
+ * Retrieves the repository the {@link FileDiff} and its commit belong to.
+ *
+ * @return the {@link Repository}
+ */
+ public Repository getRepository() {
+ return repository;
+ }
+
+ /**
* @return the old path in case of a delete, the new path otherwise, but
* never null or <code>/dev/null</code>
* @see #getNewPath()
@@ -399,7 +436,7 @@ public class FileDiff extends WorkbenchAdapter {
/**
* Whether the mark tree filter with the specified index matched during scan
* or not, see
- * {@link #compute(Repository, TreeWalk, RevCommit, RevCommit[], TreeFilter...)}
+ * {@link #compute(Repository, TreeWalk, RevCommit, RevCommit[], IProgressMonitor, TreeFilter...)}
* .
*
* @param index
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffContentProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffContentProvider.java
index 13e34074c..1ed9ccf2b 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffContentProvider.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffContentProvider.java
@@ -1,6 +1,7 @@
/*******************************************************************************
* Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
* Copyright (C) 2012, Robin Stocker <robin@nibor.org>
+ * Copyright (C) 2018, 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 2.0
@@ -12,78 +13,232 @@
package org.eclipse.egit.ui.internal.history;
import java.io.IOException;
-import java.util.Set;
+import java.text.MessageFormat;
+import java.util.Collection;
+import java.util.Objects;
+import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.jobs.IJobChangeEvent;
+import org.eclipse.core.runtime.jobs.ISchedulingRule;
+import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.jobs.JobChangeAdapter;
import org.eclipse.egit.ui.Activator;
import org.eclipse.egit.ui.internal.UIText;
import org.eclipse.jface.viewers.IStructuredContentProvider;
+import org.eclipse.jface.viewers.StructuredSelection;
import org.eclipse.jface.viewers.Viewer;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
import org.eclipse.jgit.treewalk.filter.TreeFilter;
-import org.eclipse.osgi.util.NLS;
+import org.eclipse.ui.progress.UIJob;
/**
- * Content provider for {@link FileDiff} objects
+ * Content provider for {@link FileDiff} objects. The diffs are computed
+ * asynchronously in a background job.
*/
public class FileDiffContentProvider implements IStructuredContentProvider {
static final int INTERESTING_MARK_TREE_FILTER_INDEX = 0;
- private TreeWalk walk;
-
- private RevCommit commit;
-
private FileDiff[] diff;
private TreeFilter markTreeFilter = TreeFilter.ALL;
- private Repository repo;
+ private CommitFileDiffViewer viewer;
+
+ private boolean needsRecompute;
+
+ private FileDiffLoader loader;
+
+ private FileDiffInput currentInput;
@Override
public void inputChanged(final Viewer newViewer, final Object oldInput,
final Object newInput) {
+ cancel();
+ viewer = (CommitFileDiffViewer) newViewer;
if (newInput != null) {
- repo = ((CommitFileDiffViewer) newViewer).getRepository();
- walk = ((CommitFileDiffViewer) newViewer).getTreeWalk();
- commit = (RevCommit) newInput;
+ currentInput = (FileDiffInput) newInput;
+ setInterestingPaths(currentInput.getInterestingPaths());
} else {
- repo = null;
- walk = null;
- commit = null;
+ currentInput = null;
}
diff = null;
+ needsRecompute = true;
}
/**
* Set the paths which are interesting and should be highlighted in the view.
* @param interestingPaths
*/
- void setInterestingPaths(Set<String> interestingPaths) {
- if (interestingPaths != null)
+ void setInterestingPaths(Collection<String> interestingPaths) {
+ if (interestingPaths != null) {
this.markTreeFilter = PathFilterGroup.createFromStrings(interestingPaths);
- else
+ } else {
this.markTreeFilter = TreeFilter.ALL;
- // FileDiffs need to be updated
- this.diff = null;
+ }
+ needsRecompute = true;
}
@Override
public Object[] getElements(final Object inputElement) {
- if (diff == null && walk != null && commit != null)
- try {
- diff = FileDiff.compute(repo, walk, commit, markTreeFilter);
- } catch (IOException err) {
- Activator.handleError(NLS.bind(UIText.FileDiffContentProvider_errorGettingDifference,
- commit.getId()), err, false);
+ if (!needsRecompute) {
+ return diff != null ? diff : new Object[0];
+ }
+ needsRecompute = false;
+ FileDiffInput input = currentInput;
+ if (input == null) {
+ diff = null;
+ return new Object[0];
+ }
+ cancel();
+ FileDiffLoader job = new FileDiffLoader(input, markTreeFilter);
+ job.addJobChangeListener(new JobChangeAdapter() {
+ @Override
+ public void done(IJobChangeEvent event) {
+ if (!event.getResult().isOK()) {
+ return;
+ }
+ UIJob updater = new UpdateJob(MessageFormat.format(
+ UIText.FileDiffContentProvider_updatingFileDiffs,
+ input.getCommit().getName()), job);
+ updater.schedule();
}
- return diff != null ? diff : new Object[0];
+ });
+ job.setUser(false);
+ job.setSystem(true);
+ loader = job;
+ loader.schedule();
+ return new Object[0];
+ }
+
+ private void cancel() {
+ if (loader != null) {
+ loader.cancel();
+ loader = null;
+ }
}
@Override
public void dispose() {
- // Nothing.
+ cancel();
+ viewer = null;
+ diff = null;
+ currentInput = null;
}
+
+ private static class FileDiffLoader extends Job {
+
+ private FileDiff[] diffs;
+
+ private final FileDiffInput input;
+
+ private final TreeFilter filter;
+
+ public FileDiffLoader(FileDiffInput input, TreeFilter filter) {
+ super(MessageFormat.format(
+ UIText.FileDiffContentProvider_computingFileDiffs,
+ input.getCommit().getName()));
+ this.input = input;
+ this.filter = filter;
+ setRule(new TreeWalkSchedulingRule(input.getTreeWalk()));
+ }
+
+ @Override
+ protected IStatus run(IProgressMonitor monitor) {
+ try {
+ if (monitor.isCanceled()) {
+ return Status.CANCEL_STATUS;
+ }
+ diffs = FileDiff.compute(input.getRepository(),
+ input.getTreeWalk(),
+ input.getCommit(), monitor, filter);
+ } catch (IOException err) {
+ Activator.handleError(MessageFormat.format(
+ UIText.FileDiffContentProvider_errorGettingDifference,
+ input.getCommit().getId()), err, false);
+ }
+ if (monitor.isCanceled()) {
+ return Status.CANCEL_STATUS;
+ }
+ return Status.OK_STATUS;
+ }
+
+ public FileDiff[] getDiffs() {
+ return diffs;
+ }
+ }
+
+ private class UpdateJob extends UIJob {
+
+ FileDiffLoader loadJob;
+
+ public UpdateJob(String name, FileDiffLoader loadJob) {
+ super(name);
+ this.loadJob = loadJob;
+ }
+
+ @Override
+ public IStatus runInUIThread(IProgressMonitor monitor) {
+ if (viewer.getControl().isDisposed() || loader != loadJob) {
+ return Status.CANCEL_STATUS;
+ }
+ diff = loadJob.getDiffs();
+ viewer.refresh();
+ FileDiff interesting = getFirstInterestingElement();
+ if (interesting != null) {
+ if (currentInput.isSelectMarked()) {
+ viewer.setSelection(new StructuredSelection(interesting),
+ true);
+ } else {
+ viewer.reveal(interesting);
+ }
+ }
+ return Status.OK_STATUS;
+ }
+
+ private FileDiff getFirstInterestingElement() {
+ FileDiff[] diffs = diff;
+ if (diffs != null) {
+ for (FileDiff d : diffs) {
+ if (d.isMarked(INTERESTING_MARK_TREE_FILTER_INDEX)) {
+ return d;
+ }
+ }
+ }
+ return null;
+ }
+
+ }
+
+ /**
+ * Serializes all load jobs using the same tree walk. Tree walks are not
+ * thread safe.
+ */
+ private static class TreeWalkSchedulingRule implements ISchedulingRule {
+
+ private final TreeWalk treeWalk;
+
+ public TreeWalkSchedulingRule(TreeWalk treeWalk) {
+ this.treeWalk = treeWalk;
+ }
+
+ @Override
+ public boolean contains(ISchedulingRule rule) {
+ if (rule instanceof TreeWalkSchedulingRule) {
+ return Objects.equals(treeWalk,
+ ((TreeWalkSchedulingRule) rule).treeWalk);
+ }
+ return false;
+ }
+
+ @Override
+ public boolean isConflicting(ISchedulingRule rule) {
+ return contains(rule);
+ }
+
+ }
+
}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffInput.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffInput.java
new file mode 100644
index 000000000..35aa6680d
--- /dev/null
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/FileDiffInput.java
@@ -0,0 +1,88 @@
+/*******************************************************************************
+ * Copyright (C) 2018, 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 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.egit.ui.internal.history;
+
+import java.util.Collection;
+
+import org.eclipse.jgit.annotations.Nullable;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.treewalk.TreeWalk;
+
+/**
+ * Data object to use as input for the {@link CommitFileDiffViewer} in the
+ * history page.
+ */
+public class FileDiffInput {
+
+ private final Repository repository;
+
+ private final TreeWalk walk;
+
+ private final RevCommit commit;
+
+ private final Collection<String> interestingPaths;
+
+ private final boolean selectMarked;
+
+ /**
+ * Creates a new {@link FileDiffInput}.
+ *
+ * @param repository
+ * @param walk
+ * @param commit
+ * @param interestingPaths
+ * @param selectMarked
+ */
+ public FileDiffInput(Repository repository, TreeWalk walk, RevCommit commit,
+ Collection<String> interestingPaths, boolean selectMarked) {
+ this.commit = commit;
+ this.selectMarked = selectMarked;
+ this.repository = repository;
+ this.walk = walk;
+ this.interestingPaths = interestingPaths;
+ }
+
+ /**
+ * @return the commit
+ */
+ public RevCommit getCommit() {
+ return commit;
+ }
+
+ /**
+ * @return whether the first marked entry shall be selected
+ */
+ public boolean isSelectMarked() {
+ return selectMarked;
+ }
+
+ /**
+ * @return the {@link Repository}
+ */
+ public Repository getRepository() {
+ return repository;
+ }
+
+ /**
+ * @return the {@link TreeWalk}
+ */
+ public TreeWalk getTreeWalk() {
+ return walk;
+ }
+
+ /**
+ * @return the interesting paths.
+ */
+ public @Nullable Collection<String> getInterestingPaths() {
+ return interestingPaths;
+ }
+}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java
index 0fbcbb7e9..0b7a9e983 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java
@@ -111,8 +111,6 @@ import org.eclipse.jface.viewers.TableViewer;
import org.eclipse.jgit.annotations.NonNull;
import org.eclipse.jgit.diff.DiffConfig;
import org.eclipse.jgit.diff.DiffEntry;
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.events.ListenerHandle;
import org.eclipse.jgit.events.RefsChangedEvent;
import org.eclipse.jgit.events.RefsChangedListener;
@@ -121,7 +119,6 @@ import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revplot.PlotCommit;
import org.eclipse.jgit.revplot.PlotWalk;
import org.eclipse.jgit.revwalk.FollowFilter;
import org.eclipse.jgit.revwalk.RenameCallback;
@@ -818,6 +815,9 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
/** Tracks the file names that are to be highlighted in the diff file viewer */
private Set<String> fileViewerInterestingPaths;
+ /** Tree walker to use in the file diff viewer. */
+ private TreeWalk fileDiffWalker;
+
// react on changes to the relative date preference
private final IPropertyChangeListener listener = new IPropertyChangeListener() {
@Override
@@ -1440,7 +1440,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
if (input == null) {
return;
}
- final PlotCommit<?> c = (PlotCommit<?>) sel.getFirstElement();
+ final SWTCommit c = (SWTCommit) sel.getFirstElement();
if (c.equals(commentViewer.getInput())) {
return;
}
@@ -1450,15 +1450,17 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
final RevCommit unfilteredCommit = walk.parseCommit(c);
for (RevCommit parent : unfilteredCommit.getParents())
walk.parseBody(parent);
- fileViewer.setInput(unfilteredCommit);
+ fileViewer.setInput(new FileDiffInput(input.getRepository(),
+ fileDiffWalker, unfilteredCommit,
+ fileViewerInterestingPaths,
+ input.getSingleFile() != null));
} catch (IOException e) {
- fileViewer.setInput(c);
+ fileViewer.setInput(new FileDiffInput(input.getRepository(),
+ fileDiffWalker, c, fileViewerInterestingPaths,
+ input.getSingleFile() != null));
} finally {
walk.dispose();
}
-
- if (input.getSingleFile() != null)
- fileViewer.selectFirstInterestingElement();
}
});
commentViewer
@@ -1626,19 +1628,16 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
currentRepo = null;
name = ""; //$NON-NLS-1$
input = null;
- clearCommentViewer();
- clearFileViewer();
- setInput(null);
- }
-
- private void clearCommentViewer() {
- commentViewer.setRepository(null);
commentViewer.setInput(null);
+ fileViewer.setInput(null);
+ setInput(null);
}
- private void clearFileViewer() {
- fileViewer.setTreeWalk(null, null);
- fileViewer.setInput(null);
+ private void clearViewers() {
+ TableViewer viewer = graph.getTableView();
+ viewer.setInput(new SWTCommit[0]);
+ // Force a selection changed event
+ viewer.setSelection(viewer.getSelection());
}
@Override
@@ -1648,9 +1647,9 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
@Override
public void refresh() {
- if (repoHasBeenRemoved(currentRepo))
+ if (repoHasBeenRemoved(currentRepo)) {
clearHistoryPage();
-
+ }
this.input = null;
inputSet();
}
@@ -2201,6 +2200,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
private void updateInterestingPathsOfFileViewer() {
fileViewer.setInterestingPaths(fileViewerInterestingPaths);
+ fileViewer.refresh();
}
private void setWarningText(String warning) {
@@ -2232,13 +2232,10 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
AnyObjectId headId = resolveHead(db, true);
if (headId == null) {
- TableViewer viewer = graph.getTableView();
- viewer.setInput(new SWTCommit[0]);
currentHeadId = null;
currentFetchHeadId = null;
currentRepo = db;
- // Force a selection changed event
- viewer.setSelection(viewer.getSelection());
+ clearViewers();
return;
}
AnyObjectId fetchHeadId = resolveFetchHead(db);
@@ -2246,19 +2243,34 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
List<FilterPath> paths = buildFilterPaths(input.getItems(), input
.getFileList(), db);
- if (forceNewWalk || shouldRedraw(db, headId, fetchHeadId, paths)) {
+ boolean repoChanged = false;
+ if (!db.equals(currentRepo)) {
+ repoChanged = true;
+ currentRepo = db;
+ }
+
+ if (forceNewWalk || repoChanged
+ || shouldRedraw(headId, fetchHeadId, paths)) {
releaseGenerateHistoryJob();
+ if (repoChanged) {
+ // Clear all viewers. Otherwise it may be possible that the
+ // user invokes a context menu command and due to to the
+ // highly asynchronous loading we end up with inconsistent
+ // diff computations trying to find the diff for a commit in
+ // the wrong repository.
+ clearViewers();
+ }
+
SWTWalk walk = createNewWalk(db, headId, fetchHeadId);
- setWalkStartPoints(walk, db, headId);
- setupFileViewer(walk, db, paths);
- setupCommentViewer(db);
+ fileDiffWalker = createFileWalker(walk, db, paths);
loadInitialHistory(walk);
- } else
+ } else {
// needed for context menu and double click
graph.setHistoryPageInput(input);
+ }
} finally {
if (trace)
GitTraceLocation.getTrace().traceExit(
@@ -2267,11 +2279,10 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
}
}
- private boolean shouldRedraw(Repository db, AnyObjectId headId,
- AnyObjectId fetchHeadId, List<FilterPath> paths) {
+ private boolean shouldRedraw(AnyObjectId headId, AnyObjectId fetchHeadId,
+ List<FilterPath> paths) {
boolean pathChanged = pathChanged(pathFilters, paths);
boolean headChanged = headId == null || !headId.equals(currentHeadId);
- boolean repoChanged = false;
boolean allBranchesChanged = currentShowAllBranches != store
.getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_ALL_BRANCHES);
@@ -2293,12 +2304,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
boolean followRenamesChanged = currentFollowRenames != getFollowRenames();
currentFollowRenames = getFollowRenames();
- if (!db.equals(currentRepo)) {
- repoChanged = true;
- currentRepo = db;
- }
-
- return pathChanged || headChanged || fetchHeadChanged || repoChanged
+ return pathChanged || headChanged || fetchHeadChanged
|| allBranchesChanged || additionalRefsChange
|| showNotesChanged || followRenamesChanged;
}
@@ -2420,7 +2426,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
AnyObjectId fetchHeadId) {
currentHeadId = headId;
currentFetchHeadId = fetchHeadId;
- SWTWalk walk = new SWTWalk(db);
+ SWTWalk walk = new GitHistoryWalk(db, headId);
try {
if (store
.getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_ADDITIONAL_REFS))
@@ -2440,45 +2446,6 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
return walk;
}
- private void setWalkStartPoints(RevWalk walk, Repository db, AnyObjectId headId) {
- try {
- if (store
- .getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_ALL_BRANCHES)) {
- markStartAllRefs(walk, Constants.R_HEADS);
- markStartAllRefs(walk, Constants.R_REMOTES);
- markStartAllRefs(walk, Constants.R_TAGS);
- }
- if (store
- .getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_ADDITIONAL_REFS))
- markStartAdditionalRefs(walk);
- if (store
- .getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_NOTES))
- markStartAllRefs(walk, Constants.R_NOTES);
- else
- markUninteresting(walk, Constants.R_NOTES);
-
- walk.markStart(walk.parseCommit(headId));
- } catch (IOException e) {
- throw new IllegalStateException(NLS.bind(
- UIText.GitHistoryPage_errorSettingStartPoints, Activator
- .getDefault().getRepositoryUtil()
- .getRepositoryName(db)), e);
- }
- }
-
- private void setupCommentViewer(Repository db) {
- commentViewer.setRepository(db);
- commentViewer.refresh();
- }
-
- private TreeWalk setupFileViewer(RevWalk walk, Repository db, List<FilterPath> paths) {
- final TreeWalk fileWalker = createFileWalker(walk, db, paths);
- fileViewer.setTreeWalk(db, fileWalker);
- fileViewer.setInterestingPaths(fileViewerInterestingPaths);
- fileViewer.refresh();
- return fileWalker;
- }
-
private void formatDiffs(final List<FileDiff> diffs) {
Job.getJobManager().cancel(JobFamilies.HISTORY_DIFF);
if (diffs.isEmpty()) {
@@ -2490,7 +2457,6 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
return;
}
- final Repository repository = fileViewer.getRepository();
Job formatJob = new Job(UIText.GitHistoryPage_FormatDiffJobName) {
@Override
@@ -2513,7 +2479,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
}
progress.subTask(diff.getPath());
try {
- formatter.write(repository, diff);
+ formatter.write(diff.getRepository(), diff);
} catch (IOException ignore) {
// Ignored
}
@@ -2757,60 +2723,6 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
j.schedule();
}
- /**
- * {@link RevWalk#markStart(RevCommit)} all refs with given prefix to mark
- * start of graph traversal using currentWalker
- * @param walk the walk to be configured
- *
- * @param prefix
- * prefix of refs to be marked
- * @throws IOException
- * @throws MissingObjectException
- * @throws IncorrectObjectTypeException
- */
- private void markStartAllRefs(RevWalk walk, String prefix) throws IOException,
- MissingObjectException, IncorrectObjectTypeException {
- for (Ref ref : input.getRepository().getRefDatabase()
- .getRefsByPrefix(prefix)) {
- if (ref.isSymbolic())
- continue;
- markStartRef(walk, ref);
- }
- }
-
- private void markStartAdditionalRefs(RevWalk walk) throws IOException {
- List<Ref> additionalRefs = input.getRepository().getRefDatabase()
- .getAdditionalRefs();
- for (Ref ref : additionalRefs)
- markStartRef(walk, ref);
- }
-
- private void markStartRef(RevWalk walk, Ref ref) throws IOException,
- IncorrectObjectTypeException {
- try {
- RevObject refTarget = walk.parseAny(ref.getLeaf().getObjectId());
- RevObject peeled = walk.peel(refTarget);
- if (peeled instanceof RevCommit)
- walk.markStart((RevCommit) peeled);
- } catch (MissingObjectException e) {
- // If there is a ref which points to Nirvana then we should simply
- // ignore this ref. We should not let a corrupt ref cause that the
- // history view is not filled at all
- }
- }
-
- private void markUninteresting(RevWalk walk, String prefix) throws IOException,
- MissingObjectException, IncorrectObjectTypeException {
- for (Ref ref : input.getRepository().getRefDatabase()
- .getRefsByPrefix(prefix)) {
- if (ref.isSymbolic())
- continue;
- Object refTarget = walk
- .parseAny(ref.getLeaf().getObjectId());
- if (refTarget instanceof RevCommit)
- walk.markUninteresting((RevCommit) refTarget);
- }
- }
private void releaseGenerateHistoryJob() {
if (job != null) {
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryWalk.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryWalk.java
new file mode 100644
index 000000000..ec5fd7c7d
--- /dev/null
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryWalk.java
@@ -0,0 +1,145 @@
+/*******************************************************************************
+ * Copyright (C) 2018, 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 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.egit.ui.internal.history;
+
+import java.io.IOException;
+import java.text.MessageFormat;
+
+import org.eclipse.egit.ui.Activator;
+import org.eclipse.egit.ui.UIPreferences;
+import org.eclipse.egit.ui.internal.UIText;
+import org.eclipse.jface.preference.IPreferenceStore;
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.AnyObjectId;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
+
+/**
+ * An {@link SWTWalk} that sets its start points on the first call to next()
+ * depending on the git history preferences. The point of doing so is to be sure
+ * that potentially expensive operations are done in the GerenrateHistoryJob and
+ * not in the UI thread.
+ */
+class GitHistoryWalk extends SWTWalk {
+
+ private boolean initialized = false;
+
+ private final AnyObjectId headId;
+
+ GitHistoryWalk(Repository repository, AnyObjectId headId) {
+ super(repository);
+ this.headId = headId;
+ }
+
+ @Override
+ public RevCommit next() throws MissingObjectException,
+ IncorrectObjectTypeException, IOException {
+ if (!initialized) {
+ initialize();
+ }
+ return super.next();
+ }
+
+ @Override
+ protected void reset(int retainFlags) {
+ super.reset(retainFlags);
+ initialized = false;
+ }
+
+ /**
+ * Set the walk's start points as defined by the git history preferences.
+ *
+ * @throws IOException
+ * on errors
+ */
+ private void initialize() throws IOException {
+ IPreferenceStore store = Activator.getDefault().getPreferenceStore();
+ RefDatabase db = getRepository().getRefDatabase();
+ try {
+ if (store.getBoolean(
+ UIPreferences.RESOURCEHISTORY_SHOW_ALL_BRANCHES)) {
+ markStartAllRefs(db, Constants.R_HEADS);
+ markStartAllRefs(db, Constants.R_REMOTES);
+ markStartAllRefs(db, Constants.R_TAGS);
+ }
+ if (store.getBoolean(
+ UIPreferences.RESOURCEHISTORY_SHOW_ADDITIONAL_REFS)) {
+ markStartAdditionalRefs(db);
+ }
+ if (store.getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_NOTES)) {
+ markStartAllRefs(db, Constants.R_NOTES);
+ } else {
+ markUninteresting(db, Constants.R_NOTES);
+ }
+ markStart(parseCommit(headId));
+ } catch (IOException e) {
+ throw new IOException(MessageFormat.format(
+ UIText.GitHistoryPage_errorSettingStartPoints,
+ Activator.getDefault().getRepositoryUtil()
+ .getRepositoryName(getRepository())),
+ e);
+ }
+ initialized = true;
+ }
+
+ private void markStartAllRefs(RefDatabase db, String prefix)
+ throws IOException, IncorrectObjectTypeException {
+ for (Ref ref : db.getRefsByPrefix(prefix)) {
+ if (!ref.isSymbolic()) {
+ markStartRef(ref);
+ }
+ }
+ }
+
+ private void markStartAdditionalRefs(RefDatabase db)
+ throws IOException, IncorrectObjectTypeException {
+ for (Ref ref : db.getAdditionalRefs()) {
+ markStartRef(ref);
+ }
+ }
+
+ private void markStartRef(Ref ref)
+ throws IOException, IncorrectObjectTypeException {
+ try {
+ RevObject refTarget = parseAny(ref.getLeaf().getObjectId());
+ RevObject peeled = peel(refTarget);
+ if (peeled instanceof RevCommit) {
+ markStart((RevCommit) peeled);
+ }
+ } catch (MissingObjectException e) {
+ // If there is a ref which points to Nirvana then we should simply
+ // ignore this ref. We should not let a corrupt ref cause that the
+ // history view is not filled at all.
+ }
+ }
+
+ private void markUninteresting(RefDatabase db, String prefix)
+ throws IOException, IncorrectObjectTypeException {
+ for (Ref ref : db.getRefsByPrefix(prefix)) {
+ if (!ref.isSymbolic()) {
+ try {
+ RevObject refTarget = parseAny(ref.getLeaf().getObjectId());
+ if (refTarget instanceof RevCommit) {
+ markUninteresting((RevCommit) refTarget);
+ }
+ } catch (MissingObjectException e) {
+ // See above.
+ }
+ }
+ }
+ }
+
+}
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/SWTCommit.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/SWTCommit.java
index 2765fefd6..e890c3f96 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/SWTCommit.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/SWTCommit.java
@@ -53,7 +53,7 @@ class SWTCommit extends PlotCommit<SWTCommitList.SWTLane>
if (adapter != Repository.class) {
return null;
}
- return adapter.cast(walk.getRepository());
+ return getRepository();
}
@Override
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 f51b40273..518a47682 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
@@ -1024,7 +1024,9 @@ FetchWizard_cantSaveMessage=Couldn't save specified specifications in configurat
FetchWizard_cantSaveTitle=Configuration Storage Warning
FetchWizard_windowTitleDefault=Fetch from Another Repository
FetchWizard_windowTitleWithSource=Fetch from: {0}
+FileDiffContentProvider_computingFileDiffs=Computing file differences of commit {0}
FileDiffContentProvider_errorGettingDifference=Can''t get file difference of {0}.
+FileDiffContentProvider_updatingFileDiffs=Updating file differences of commit {0}
FileDiffLabelProvider_RenamedFromToolTip=Renamed from {0}
FileRevisionEditorInput_NameAndRevisionTitle={0} {1}
FileRevisionEditorInput_cannotWriteTempFile=Cannot write temporary file for {0}.

Back to the top