Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #250419 +++ See bug 249981, comment 0 for details. While in compare.core we and other RCP plug-ins cannot use the range differencer as the plug-in depends on core.resources. It would be great to have the core plug-in be independent of core.resources because otherwise it would be of no use for us.
Another possibility is to make the plug-in optionally depend on core.resources. This means you would still need the core.resources bundle to build core.compare but the plug-in would still work without resources. If this approach was taken, you would need to isolate the use of IResource, IStorage and other API that came from the resources bundle to classes that were not required for RCP use.
Creating a separate plugin sounds like a good idea for me. I don't like these optional dependencies.
BTW, why does RCP app have to take whole compare.core plug-in, if it just needs RangeDifferencer API.
>BTW, why does RCP app have to take whole compare.core plug-in Because there is currently no alternative ;-)
Created attachment 125131 [details] Fix v01 (compare and compare.core)
Created attachment 125132 [details] Fix v01 (new compare.rangedifferencer)
Fix v01 is the first step. The second would be to rename compare.core into compare.patch.
Szymon, I wonder if it would be worth it to push much of the patching code into a non-resource based plug-in as well. In the patch API, there are only two places where IStorage is used: 1) The PatchParser class uses IStorage as the input 2) IFilePath#apply uses IStorage as the input You could define the equivalent of IStorage and IEncodedStorage in the Compare/Core plug-in and use these in the Core patching support. The tricky part is how to deal with IFilePath#apply. You would probably need to define a new super interface with no dependencies on IStorage and have IFilePatch extend that and define the IStorage based apply. I mention this because we, Jazz SCM, are moving away, as much as possible, from depending on the Eclipse resource layer so we can provide better support form non-Eclipse based client like Visual Studio. Any work you do in decoupling Compare functionality from Resources would be helpful in this respect. Ideally, there would be a Compare/Core and a Compare/UI that did not depend on the Resource layer at all. P.S. Another approach to this would be to push IStorage/IEncodedStorage down into a more RCP accessible bundle.
(In reply to comment #8) I'm going to create a separate compare.rangedifferencer plug-in anyway. I raised a bug for moving IStorage down to runtime/equinox plug-in (bug 264327). Depending on that I'll try to remove core.resources dependency in compare.core too.
I like the approach that Michael suggested in comment 8 better. Creating a new bundle for just the range differencer sounds like overkill to me.
(In reply to comment #10) > I like the approach that Michael suggested in comment 8 better. Creating a new > bundle for just the range differencer sounds like overkill to me. > Only if we move IStorage down, the approach from the comment 8 is ok for me. Creating an interface similar to IStorage in the compare doesn't look like a good idea.
What about using EFS API like IFileStore?
(In reply to comment #12) > What about using EFS API like IFileStore? I assume that we don't want to make a dependency to core.filesystem. Ideally, compare.core would depend only on core.runtime.
>I assume that we don't want to make a dependency to core.filesystem Yes, you are right.
Created attachment 125622 [details] Fix v02
I got rid of core.resources in compare.core.
So, just to summarize, you have pushed IFilePath and ParsePatcher back up to org.eclipse.compare and created an IFilePatch2 and ParsePatcher2 in org.eclipse.compare.core? I believe that PatchParser is new in 3.5 so you should not need to make a PatchParser2 for that class. You could just change PatchParser to take a ReaderFactory. Also, IFilePatch is not implementable by clients so technically, IFilePatch could extend IFilePatch2 and just define the one method that takes IStorage. Similarly, FileDiffWrapper in org.eclipse.compare could extend FileDiff and define the apply method that takes an IStorage.
(In reply to comment #17) Yes. PatchParser could be removed and IFilePatch can extend IFilePatch2. I'm going to release it before next IBuild.
Created attachment 125766 [details] Fix v03
Fix v03 released to HEAD with some minor mods (e.g. Utilities.createReader is used more often).
