Re: [EGIT PATCH 1/2] Add utilities for figuring out repositories for selected resources

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