[EGIT PATCH] Select all changes in repository for commit by default

This is an old patch series, previously send under the subject [EGIT RFC] Commit behaviour
now squash and it seems to work with Eclipse 3.4 too.

This tentative feature allows me to hit the commit button
when any resource is selected and figure out which resources have been
modified. This makes it much easier to commit. Only the toolbar commit
is affected for now.

-- robin

We want to go from selected resources to repositories and back
to any resource in those repositories

Signed-off-by: Robin Rosenberg <redacted>
---
 .../egit/ui/internal/actions/RepositoryAction.java |   50 ++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/RepositoryAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/RepositoryAction.java
index c4e3256..8c250ca 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/RepositoryAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/RepositoryAction.java

@@ -13,6 +13,8 @@
 import java.util.Set;
 
 import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.jface.action.IAction;
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.team.internal.ui.actions.TeamAction;

@@ -36,6 +38,54 @@ public void execute(IAction action) {
 	}
 
 	/**
+	 * @return the projects hosting the selected resources
+	 */
+	protected IProject[] getProjectsForSelectedResources() {
+		Set<IProject> ret = new HashSet<IProject>();
+		for (IResource resource : (IResource[])getSelectedAdaptables(getSelection(), IResource.class))
+			ret.add(resource.getProject());
+		return ret.toArray(new IProject[ret.size()]);
+	}
+
+	/**
+	 * @param projects
+	 *            a list of projects
+	 * @return the repositories that projects map to iff all projects are mapped
+	 */ 
+	protected Repository[] getRepositoriesFor(final IProject[] projects) {
+		Set<Repository> ret = new HashSet<Repository>();
+		for (IProject project : projects) {
+			RepositoryMapping repositoryMapping = RepositoryMapping.getMapping(project);
+			if (repositoryMapping == null)
+				return new Repository[0];
+			ret.add(repositoryMapping.getRepository());
+		}
+		return ret.toArray(new Repository[ret.size()]);
+	}
+	
+	/**
+	 * List the projects with selected resources, if all projects are connected
+	 * to a Git repository.
+	 * 
+	 * @return the tracked projects affected by the current resource selection
+	 */
+	public IProject[] getProjectsInRepositoryOfSelectedResources() {
+		Set<IProject> ret = new HashSet<IProject>();
+		Repository[] repositories = getRepositoriesFor(getProjectsForSelectedResources());
+		final IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
+		for (IProject project : projects) {
+			RepositoryMapping mapping = RepositoryMapping.getMapping(project);
+			for (Repository repository : repositories) {
+				if (mapping != null && mapping.getRepository() == repository) {
+					ret.add(project);
+					break;
+				}
+			}
+		}
+		return ret.toArray(new IProject[ret.size()]);
+	}
+
+	/**
 	 * Figure out which repository to use. All selected
 	 * resources must map to the same Git repository.
 	 *

-- 
1.6.0.rc2.35.g04c6e9

By default the commit dialog will be populated with all changed resources
in the projects that contain the selected resources, provided the projects
are associated with a Git respository.

Signed-off-by: Robin Rosenberg <redacted>
---
 .../egit/ui/internal/actions/CommitAction.java     |   33 +++++++++----------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
index 8db701c..d703048 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java

@@ -68,7 +68,7 @@ public void run(IAction act) {
 			return;
 		}
 
-		Repository[] repos = getRepositories();
+		Repository[] repos = getRepositoriesFor(getProjectsForSelectedResources());
 		amendAllowed = repos.length == 1;
 		for (Repository repo : repos) {
 			if (!repo.getRepositoryState().canCommit()) {

@@ -124,7 +124,7 @@ private void resetState() {
 	}
 
 	private void loadPreviousCommit() {
-		IProject project = getSelectedProjects()[0];
+		IProject project = getProjectsForSelectedResources()[0];
 
 		Repository repo = RepositoryMapping.getMapping(project).getRepository();
 		try {

@@ -313,21 +313,20 @@ private void writeTreeWithSubTrees(Tree tree) throws TeamException {
 	}
 
 	private void buildIndexHeadDiffList() throws IOException {
-		for (IProject project : getSelectedProjects()) {
+		for (IProject project : getProjectsInRepositoryOfSelectedResources()) {
 			RepositoryMapping repositoryMapping = RepositoryMapping.getMapping(project);
-			if (repositoryMapping != null) {
-				Repository repository = repositoryMapping.getRepository();
-				Tree head = repository.mapTree("HEAD");
-				GitIndex index = repository.getIndex();
-				IndexDiff indexDiff = new IndexDiff(head, index);
-				indexDiff.diff();
-
-				includeList(project, indexDiff.getAdded(), indexChanges);
-				includeList(project, indexDiff.getChanged(), indexChanges);
-				includeList(project, indexDiff.getRemoved(), indexChanges);
-				includeList(project, indexDiff.getMissing(), notIndexed);
-				includeList(project, indexDiff.getModified(), notIndexed);
-			}
+			assert repositoryMapping != null;
+			Repository repository = repositoryMapping.getRepository();
+			Tree head = repository.mapTree("HEAD");
+			GitIndex index = repository.getIndex();
+			IndexDiff indexDiff = new IndexDiff(head, index);
+			indexDiff.diff();
+
+			includeList(project, indexDiff.getAdded(), indexChanges);
+			includeList(project, indexDiff.getChanged(), indexChanges);
+			includeList(project, indexDiff.getRemoved(), indexChanges);
+			includeList(project, indexDiff.getMissing(), notIndexed);
+			includeList(project, indexDiff.getModified(), notIndexed);
 		}
 	}
 

@@ -395,7 +394,7 @@ private boolean isChanged(RepositoryMapping map, IFile resource) {
 
 	@Override
 	public boolean isEnabled() {
-		return getRepositories().length > 0;
+		return getProjectsInRepositoryOfSelectedResources().length > 0;
 	}
 
 }

-- 
1.6.0.rc2.35.g04c6e9

Robin Rosenberg [off-list ref] wrote:

We want to go from selected resources to repositories and back
to any resource in those repositories

...

+	protected Repository[] getRepositoriesFor(final IProject[] projects) {
+		Set<Repository> ret = new HashSet<Repository>();
+		for (IProject project : projects) {
+			RepositoryMapping repositoryMapping = RepositoryMapping.getMapping(project);
+			if (repositoryMapping == null)
+				return new Repository[0];
+			ret.add(repositoryMapping.getRepository());
+		}

Hmm.  So if any one of the selected projects doesn't have a Git
repository at its top level we just plain fail and pretend none of
them have a Git repository?  That doesn't seem right to me.  We
should just skip that project and move to another project.

But I also wonder if that really makes sense when a project could
have a linked resource under it that points to the repository's
working directory.  In such cases we want operations on that project
to potentially impact the inner repository as maybe the project
repository does not have a repository itself.  IOW I'm questioning
the idea of getRepositoriesFor(getProjectsForSelectedResources()).

-- 
Shawn.

tisdagen den 2 september 2008 17.03.23 skrev Shawn O. Pearce:

Robin Rosenberg [off-list ref] wrote:

quoted

We want to go from selected resources to repositories and back
to any resource in those repositories

...

quoted

+	protected Repository[] getRepositoriesFor(final IProject[] projects) {
+		Set<Repository> ret = new HashSet<Repository>();
+		for (IProject project : projects) {
+			RepositoryMapping repositoryMapping = RepositoryMapping.getMapping(project);
+			if (repositoryMapping == null)
+				return new Repository[0];
+			ret.add(repositoryMapping.getRepository());
+		}

Hmm.  So if any one of the selected projects doesn't have a Git
repository at its top level we just plain fail and pretend none of
them have a Git repository?  That doesn't seem right to me.  We
should just skip that project and move to another project.

That is the way everything in Eclipse work. For an operation to be applicable
to a selection, it must be applicable to eache element. Yes, I get annoyed
at that sometimes myself, but that doesn't convince me we should be
the one plugin that breaks the pattern.

Actually what happens is that the whole action gets disabled if all projects
do not satisfy the criteria.

But I also wonder if that really makes sense when a project could
have a linked resource under it that points to the repository's
working directory.  In such cases we want operations on that project
to potentially impact the inner repository as maybe the project
repository does not have a repository itself.  IOW I'm questioning
the idea of getRepositoriesFor(getProjectsForSelectedResources()).

Ouch for linked resources. I really think we should ignore the link as much
as we can. It's a trap! I any way it needs more fixing that this and not
only here. A patch does not have to solve all problems in the world :)

I have no experience with linked resources, other that I found them enough
awkward to use to prevent from doing that. How useful and portable is
a C:\foo on my system? I don't mind anyone solving the problem, but it is
very low on my personal agenda.

-- robin

Robin Rosenberg [off-list ref] wrote:

tisdagen den 2 september 2008 17.03.23 skrev Shawn O. Pearce:

quoted

Hmm.  So if any one of the selected projects doesn't have a Git
repository at its top level we just plain fail and pretend none of
them have a Git repository?

That is the way everything in Eclipse work. For an operation to be applicable
to a selection, it must be applicable to eache element.

OK.  Good reason then.
 

quoted

But I also wonder if that really makes sense when a project could
have a linked resource under it that points to the repository's
working directory.  In such cases we want operations on that project
to potentially impact the inner repository as maybe the project
repository does not have a repository itself.  IOW I'm questioning
the idea of getRepositoriesFor(getProjectsForSelectedResources()).

Ouch for linked resources. I really think we should ignore the link as much
as we can. It's a trap! I any way it needs more fixing that this and not
only here. A patch does not have to solve all problems in the world :)

I have no experience with linked resources, other that I found them enough
awkward to use to prevent from doing that. How useful and portable is
a C:\foo on my system? I don't mind anyone solving the problem, but it is
very low on my personal agenda.

Linked resources are useful (sometimes), especially if the project files
are automatically generated for you from your build system.  My prior
day-job had a really nice build system that did this.  My current day-job
also has a build system that can generate Eclipse project files for you.

In both cases the project files aren't considered part of the source as
they can be rebuilt at any time and they both used linked resources to
point to the actual place where the source was stored, which typically
was outside of your workspace directory, while the project was within
the workspace directory.

Anyway, I wasn't trying to suggest solving the worlds problems.
But RepositoryMapping is smarter now and can (mostly) handle a
linked resource just fine.  Its the higher level GUI actions that
run into trouble.

I guess we should stick this series in as-is then.

-- 
Shawn.