Community
Participate
Working Groups
Currently quick diff providers are associated at the text editor level. This mechanism is fine when there is only one team provider in use. When there are multiple team providers in use in the workspace the user has to either juggle the default value or manually switch the quick diff for each editor. It would be really nice if there were one 'team' quick diff provider that would use the quick diff that is provided by the team provider of the file being edited. This approach may be problematic if a provider has more than one quick diff.
Its a good idea but it's too late to consider this for 3.1
Would it be possible to do this using the new getSubscriber API on RepositoryProviderType? So long as the provider has a working sync, it should work. I have hacked up the existing CVS provider to use this API and it seems to work when the file is managed by Subclipse. If you think this is the correct approach i will attach a patch.
Created attachment 45634 [details] patch to org.eclipse.team.ui This patch adds a 'Team state' quick diff provider. The provider is a copy of the CVS provider with references to the CVSWorksapceSubscriber replaced with calls to the subscriber of the current file (which is setup when the editor is activated). I also removed the use of the buffered reader as it is already reading into a buffer. This should save 15kb of garbage per reference load. I have tested that it works for both CSV and Subclipse. remaining issues: NLS needs to be done What to do when a file is opened before the team provider is assocciated
Instead of using the remote resource variant, would it be possible to use the base resource variant instead? It makes more sense to show the user how their Working Copy differs to what they have checked out instead of what was at the head revision when the file was opened. It might even make it possible for the quick diff to work for CVS when the user is offline as there should be no need to contact the server.
Using the base makes sense, especially considering that the QuickDiff annotation makes use of this. I don't mind making the QuickDiff provider API but I have concerns about having a workbench quickdiff provider that delegates to the Subscriber since we cannot expect all repository providers to implement a Subscriber. A better approach would be to have a factory associated with the RepositoryProviderType from which the workspace quickdiff provider can be obtained. This would give repository providers the flexibility to decide how to implement their providers.
Do you know how many team providers do not use the synchronize infrastructure? I think Team Providers should be encouraged to implement the synchronize interface (and therefore provide a workbench subscriber). The subscriber could possibly be used for other features such as cross provider sync as well as cross provider commit. I know Mylar could use it for its version of cross provider commit to know which providers have outgoing changes in the current selection. It could also be used to show incoming/outgoing decorators on workspace files. If no factory is provided, would it use a default one that uses the subscriber anyway? As someone who works on a team provider, i can assure you i would prefer to get something for free :)
CVS and Subversion are the only two repository providers I am aware of the use the Subscriber API. There are probably others but they haven't made themselves known to me. I know of two that definitly don't (and may never since it would mean throwing away what they have and starting again) and one other that probably doesn't (based on the UI I saw). In regards to incoming/outgoing decoration, we introduced an ITeamStateProvider in 3.2 which could be used for this purpose. It follows the same pattern I suggested in comment 5 and has a subclass that makes use of a Subscriber for those that have one. This pattern is really the best of both worlds. It allows all to participate and makes it easier for those that have a Subscriber. If a repository provider did not choose to provide a quick diff through the RepositoryProviderType factory, we could default to looking for a Subscriber. It is the migration story that actually concerns me. We will inevitably end up in a state where some quick diff providers will hook in and others will not (due to release dates, manpower, etc). One possible solution to this would be to allow the user to associate a QuickDiff type with a particular project or repository provider type themselves. This could actually be combined with the other strategies discussed (i.e. Subscriber, RepositoryProviderType adapted to a factory) to ensure that the proper quick diff is used when the user chooses to see the Team state. With regards to reusing components provided by Team like the Subscriber API, I also thought that there would be a lot of interest in this but it turns out there isn't. I suspect the reasons for this are as follows. The first is that many commercial repository providers already have a large code base and don't want to start over. The second is that the Eclipse code is fixed for a particular release so if the shape isn't quite right for your repository provider, you need to come up with your own solution if you want to provide a plugin for that release. This means that using the code in Team is actually a liability for some. The third is that there is actually a large difference in how repository providers work. CVS and Subversion are very similar (since Subversion is the evolution of CVS) but I have seen several other repositories that have distintly different ways of working. For these repositories, the Subscriber API, for example, doesn't make sense.
Thanks Michael. I agree your solution is better as it works for everyone. My only concern is that RepositoryProviderType is a core class, so can't reference quick diff classes which are in the workbech text editor plugin. Perhaps a UI version fo the RepositoryProviderType is needed? Mylar already has something like this to ask a team provider if there are outgoing changes and to open the commit wizard. I am not sure that you really need an elaborate migration strategy. Most users will only have one team provider and those that have more than one already have to switch between quick diff providers, so they lose nothing (although they also gain nothing)
Your concern about the Core/UI split is valid. Eclipse has the IAdaptable/IAdapterFactory feature to deal with this. We used this for the ITeamStateProvider (and many other cases). Basically, the Team/UI plugin registers an adapter factory that adapts a RepositoryProviderType to an ITeamStateProvider. Clients call RepositoryProviderType#getAdapter(ITeamStateProvier.class) to get the appropriate UI class. For QuickDiff, we would need to adapt the RepositoryProviderType to a QuickDiffFactory that would create a provider for a particular editor.
Created attachment 46341 [details] Updated patch using IAdaptable Here is an updated patch using IAdaptable and factories. I had to make the following changes: - Made RepositoryProviderType implement IAdaptable - Added an IQuickDiffFactory interface - Implemented IQuickDiffFactory using an internal class in Team UI that returns the subscriber based quick diff provider - Added a delegating quick diff provider that delegates to the provider created by the quick diff factory - Added WorkspaceSubscriberQuickDiffProvider that creates reference documents from the Team Provider's subscriber (if available) There are no unit tests and no documentation so far. I didn't create an extension point to allow team providers to supply their own quick diff factory. Instead, they can create an adaptable factory for IQuickDiffFactory on their subclass of RepositoryProviderType.
Thanks for the patch. The general direction sounds good. I'm going on vacation for the next 3 weeks or so. I'll have a more in-depth look at the patch once I get back. The one thing I will mention is copyrights. The ones on the new files are probably acceptable but they should probably mention you as the initial contributor and not IBM (although if a class is mostly a copy of another, you could leave IBM as the initial contributor and add yourself as another contributor). I would rather see your name there then in the class comment (i.e. we generally don't include the author names in our class comments).
Created attachment 49292 [details] Updated patch with documentation I have attached an updated patch with better documentation and the author and headers fixed. I have verified that the quick diff provider works with both Subclipse and the CVS provider without any further changes.
Thanks. I'll have a look.
I am conflicted about this. I appreciate the effort you have made and I like the idea but I am still concerned about the presentation. Consider the following scenario: A user has projects from Subversion and Repository X in their workspace. They see the following QuickDiff providers: Local File CVS Subversion Repository X Team state They know that they want to use Repository X for their repository X projects and Subversion for their subversion projects. It is not clear from the label that this is what Team state would provide. Let's say that the are able to guess that this is the purpose of Team state and they select it. Lets also say that repository X does not use a Subscriber and doesn't provide their own quick diff factory. From the users perspective, Team state would be equivalent to Subversion and they would have no clue how to fix it to include Repository X. Based on this, I feel that the potential for confusion this patch would cause outweighs the benefit of putting it in. I would prefer an approach that associated the quick diff provider on a per project basis and had some means of the repository provider setting the default. This would be more intuitive to the user and provide a means for them to do it manually for repository providers that didn't set the default. Per project quick diff is discussed in bug 46090. I am making this bug depend on that because of the reasons I mentioned. On a side note, I have made RepositoryProviderType extend PlatformObject which means that the adapter manager is consulted to get adaptables. With this change, there is nothing to stop you from creating a separate plugin that creates the Team state QuickDiff provider. I admit this is not ideal but it will let you (and others) use the funtionality until bug 46090 is addressed. The one gotcha is the getAdapter vs loadAdapter. Since RepositoryProviderType extends PlatformObject it is calling getAdapter. Bug 82973 discusses this issue. Until that is resolved, the workaround is to have your plugin implement the IStartup hook.
Michael, you maybe right about delegating provider, but on the other hand there is already a link between project's resources and the team provider. Team provider and the diff factory should know what provider to use for resources from given project. So, that selection should be taken care automatically without additional preferences (as long as you can find matching team provider).
Yes, there is a link between a project and a team provider but there is no link between a team provider and there quick diff provider. Also, there is no way to tailor the quick diff provider on a per project basis. Both of these are required to adequately solve the problem.
wrt comment 14, I would have expected the team provider to remove their specific quick diff provider once they support the 'team' one. I don't really understand why you need to link a project directly to a quick diff provider. Projects are already linked to team providers and there will only be one quick diff for each team provider. It also doesn't really make sense to use the local file quick diff for a team managed project as the team quick diffs are able to provide more helpful information. Are you more concerend about team providers that do not publish a team quick diff? Is it safe to assume that bug 46090 isn't targeted for 3.3?
BTW, I think it could be interesting in some cases to have local quick diff... though it has limited usage, because local history does not allow to use some kind of markers/tags...
How about if it is renamed to 'Smart diff' or something like that and have it delegate to the local quick diff if the project is not team managed. This should cover most of the platform use cases. People with team providers that do not provide a hook for the 'smart' quick diff can continue to switch the global provider (as they already do). It might also be a good idea to display a description with the quick diff provider in the preference page (but this would require an addition to the schema). This would reduce the confusion about what this new provider does.
Yes, I am concerned about the case where a repository provider does not hook the global quick diff as the scenario I described points out. From a user standpoint, it would just seem odd (not to mention frustrating) that only some repository providers are hooked in and they won't understand why they can't hook up theirs. Given this, I think the only viable solution is one that allows the user to specify which quickdiff to use on a per project basis. Repository providers that are fast to move onto new features can help out by automatically associating their QuickDiff provider with their projects. For those repository providers that do not react, the user would still have the capability of setting the QuickDiff for the projects manually. This seems a lot cleaner then introducing a new Quick Diff provider called "Team state" or "Smart Diff" that works in some cases and not in others.
Michael, don't you find this really weird? Wouldn't it be the right thing to fix/update legacy team provider to support new diff features instead of patching them up?
I also think the per project approach fits best. However, instead of letting the user to configure each project, there could be a (team) extension point that allows to map quick diff reference providers to team provider. This would override the global preference (see also bug 46090 comment 20). On a new preference page the user could select the quick diff reference provider per team provider and out of the box it would be the local one (to prevent server round trips without letting the user know).
You may not want to allow the user to override the quick diff on a per repository type basis. For instance, you may be using both a local CVS server as well as one served on the internet. You will most likely want quick diff on for the local one, but off for the remote one. What i would prefer is a way to override the quick diff on a per project basis. This way if a user sets up their global quick diff to be the 'smart' quick diff, it would work as expected for non team managed projects (by using the version on disk quick diff) as well as for projects managed by team providers that have either registered a default quick diff or provide a workspace subscriber. For cases where the project is managed by a non compliant team provider the user can either change the global quick diff provider (as they do now) or use the not yet implemented project level preference to pick a better one.
Regarding comment 21, in an ideal world, products built on Eclipse would adopt the new features added to every release. In the real world of tight schedules and scarce resources, this does not occur. Therefore, I doubt tat existing repository providers would adopt this feature. Also, my main concern here is from a users standpoint. Most user only have one repository provider so having a Team state or Smart diff would just be confusing (although in this case they could just ignore it). In the case where the user has multipel repository providers, I think it is much more understandable to have the name of the Quick Diff provider match the name of the repository tooling and make the association at the project level. I like the solution Dani mentioned in comment 22. If an option was added to the Quick Diff page that read "Use Repository Provider QuickDiff by default" and the settings were per project, the user could then inspect the properties for a project and see that the setting was indeed for the right QuickDiff (and even override it on a per project basis if they desired). For repository providers that didn't provide the mapping in their QuickDiff definition, we could provide a simple UI that allowed the user to make the association manually.
(In reply to comment #24) > Most user only have one repository provider [...] Just to throw my two cents in ... I think this statement was true some years ago and probably is still true in some corporate environment. However, for the majority of active developers that work on projects consuming and contributing to a variety of Open Source projects this statement is wrong. Having said that, I agree with Michael that some smart quick diff provider would be too much overhead and would complicate things. Instead the quick diff provider selection algorithm should be smart enough to automatically select the quick diff provider from the repository provider of the current project or fallback to the configured one (as Dani proposed in comment 22).
(In reply to comment #25) > Having said that, I agree with Michael that some smart quick diff provider > would be too much overhead and would complicate things. Instead the quick diff > provider selection algorithm should be smart enough to automatically select the > quick diff provider from the repository provider of the current project or > fallback to the configured one (as Dani proposed in comment 22). > How do you propose this should work? Quick diffs are supplied by the text plugin which has no knowledge of team providers. One solution is to force the user to set this up on a per project basis (manually), or have the team provider setup a 'magic' property on the project. The other solution (which the patch uses) is to provide a team level quick diff provider that uses team specific knowledge to pick an appropriate provider. That is, the one specifically requested by the provider, one based on sync states or the on disk one as a last resort.
One possible solution would involve the following: 1) The ability to set the QuickDiff provider on a per project basis 2) The ability to flag a particular QuickDiff provider as the Team provider (either as a flag on the QuickDiff definition or a new extension point at the Team/UI level). 3) A preference to indicate that the default provider for all projects should be the Team provider. 4) The ability to programmatically set the QuickDiff provider of a project (so that the QuickDiff provider can be changed when the preference setting is changed or a new project is created). When the preference was enabled, all existing projects with the default provider would be changed to the Team provider. Any newly created projects would also need to have the provider set. The implementation of this would be easiest if the Text plug-in managed the preference but it is doable from Team/UI as well. Another simplification would be to have Text delegate the default determination to the RepositoryProvider by adapting the provider to a particular interface and, if it finds it, querying the interface for the default QuickDiff provider.
*** Bug 337331 has been marked as a duplicate of this bug. ***
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. If the bug is still relevant, please remove the "stalebug" whiteboard tag.
