threads / rfc / 13923

[EGIT RFC] Commit behaviour

Subject: [EGIT RFC] Commit behaviour

## tl;dr

11 messages between Jun 12, 2008 and Jun 16, 2008.

replies: 10people: 3as markdown or json

Robin Rosenberg· Jun 12, 2008, 23:39 UTC · lore

I got annoyed about having change my selection to a project to be able to commit. 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.

Another twist would be to list all changed resources, but only enable the selected ones, or only the ones in in the same projects as the selected resources. Comments?

Code is not efficient either. This is question about the user interface.

Then we could ask ourselved, should we do something similar for Checkout and reset too? I think that is not as important as those operations are much less frequent.

-- robin
Robin Rosenberg· Jun 12, 2008, 23:39 UTC · re: Robin Rosenberg · lore

[EGIT RFC] Add utilities for figuring out repositories for selected resources

We want to go from selected resources to repositories and back
to any resource in those repositories
---
 .../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.HashSet;
 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 abstract class RepositoryAction extends TeamAction {
 	}
 
 	/**
+	 * @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.5.5.1.178.g1f811
Robin Rosenberg· Jun 12, 2008, 23:39 UTC · re: Robin Rosenberg · lore

[EGIT RFC] Enable commit for any resource in a Git-shared project

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.
---
 .../egit/ui/internal/actions/CommitAction.java     |   31 +++++++++----------
 1 files changed, 15 insertions(+), 16 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 db38c10..da32ea5 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 class CommitAction extends RepositoryAction {
 			return;
 		}
 
-		Repository[] repos = getRepositories();
+		Repository[] repos = getRepositoriesFor(getProjectsForSelectedResources());
 		amendAllowed = repos.length == 1;
 		for (Repository repo : repos) {
 			if (!repo.getRepositoryState().canCommit()) {
@@ -313,21 +313,20 @@ public class CommitAction extends RepositoryAction {
 	}
 
 	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 @@ public class CommitAction extends RepositoryAction {
 
 	@Override
 	public boolean isEnabled() {
-		return getRepositories().length > 0;
+		return getProjectsInRepositoryOfSelectedResources().length > 0;
 	}
 
 }

-- 
1.5.5.1.178.g1f811
Marek Zawirski· Jun 13, 2008, 22:07 UTC · re: Robin Rosenberg · lore

Re: [EGIT RFC] Commit behaviour

Robin Rosenberg wrote:
Show 5 quoted lines
> I got annoyed about having change my selection to a project to be able
> to commit. 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.

That's a nice idea! I also thought about that one day, as such feature already exists in (e.g.) Subclipse that I was used to.

> 
> Another twist would be to list all changed resources, but only enable
> the selected ones, or only the ones in in the same projects as the selected
> resources. Comments?

I think that only selected && changed ones should be listed. When user selects explicitly some resources, he/she is probably interested only in these ones. And he/she probably did it for easier selection than from list of all changed resources, isn't it?

> Code is not efficient either. This is question about the user interface.

Theses patches don't work for me however :/ When I introduce some change to a resource/file, even add it to index and click commit (toolbar or menu), commit action does nothing. It is - nothing happens. Can you reproduce this problem, or should I debug it on my instance? Or do I use it in some wrong way?

Show 5 quoted lines
> Then we could ask ourselved, should we do something similar for Checkout
> and reset too? I think that is not as important as those operations are
> much less frequent.
> 
> -- robin

Well, I think that checkout (maybe reset too) would be useful too - to revert some file quickly.

-- 
Marek Zawirski [zawir]
marek.zawirski@gmail.com
Marek Zawirski· Jun 13, 2008, 22:41 UTC · re: Marek Zawirski · lore

Re: [EGIT RFC] Commit behaviour

> Robin Rosenberg wrote:
>> Then we could ask ourselved, should we do something similar for Checkout
>> and reset too? I think that is not as important as those operations are
>> much less frequent.

Hey, by the way - do we already have some action for checkout? Did I missed something?

-- 
Marek Zawirski [zawir]
marek.zawirski@gmail.com
Robin Rosenberg· Jun 14, 2008, 08:40 UTC · re: Marek Zawirski · lore

Re: [EGIT RFC] Commit behaviour

lördagen den 14 juni 2008 00.41.33 skrev Marek Zawirski:
Show 7 quoted lines
> > Robin Rosenberg wrote:
> >> Then we could ask ourselved, should we do something similar for Checkout
> >> and reset too? I think that is not as important as those operations are
> >> much less frequent.
> 
> Hey, by the way - do we already have some action for checkout? Did I 
> missed something?
You missed it. 
It's been there for a while (thanks to Roger Soares), but it is currently only available when a project is selected. It's in the team menu or, if you enable Git in Window/Customize Perspective, in the toolbar and menu bar. 
-- robin
Robin Rosenberg· Jun 14, 2008, 10:05 UTC · re: Robin Rosenberg · lore

Re: [EGIT RFC] Commit behaviour

lördagen den 14 juni 2008 10.40.02 skrev Robin Rosenberg:
Show 12 quoted lines
> lördagen den 14 juni 2008 00.41.33 skrev Marek Zawirski:
> > > Robin Rosenberg wrote:
> > >> Then we could ask ourselved, should we do something similar for Checkout
> > >> and reset too? I think that is not as important as those operations are
> > >> much less frequent.
> > 
> > Hey, by the way - do we already have some action for checkout? Did I 
> > missed something?
> 
> You missed it. 
> 
> It's been there for a while (thanks to Roger Soares), but it is currently only available when a project is selected. It's in the team menu or, if you enable Git in Window/Customize Perspective, in the toolbar and menu bar.

Sorry about that. It's the Branch action. I think the original thought was to make the dialog do more, like create branches. We should rename it regardless.

-- robin
Robin Rosenberg· Jun 14, 2008, 09:08 UTC · re: Marek Zawirski · lore

Re: [EGIT RFC] Commit behaviour

lördagen den 14 juni 2008 00.07.59 skrev Marek Zawirski:
Show 19 quoted lines
> Robin Rosenberg wrote:
> > I got annoyed about having change my selection to a project to be able
> > to commit. 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.
> 
> That's a nice idea! I also thought about that one day, as such feature 
> already exists in (e.g.) Subclipse that I was used to.
> 
> > 
> > Another twist would be to list all changed resources, but only enable
> > the selected ones, or only the ones in in the same projects as the selected
> > resources. Comments?
> 
> I think that only selected && changed ones should be listed. When user 
> selects explicitly some resources, he/she is probably interested only in 
> these ones. And he/she probably did it for easier selection than from 
> list of all changed resources, isn't it?

Selection is often implicit, i.e. if you link editor and selection, which I do, the selection change every time you switch editor.

Show 7 quoted lines
> > Code is not efficient either. This is question about the user interface.
> 
> Theses patches don't work for me however :/ When I introduce some change 
> to a resource/file, even add it to index and click commit (toolbar or 
> menu), commit action does nothing. It is - nothing happens. Can you 
> reproduce this problem, or should I debug it on my instance? Or do I use 
> it in some wrong way?

It it's too hard to use properly it's wrong. Anyway I could reproduce it. Not sure why it works sometimes though. I found this in the workspace log. Please check if you see it too, so I know we're chasing the same bug.

java.lang.ArrayIndexOutOfBoundsException: 0
        at org.spearce.egit.ui.internal.actions.CommitAction.loadPreviousCommit(CommitAction.java:127)
Show 8 quoted lines
> > Then we could ask ourselved, should we do something similar for Checkout
> > and reset too? I think that is not as important as those operations are
> > much less frequent.
> > 
> > -- robin
> 
> Well, I think that checkout (maybe reset too) would be useful too - to 
> revert some file quickly.
We do not have a per-file checout/reset yet. What we have checks out all files in the repository.
-- robin
Robin Rosenberg· Jun 14, 2008, 10:23 UTC · re: Marek Zawirski · lore

Re: [EGIT RFC] Commit behaviour

lördagen den 14 juni 2008 00.07.59 skrev Marek Zawirski:
Show 5 quoted lines
> Theses patches don't work for me however :/ When I introduce some change 
> to a resource/file, even add it to index and click commit (toolbar or 
> menu), commit action does nothing. It is - nothing happens. Can you 
> reproduce this problem, or should I debug it on my instance? Or do I use 
> it in some wrong way?

Found it. Obviously this won't work for commits to multiple repositories which is supposed to work really, but I think that was broken earlier by yours truly. This fix should make it possible to test the idea though this thread is about, and see if we have more selection problems. 

-- robin
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 da32ea5..922649b 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
@@ -124,7 +124,7 @@ public class CommitAction extends RepositoryAction {
        }

        private void loadPreviousCommit() {
-               IProject project = getSelectedProjects()[0];
+               IProject project = getProjectsForSelectedResources()[0];

                Repository repo = RepositoryMapping.getMapping(project).getRepository();
                try {
Marek Zawirski· Jun 15, 2008, 02:00 UTC · re: Robin Rosenberg · lore

Re: [EGIT RFC] Commit behaviour

Robin Rosenberg wrote:
Show 21 quoted lines
> Found it. Obviously this won't work for commits to multiple repositories which is supposed
> to work really, but I think that was broken earlier by yours truly. This fix should make it
> possible to test the idea though this thread is about, and see if we have more selection
> problems. 
> 
> -- robin
> 
> 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 da32ea5..922649b 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
> @@ -124,7 +124,7 @@ public class CommitAction extends RepositoryAction {
>         }
> 
>         private void loadPreviousCommit() {
> -               IProject project = getSelectedProjects()[0];
> +               IProject project = getProjectsForSelectedResources()[0];
> 
>                 Repository repo = RepositoryMapping.getMapping(project).getRepository();
>                 try {
> 
Yeah, it works for me now, with this patch applied.
And yes, without this patch, I found mentioned exception in log:
java.lang.ArrayIndexOutOfBoundsException: 0
  at 
org.spearce.egit.ui.internal.actions.CommitAction.loadPreviousCommit(CommitAction.java:127)
-- 
Marek Zawirski [zawir]
marek.zawirski@gmail.com
Shawn O. Pearce· Jun 16, 2008, 02:11 UTC · re: Robin Rosenberg · lore

Re: [EGIT RFC] Commit behaviour

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
Show 9 quoted lines
> I got annoyed about having change my selection to a project to be able
> to commit. 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.
> 
> Another twist would be to list all changed resources, but only enable
> the selected ones, or only the ones in in the same projects as the selected
> resources. Comments?

I think this is a good idea. You really do want to make commits that cover the entire repository, rather than a subsection of it. I think that covering the whole repository is much more common.

> Then we could ask ourselved, should we do something similar for Checkout
> and reset too? I think that is not as important as those operations are
> much less frequent.

Yes, we do want to. If you are doing a reset you probably need to whack everything that is dirty and restore the state, likewise if you are doing a checkout you are likely switching branches and would expect the entire repository to switch at once.

-- 
Shawn.

← back to recent threads