Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2019-07-06 13:18:52 -0400
committerThomas Wolf2019-07-07 09:51:12 -0400
commit10293f9f0c3739a66e772d7cd3e7560f7c4b7ccd (patch)
tree1016e62c613fff40582fbe13bda40a73f1d2fd11
parentb9df19f1d83ca2daa7272369d264f9806d8118ab (diff)
downloadegit-10293f9f0c3739a66e772d7cd3e7560f7c4b7ccd.tar.gz
egit-10293f9f0c3739a66e772d7cd3e7560f7c4b7ccd.tar.xz
egit-10293f9f0c3739a66e772d7cd3e7560f7c4b7ccd.zip
Fix showing selected refs or commits in history view
When the history view shows only the current branch ("Show all branches and tags" _off_), selecting some other branch in the git repositories view might not show the selected branch. It would be shown only if that branch happened to be merged into the currently checked out branch. The same could be observed when using "Show In History" on a commit in the results of a "Git Search": the commit would only be selected if it was contained in the branch currently checked out. Change this by always considering such a selected branch or commit as an additional start point for the RevWalk. This enables the user to make the history view show exactly two branches (plus all those merged into these two), and also correctly selects commits not on the current branch. Bug: 360868 Change-Id: Ib2e071f40c57b06a0ce0a51defac3fe30f481e03 Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
-rw-r--r--org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/history/HistoryViewTest.java111
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPage.java33
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryWalk.java19
3 files changed, 130 insertions, 33 deletions
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/history/HistoryViewTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/history/HistoryViewTest.java
index 3e95720b0..268ce7b83 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/history/HistoryViewTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/test/history/HistoryViewTest.java
@@ -14,6 +14,7 @@ package org.eclipse.egit.ui.test.history;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -25,12 +26,16 @@ import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.Adapters;
import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.egit.core.RepositoryUtil;
+import org.eclipse.egit.ui.Activator;
import org.eclipse.egit.ui.JobFamilies;
-import org.eclipse.egit.ui.common.LocalRepositoryTestCase;
import org.eclipse.egit.ui.internal.UIText;
+import org.eclipse.egit.ui.internal.repository.RepositoriesView;
import org.eclipse.egit.ui.test.ContextMenuHelper;
import org.eclipse.egit.ui.test.TestUtil;
+import org.eclipse.egit.ui.view.repositories.GitRepositoriesViewTestBase;
import org.eclipse.jface.dialogs.IDialogConstants;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.lib.Constants;
@@ -38,6 +43,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;
import org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView;
import org.eclipse.swtbot.swt.finder.SWTBot;
@@ -56,7 +62,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
@RunWith(SWTBotJunit4ClassRunner.class)
-public class HistoryViewTest extends LocalRepositoryTestCase {
+public class HistoryViewTest extends GitRepositoriesViewTestBase {
private static final String SECONDFOLDER = "secondFolder";
private static final String ADDEDFILE = "another.txt";
@@ -68,7 +74,7 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
private File repoFile;
@Before
- public void setup() throws Exception {
+ public void setupTests() throws Exception {
repoFile = createProjectAndCommitToRepository();
IProject prj = ResourcesPlugin.getWorkspace().getRoot()
.getProject(PROJ1);
@@ -81,6 +87,9 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
addAndCommit(addedFile, ADDEDMESSAGE);
// TODO count the commits
commitCount = 3;
+ RepositoryUtil repositoryUtil = Activator.getDefault()
+ .getRepositoryUtil();
+ repositoryUtil.addConfiguredRepository(repoFile);
}
@Test
@@ -228,6 +237,15 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
getHistoryViewTable(PROJ1).getTableItem(0).getText(1));
}
+ private SWTBotTable getHistoryViewTable() throws Exception {
+ SWTBot historyView = getHistoryViewBot();
+ Job.getJobManager().join(JobFamilies.GENERATE_HISTORY, null);
+ historyView.getDisplay().syncExec(() -> {
+ // Join UI update triggered by GenerateHistoryJob
+ });
+ return historyView.table();
+ }
+
/**
* @param path
* must be length 2 or three (folder or file)
@@ -252,17 +270,7 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
explorerItem.select();
ContextMenuHelper.clickContextMenuSync(projectExplorerTree, "Team",
"Show in History");
- // join GenerateHistoryJob
- Job.getJobManager().join(JobFamilies.GENERATE_HISTORY, null);
- // join UI update triggered by GenerateHistoryJob
- projectExplorerTree.widget.getDisplay().syncExec(new Runnable() {
- @Override
- public void run() {
- // empty
- }
- });
-
- return getHistoryViewBot().table();
+ return getHistoryViewTable();
}
private SWTBotTable getFileDiffTable() throws Exception {
@@ -280,6 +288,55 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
}
@Test
+ public void testSelectBranch() throws Exception {
+ toggleShowAllBranchesButton(false);
+ SWTBotTable commitTable = getHistoryViewTable(PROJ1);
+ assertEquals("Unexpected number of commits", commitCount,
+ commitTable.rowCount());
+ // Current branch is "master". Create a new commit on a new branch, then
+ // switch back to master.
+ try (Git git = new Git(lookupRepository(repoFile))) {
+ git.checkout().setCreateBranch(true).setName("otherBranch").call();
+ TestUtil.waitForDecorations();
+ touchAndSubmit("Updated");
+ ObjectId otherCommit = git.getRepository().resolve("otherBranch");
+ Ref master = git.checkout().setName(Constants.MASTER).call();
+ assertNotNull("Branch is null", master.getLeaf().getObjectId());
+ assertNotEquals("Branch not switched", otherCommit,
+ master.getLeaf().getObjectId());
+ TestUtil.waitForDecorations();
+ // History table should not show otherCommit
+ commitTable = getHistoryViewTable();
+ assertEquals("Unexpected number of commits", commitCount,
+ commitTable.rowCount());
+ // Open git repo view, select "otherBranch".
+ SWTBotView view = TestUtil.showView(RepositoriesView.VIEW_ID);
+ TestUtil.joinJobs(JobFamilies.REPO_VIEW_REFRESH);
+ TestUtil.waitForDecorations();
+ SWTBotTree tree = view.bot().tree();
+ SWTBotTreeItem localBranches = myRepoViewUtil
+ .getLocalBranchesItem(tree, repoFile);
+ TestUtil.expandAndWait(localBranches).getNode("otherBranch")
+ .select();
+ ContextMenuHelper.clickContextMenuSync(tree, "Show In", "History");
+ // History table should show both branches
+ commitTable = getHistoryViewTable();
+ assertEquals("Unexpected number of commits", commitCount + 1,
+ commitTable.rowCount());
+ Table swtTable = commitTable.widget;
+ ObjectId[] firstId = { null };
+ swtTable.getDisplay().syncExec(() -> {
+ Object obj = swtTable.getItem(0).getData();
+ RevCommit c = Adapters.adapt(obj, RevCommit.class);
+ if (c != null) {
+ firstId[0] = c.getId();
+ }
+ });
+ assertEquals("Unexpected commit in table", otherCommit, firstId[0]);
+ }
+ }
+
+ @Test
public void testAddBranch() throws Exception {
Repository repo = lookupRepository(repoFile);
assertNull(repo.resolve(Constants.R_HEADS + "NewBranch"));
@@ -341,17 +398,21 @@ public class HistoryViewTest extends LocalRepositoryTestCase {
@Test
public void testShowAllBranches() throws Exception {
- toggleShowAllBranchesButton(true);
- final SWTBotTable table = getHistoryViewTable(PROJ1);
- int commits = getHistoryViewTable(PROJ1).rowCount();
- checkoutLine(table, 1);
-
- toggleShowAllBranchesButton(false);
- assertEquals("Wrong number of commits", commits - 1,
- getHistoryViewTable(PROJ1).rowCount());
- toggleShowAllBranchesButton(true);
- assertEquals("Wrong number of commits", commits,
- getHistoryViewTable(PROJ1).rowCount());
+ try {
+ toggleShowAllBranchesButton(true);
+ final SWTBotTable table = getHistoryViewTable(PROJ1);
+ int commits = getHistoryViewTable(PROJ1).rowCount();
+ checkoutLine(table, 1);
+
+ toggleShowAllBranchesButton(false);
+ assertEquals("Wrong number of commits", commits - 1,
+ getHistoryViewTable(PROJ1).rowCount());
+ toggleShowAllBranchesButton(true);
+ assertEquals("Wrong number of commits", commits,
+ getHistoryViewTable(PROJ1).rowCount());
+ } finally {
+ toggleShowAllBranchesButton(false);
+ }
}
@Test
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 589d849b8..3a9c6ec71 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
@@ -809,6 +809,9 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
/** Repository of the last input*/
private Repository currentRepo;
+ /** ObjectId of the ref or commit of the last input, if any. */
+ private ObjectId selectedObj;
+
private boolean currentShowAllBranches;
private boolean currentShowAdditionalRefs;
@@ -1661,6 +1664,8 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
}
renameTracker.reset(null);
Job.getJobManager().cancel(JobFamilies.HISTORY_DIFF);
+ currentRepo = null;
+ selectedObj = null;
super.dispose();
}
@@ -1685,6 +1690,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
private void clearHistoryPage() {
currentRepo = null;
+ selectedObj = null;
name = ""; //$NON-NLS-1$
input = null;
commentViewer.setInput(null);
@@ -1957,7 +1963,13 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
setErrorMessage(null);
try {
- initAndStartRevWalk(false);
+ ObjectId id = null;
+ if (ref != null) {
+ id = ref.getLeaf().getObjectId();
+ } else if (selection != null) {
+ id = selection.getId();
+ }
+ initAndStartRevWalk(false, id);
} catch (IllegalStateException e) {
Activator.handleError(e.getMessage(), e, true);
return false;
@@ -2291,7 +2303,13 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
warningComposite.getParent().layout(true);
}
- void initAndStartRevWalk(boolean forceNewWalk) throws IllegalStateException {
+ private void initAndStartRevWalk(boolean forceNewWalk) {
+ initAndStartRevWalk(forceNewWalk, selectedObj);
+ }
+
+ private void initAndStartRevWalk(boolean forceNewWalk,
+ ObjectId newSelectedObj)
+ throws IllegalStateException {
try {
if (trace)
GitTraceLocation.getTrace().traceEntry(
@@ -2309,6 +2327,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
if (headId == null) {
currentHeadId = null;
currentFetchHeadId = null;
+ selectedObj = null;
currentRepo = db;
clearViewers();
return;
@@ -2324,7 +2343,13 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
currentRepo = db;
}
- if (forceNewWalk || repoChanged
+ boolean objChanged = false;
+ if (newSelectedObj != null && newSelectedObj != selectedObj) {
+ objChanged = !newSelectedObj.equals(selectedObj);
+ }
+ selectedObj = newSelectedObj;
+
+ if (forceNewWalk || repoChanged || objChanged
|| shouldRedraw(headId, fetchHeadId, paths)) {
releaseGenerateHistoryJob();
@@ -2501,7 +2526,7 @@ public class GitHistoryPage extends HistoryPage implements RefsChangedListener,
AnyObjectId fetchHeadId) {
currentHeadId = headId;
currentFetchHeadId = fetchHeadId;
- SWTWalk walk = new GitHistoryWalk(db, headId);
+ SWTWalk walk = new GitHistoryWalk(db, headId, selectedObj);
try {
if (store
.getBoolean(UIPreferences.RESOURCEHISTORY_SHOW_ADDITIONAL_REFS))
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
index ec5fd7c7d..57ccd8e79 100644
--- 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
@@ -21,6 +21,7 @@ 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.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
@@ -39,9 +40,12 @@ class GitHistoryWalk extends SWTWalk {
private final AnyObjectId headId;
- GitHistoryWalk(Repository repository, AnyObjectId headId) {
+ private final ObjectId toShow;
+
+ GitHistoryWalk(Repository repository, AnyObjectId headId, ObjectId toShow) {
super(repository);
this.headId = headId;
+ this.toShow = toShow;
}
@Override
@@ -85,6 +89,9 @@ class GitHistoryWalk extends SWTWalk {
markUninteresting(db, Constants.R_NOTES);
}
markStart(parseCommit(headId));
+ if (toShow != null) {
+ markStart(toShow);
+ }
} catch (IOException e) {
throw new IOException(MessageFormat.format(
UIText.GitHistoryPage_errorSettingStartPoints,
@@ -111,11 +118,10 @@ class GitHistoryWalk extends SWTWalk {
}
}
- private void markStartRef(Ref ref)
+ private void markStart(ObjectId id)
throws IOException, IncorrectObjectTypeException {
try {
- RevObject refTarget = parseAny(ref.getLeaf().getObjectId());
- RevObject peeled = peel(refTarget);
+ RevObject peeled = peel(parseAny(id));
if (peeled instanceof RevCommit) {
markStart((RevCommit) peeled);
}
@@ -126,6 +132,11 @@ class GitHistoryWalk extends SWTWalk {
}
}
+ private void markStartRef(Ref ref)
+ throws IOException, IncorrectObjectTypeException {
+ markStart(ref.getLeaf().getObjectId());
+ }
+
private void markUninteresting(RefDatabase db, String prefix)
throws IOException, IncorrectObjectTypeException {
for (Ref ref : db.getRefsByPrefix(prefix)) {

Back to the top