Repositories view: check for submodules when reacting on selection

When the repositories was set to follow the selection and a file in
a submodule folder was selected, the submodule repository was added
as a top-level repository in the view. Fix this by also considering
repositories under submodule nodes when determining which node to
show.

Add a new test, and fix SubmoduleFolderTest to properly commit the
addition of the submodule in setUp().

Bug: 520735
Change-Id: I54c1d2c5d2b8fac95dc7a4fed658e59285639f4c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

2 files changed, 98 insertions, 16 deletions

diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
index 97a42f4b30..14b4c33091 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
@@ -13,6 +13,7 @@ package org.eclipse.egit.ui.internal.submodules;
 import static org.eclipse.egit.ui.JobFamilies.ADD_TO_INDEX;
 import static org.eclipse.egit.ui.JobFamilies.GENERATE_HISTORY;
 import static org.eclipse.egit.ui.JobFamilies.REMOVE_FROM_INDEX;
+import static org.eclipse.egit.ui.JobFamilies.REPO_VIEW_REFRESH;
 import static org.eclipse.swtbot.eclipse.finder.waits.Conditions.waitForEditor;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -42,6 +43,7 @@ import org.eclipse.egit.ui.common.LocalRepositoryTestCase;
 import org.eclipse.egit.ui.internal.UIText;
 import org.eclipse.egit.ui.internal.clone.ProjectRecord;
 import org.eclipse.egit.ui.internal.clone.ProjectUtils;
+import org.eclipse.egit.ui.internal.repository.RepositoriesView;
 import org.eclipse.egit.ui.internal.resources.IResourceState;
 import org.eclipse.egit.ui.internal.resources.ResourceStateFactory;
 import org.eclipse.egit.ui.test.ContextMenuHelper;
@@ -51,6 +53,8 @@ import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.submodule.SubmoduleWalk;
+import org.eclipse.swt.widgets.Tree;
+import org.eclipse.swt.widgets.TreeItem;
 import org.eclipse.swtbot.eclipse.finder.widgets.SWTBotView;
 import org.eclipse.swtbot.swt.finder.junit.SWTBotJunit4ClassRunner;
 import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
@@ -74,6 +78,8 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
 
 	private static final String CHILDPROJECT = "ChildProject";
 
+	private static final TestUtil UTIL = new TestUtil();
+
 	private Repository parentRepository;
 
 	private Repository childRepository;
@@ -118,7 +124,7 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
 						.toString())
 				.call();
 		TestRepository parentRepo = new TestRepository(parentRepository);
-		parentRepo.trackAllFiles(parentProject);
+		Git.wrap(parentRepository).add().addFilepattern(".").call();
 		parentRepo.commit("Commit submodule");
 		assertTrue(SubmoduleWalk.containsGitModulesFile(parentRepository));
 		parentProject.refreshLocal(IResource.DEPTH_INFINITE, null);
@@ -252,6 +258,53 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
 		shell.close();
 	}
 
+	@Test
+	public void testRepoViewFollowSelection() throws Exception {
+		SWTBotView view = TestUtil.showView(RepositoriesView.VIEW_ID);
+		TestUtil.joinJobs(REPO_VIEW_REFRESH);
+		view.toolbarButton(
+				UTIL.getPluginLocalizedValue("LinkWithSelectionCommand"))
+				.click();
+		try {
+			SWTBotTree projectExplorerTree = TestUtil.getExplorerTree();
+			SWTBotTreeItem node = TestUtil.navigateTo(projectExplorerTree,
+					childFolder.getFullPath().segments());
+			node.select();
+			TestUtil.waitForDecorations();
+			TestUtil.joinJobs(REPO_VIEW_REFRESH);
+			SWTBotTree tree = view.bot().tree();
+			int[] numberOfSelected = { 0 };
+			boolean[] parentFound = { false };
+			view.getWidget().getDisplay().syncExec(() -> {
+				Tree t = tree.widget;
+				TreeItem[] selected = t.getSelection();
+				numberOfSelected[0] = selected.length;
+				if (selected.length == 1) {
+					TreeItem root = null;
+					TreeItem parent = selected[0].getParentItem();
+					String parentRepoName = parentRepositoryGitDir
+							.getParentFile().getName();
+					while (parent != null) {
+						root = parent;
+						parent = parent.getParentItem();
+					}
+					if (root != null
+							&& root.getText().startsWith(parentRepoName)) {
+						parentFound[0] = true;
+					}
+				}
+			});
+			assertEquals("One node selected", 1, numberOfSelected[0]);
+			assertTrue("Selected node not under parent repository",
+					parentFound[0]);
+		} finally {
+			// Reset "follow selection"
+			view.toolbarButton(
+					UTIL.getPluginLocalizedValue("LinkWithSelectionCommand"))
+					.click();
+		}
+	}
+
 	/**
 	 * Tests that a CompareWithHeadAction on a file from a submodule folder does
 	 * open the right compare editor, comparing against the version from the
@@ -362,9 +415,9 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
 		// Set link with selection
 		((org.eclipse.team.internal.ui.history.GenericHistoryView) viewPart)
 				.setLinkingEnabled(true);
-		// Select PROJ1 (has 3 commits)
+		// Select PROJ1 (has 4 commits)
 		TestUtil.navigateTo(TestUtil.getExplorerTree(), PROJ1).select();
-		assertRowCountInHistory(PROJ1, 3);
+		assertRowCountInHistory(PROJ1, 4);
 		// Select the child folder (from the submodule; has 2 commits)
 		TestUtil.navigateTo(TestUtil.getExplorerTree(),
 				childFolder.getFullPath().segments()).select();
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
index 908ea75647..e1da30586c 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoriesView.java
@@ -631,13 +631,13 @@ public class RepositoriesView extends CommonNavigator implements IShowInSource,
 	private boolean checkNotConfiguredRepositories(
 			Map<Repository, Collection<String>> pathsByRepo) {
 		boolean added = false;
-		for (Map.Entry<Repository, Collection<String>> entry : pathsByRepo.entrySet()) {
-			Repository repository = entry.getKey();
+		for (Repository repository : pathsByRepo.keySet()) {
 			try {
-				boolean newOne = repositoryUtil
-						.addConfiguredRepository(repository.getDirectory());
-				if (newOne) {
-					added = true;
+				RepositoryTreeNode node = getRepositoryChildNode(repository,
+						RepositoryTreeNodeType.WORKINGDIR);
+				if (node == null) {
+					added |= repositoryUtil
+							.addConfiguredRepository(repository.getDirectory());
 				}
 			} catch (IllegalArgumentException iae) {
 				Activator.handleError(iae.getMessage(), iae, false);
@@ -1005,16 +1005,43 @@ public class RepositoriesView extends CommonNavigator implements IShowInSource,
 			RepositoryTreeNodeType type) {
 		ITreeContentProvider cp = (ITreeContentProvider) getCommonViewer()
 				.getContentProvider();
-		for (Object repo : cp.getElements(getCommonViewer().getInput())) {
+		RepositoryTreeNode repoNode = findRepositoryNode(cp,
+				cp.getElements(getCommonViewer().getInput()), repository);
+		return repoNode == null ? null : findChild(cp, repoNode, type);
+	}
+
+	private RepositoryTreeNode findChild(ITreeContentProvider cp,
+			RepositoryTreeNode root, RepositoryTreeNodeType type) {
+		for (Object child : cp.getChildren(root)) {
+			RepositoryTreeNode childNode = (RepositoryTreeNode) child;
+			if (childNode.getType() == type) {
+				return childNode;
+			}
+		}
+		return null;
+	}
+
+	private RepositoryTreeNode findRepositoryNode(
+			ITreeContentProvider cp, Object[] roots,
+			Repository repository) {
+		for (Object repo : roots) {
 			RepositoryTreeNode node = (RepositoryTreeNode) repo;
 			// TODO equals implementation of Repository?
 			if (repository.getDirectory().equals(
 					((Repository) node.getObject()).getDirectory())) {
-				for (Object child : cp.getChildren(node)) {
-					RepositoryTreeNode childNode = (RepositoryTreeNode) child;
-					if (childNode.getType() == type) {
-						return childNode;
-					}
+				return node;
+			}
+		}
+		// Might be a submodule
+		for (Object repo : roots) {
+			RepositoryTreeNode node = (RepositoryTreeNode) repo;
+			RepositoryTreeNode submodules = findChild(cp, node,
+						RepositoryTreeNodeType.SUBMODULES);
+			if (submodules != null) {
+				RepositoryTreeNode submoduleNode = findRepositoryNode(cp,
+						cp.getChildren(submodules), repository);
+				if (submoduleNode != null) {
+					return submoduleNode;
 				}
 			}
 		}
@@ -1025,7 +1052,9 @@ public class RepositoriesView extends CommonNavigator implements IShowInSource,
 			String repoRelativePath) {
 		RepositoryTreeNode currentNode = getRepositoryChildNode(repository,
 				RepositoryTreeNodeType.WORKINGDIR);
-
+		if (currentNode == null) {
+			return null;
+		}
 		ITreeContentProvider cp = (ITreeContentProvider) getCommonViewer()
 				.getContentProvider();
 		IPath relPath = new Path(repoRelativePath);