Community
Participate
Working Groups
I would like to have an export or even a print feature for trees generated with the "compare width"->"each other" feature. This is important for synchronisation of different cvs-repositorys in spacelly separatet locations without network connections or when patches are not correct applyable.
*** Bug 72918 has been marked as a duplicate of this bug. ***
As requested in bug 72918, it would be also good to be able to get a diff from any compare editor.
*** Bug 131901 has been marked as a duplicate of this bug. ***
*** Bug 68690 has been marked as a duplicate of this bug. ***
Bug 68690 asks for diffs against the filsystem.
Being able to generate a diff from any compare editor will cover scenarios described in: * bug 102681: "save diff between two versions in the Local History" * bug 151980: "generate a patch from differences between the files". The files location shouldn't be restricted to the workspace only.
*** Bug 103681 has been marked as a duplicate of this bug. ***
*** Bug 151980 has been marked as a duplicate of this bug. ***
As a reporter of bug #151981 I'd like to underline that this bug required that it should be possible to open compare editor against one or two files even _outside_ the workspace. As far as I know there's no such feature in Eclipse yet.
*** Bug 227436 has been marked as a duplicate of this bug. ***
Bug 227436 asks for a way to generate a diff from two CVS revisions. "Moreover, it would be useful to be able to create patches based on tags or dates, not only for single files but for also for folders/projects."
(In reply to comment #9) > As a reporter of bug #151981 I'd like to underline that this bug required that > it should be possible to open compare editor against one or two files even > _outside_ the workspace. As far as I know there's no such feature in Eclipse > yet. That's true, this is covered by bug 73923.
Another bug overlapping with this one is bug 236877, scenarios mentioned there: * create a patch of incoming changes * create a patch of a CVS-compare
*** Bug 236877 has been marked as a duplicate of this bug. ***
*** Bug 222634 has been marked as a duplicate of this bug. ***
Please add "Create patch" to the "Compare with" menu, too.
*** Bug 256771 has been marked as a duplicate of this bug. ***
Created attachment 119341 [details] patch v1 First version of patch. Works only for 2 files. Added new action(Create Patch...) in context menu of Compare Editor.
Created attachment 120045 [details] Patch_v02 I changed patch attached in comment 18 reusing UnifiedDiffFormatter. The new implementation differs in following ways: - compare results are no longer regenerated for patch creation, they are simply taken from DocumentMerger; - it works with Team component present, previous one assumed that EditorInput is always ResourceCompareInput type; - now it is possible to create a patch between any comparison displayed in a compare editor; - some other minor issues are addressed as well. Tomasz, please review it to make sure I addressed all your objections.
The last line in the file has to be treated with special care not to loose information if it contains a line delimiter. As of now the UnifiedDiffFormatter doesn't handle such cases. Please see bug 116023 and bug 249954 for details.
Krzysiek, Mariusz, Kacper, could you adjust the latest patch to handle cases referred in comment 20? I'm asking because your knowledge about the formatter is much greater than mine is.
Created attachment 120060 [details] Patch_v03 Excluding formatting changes from the patch.
Created attachment 120066 [details] createpatch_wizban.png To put into org.eclipse.compare\icons\full\wizban\createpatch_wizban.png.
We confirm that we wrote 100% of the code in the provided patches. This code may be contributed to Eclipse. All file headers in files created by us contain the EPL License header. Krzysztof Poglodzinski Mariusz Tanski Kacper Zdanowicz
Bug 259422 is related to Pawel request (comment 21) and fix some of the problems.
Created attachment 120987 [details] Patch_v04 Another patch. Based on v03, fixed rest of the problems mentioned in comment 20 and some minor others. (still needs solution of bug 259422 to work correctly) But I have a problem: Unix-like 'diff -u' command does not generate '/no newline at end of file' marker. Also 'patch' command does not support it. But eclipse 'apply patch' does, and behaves in different way than 'patch' command. It should be the same 'uniffied diff' format. Am I wrong, or the Eclipse 'apply patch' isn't compatible with Unix 'diff -u' command?
Created attachment 120988 [details] initial testCase for UnifiedDiffFormatter class This are the initial tests for UnifiedDiffFormatter, they are to be continued and developed further. There are 2 tests for creating patch between regular files, terminated with blank line and 1 test for creating patch between files not terminated with blank line.
(In reply to comment #26) > Unix-like 'diff -u' command does not generate '/no newline at end of file' > marker. Also 'patch' command does not support it. But eclipse 'apply patch' > does, and behaves in different way than 'patch' command. It should be the same > 'uniffied diff' format. Am I wrong, or the Eclipse 'apply patch' isn't > compatible with Unix 'diff -u' command? What do you mean by "behaves in a different way"? Does it mean that there are cases where a vanilla patch in uniffied diff format cannot be properly applied using Eclipse patcher? Is so, I would file a separate bug for it and attach the cases illustrating the issue. Feel free to add me to CC list of the bug.
(In reply to comment #26) > Created an attachment (id=120987) > Patch_v04 Before I take a deeper dive into the code ;) let me share with you what I've observed so far: === code: - we're supposed to use ICU lib for date operations - add the UniffiedDiffFormatter suite to org.eclipse.compare.tests.AllTests - catching any Exception in org.eclipse.compare.internal.CreatePatchAction.getPath(boolean) is unsafe, please take a look at org.eclipse.jdt.internal.ui.javaeditor.JavaEditor.setActionsActivated(boolean) as an example of how to use reflection more carefully. This applies to other reflection methods. Also, please comment on them why we have to use reflection here, and what would make it gone (optionally). - the "Create Patch" action is added to all 3 viewers of the TextMergeViewer. I'm especially interested in the ancestor case, how does it work? Will it generate a patch according to the ancestor? The code in org.eclipse.compare.internal.CreatePatchAction.getDocument(boolean) appears to be unaware of it. - make sure dates in copyrights section is correct, for instance SaveDiffFileWizard is a new class, use 2008 there - javadoc for SaveDiffFileWizard is inappropriate (it doesn't use CVS diff command), also I think you could mention that this is actually a copy of the CVS patch wizard and point the differences - the next step would be to file a bug to avoid the code duplication (maybe even move the wizard completely to compare and extend it in team.cvs.ui) - I would keep the same class name for the wizard in team.cvs.ui and compare - is the Create Patch action bindable, is it the same command (key binding) as Create Patch from CVS? - on the first page of the wizard add "&Left:" and "&Right:" labels - could we validate if the value entered in "Use only file path" is valid? - the "Use only file path" option has the same mnemonic key as "Unified" -> U === tests: - imho you don't need a getter for an anonymous class, create an inner class - "L", "R" and so on are accessible from org.eclipse.compare.internal.MergeViewerContentProvider - please format the code and remove unnecessary empty lines
> What do you mean by "behaves in a different way"? Does it mean that there are > cases where a vanilla patch in uniffied diff format cannot be properly applied > using Eclipse patcher? Is so, I would file a separate bug for it and attach the > cases illustrating the issue. Feel free to add me to CC list of the bug. > But about this problem: bug 259636 Currently solution of 71374(Patch_v04) generates patch compatible with Unix commands.
I confirm that I wrote 100% of the code in the provided patches. This code may be contributed to Eclipse. All file headers in files created by me contain the EPL License header. Kacper Zdanowicz
I confirm that I wrote 100% of the code in the provided patches. This code may be contributed to Eclipse. All file headers in files created by me contain the EPL License header. Krzysztof Pog這dzi雟ki
I confirm that I wrote 100% of the code in the provided patches. This code may be contributed to Eclipse. All file headers in files created by me contain the EPL License header. Mariusz Tañski
Created attachment 122015 [details] patch addressing issues from Comment #29 We addressed all the issues given by Tomasz in comment #29. We have also rewritten a GenerateDiffFileWizard from org.eclipse.team.cvs.ui plugin, so that it is now a sublass of our GenerateDiffFileWizard from org.eclipse.compare plugin. The purpose of this was to avoid code duplication (Bug 260452)
Created attachment 122016 [details] test_cases_v02.txt We added some new tests and updated previous ones.
(In reply to comment #35) > Created an attachment (id=122016) > test_cases_v02.txt > > We added some new tests and updated previous ones. One of the tests from test_cases_v02.txt fails. Could you take a look at it? AllTests (org.eclipse.compare) Test for org.eclipse.compare.tests org.eclipse.compare.tests.UnifiedDiffFormatterTest testEmptyFilesPatch(org.eclipse.compare.tests.UnifiedDiffFormatterTest) junit.framework.AssertionFailedError: Failed while reading empty1.txt at junit.framework.Assert.fail(Assert.java:47) at org.eclipse.compare.tests.UnifiedDiffFormatterTest.asInputStream(UnifiedDiffFormatterTest.java:198) at org.eclipse.compare.tests.UnifiedDiffFormatterTest.getReader(UnifiedDiffFormatterTest.java:187) at org.eclipse.compare.tests.UnifiedDiffFormatterTest.readFileToString(UnifiedDiffFormatterTest.java:175) at org.eclipse.compare.tests.UnifiedDiffFormatterTest.patch(UnifiedDiffFormatterTest.java:137) at org.eclipse.compare.tests.UnifiedDiffFormatterTest.testEmptyFilesPatch(UnifiedDiffFormatterTest.java:121) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:130) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:120) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at junit.framework.TestSuite.runTest(TestSuite.java:230) at junit.framework.TestSuite.run(TestSuite.java:225) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390) at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62) at org.eclipse.pde.internal.junit.runtime.UITestApplication$1.run(UITestApplication.java:114) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:133) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3852) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3473) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2384) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2348) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2200) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:495) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:490) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113) at org.eclipse.pde.internal.junit.runtime.UITestApplication.start(UITestApplication.java:46) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:366) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:177) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:550) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:505) at org.eclipse.equinox.launcher.Main.run(Main.java:1237) at org.eclipse.equinox.launcher.Main.main(Main.java:1213)
Created attachment 122409 [details] TestCases_v03 Thanks for the patches guys! I started from the end, and adjusted the patch with test cases. Main changes I made are: - moved all the text files used in UDFT to a separate folder - test method checks whether generated patch is similar to the expected one and then applies the generated patch and checks when the output makes any sense - extracted superclass for PatchTest and UDFT as they have a number of common methods Let me know if this still works as you expected? PS. You will have to manually add empty files (empty1.txt, empty2.txt, addition.txt) since they are not included in the patch. But you already know that, don't you? ;)
Created attachment 122410 [details] mylyn/context/zip
Unfortunately, imho the patch is still not ready to commit. Couple of findings grouped by classes. CreatePatchAction: - you can get CompareConfiguration from DocumentMerger - replace boolean flag used to distinguish between left and right viewer with LEFT_CONTRIBUTOR and RIGHT_CONTRIBUTOR this will make the code clearer - try find a way to get some of the objects you need in the action without reflection, I'm pretty sure you don't need to use it in all cases. The reflection methods make the code looks ugly, don't you think? - what does getPath(true) do in org.eclipse.compare.internal.CreatePatchAction.run()? - fix copyrights GenerateDiffFileWizard: - what's the purpose of run(..., boolean unifiedSelectionEnabled) method? Do we really need to parameterize whether the unified option is enabled? It looks like it's always true in our case. - run methods with 9 and 10 parameters scared me, can we do anything to make the list a little bit shorter? ie you can get left and right label from CompareConfiguration object or a try to get the doc from the DocumentMerger and so on. And again, I'm pretty sure you won't need to use reflection to get to this objects. If you're lacking a getter or if it's not visible as long as it's in an internal class you can add it. This will make the code clearer and afterwards we will decide whether we need to add these getters. - add an info that it's actually a copy of org.eclipse.team.internal.ccvs.ui.wizards.GenerateDiffFileWizard - since the above, try to keep class format as close to original as possible ie don't make changes like "// enable (or disable) given buttons" to "// Enable (or disable) given buttons" and don't format the code, don't remove or add empty lines. This make comparing the copy and the original harder since there is now a lot of changes which are irrelevant. - what I was able to find is that in org.eclipse.compare.internal.GenerateDiffFileWizard.DirectionSelectionPage you could use values for margins from org.eclipse.jface.dialogs.IDialogConstants instead of hard-coded ones - there are warning that some variables not used locally - I think I will be more helpful here as soon as you tidy up the code the way I described above, tia UnifiedDiffFormatter.java: - what do we need FORMAT_* constants for? - do all constants need to be public? - generateDiff() methods don't need to be public, right? - for the purpose of Clipboard creation I think we can get away with new Clipboard(Display.getDefault()); - informing that the diff created is not 100% compatible with GNU diff format is great, but it would be better if you place it also in the exact place where the difference occurs. Next to the piece of code which makes the difference. Moreover, I think it could be a global switch for UDF so there is no need to pass it as an argument. - I assume not all of the methods from the class need to be public - do we need to pass Locale to Calendar.getInstance(Locale.US).getTime() lines 134 and 135 - are we able to add time to the file diffs? - I didn't look much into the code of the UDF but as long as tests passes I guess there is no such need. I will do this as soon as you address all the other issues. Also Szymon promised to do some extra testing of the formatter, he will ping us if he finds anything unusual CompareMessages.java: - CompareMessages.Configure_the_options_used_for_the_CVS_diff_command_20 needs to be replace with something more appropriate CompareMessages.properties: - replace "Save Diffs" with "Create Patch..." All: - replace e.printStackTrace() with CompareUIPlugin.log(e); - fix sysouts - add 2009 to all copyrights as we won't make it in 2008 :)
Created attachment 122452 [details] TestCases_v04 All your changes work fine, Tomasz. I only added one small fix - now if some test files like empty1.txt or empty2.txt don't exist, test class creates them in a proper directory.
Created attachment 122882 [details] Patch_v06 New version of patch added, addressing issues from Comment #39 by Tomasz CreatePatchAction: - you can get CompareConfiguration from DocumentMerger fix - replace boolean flag used to distinguish between left and right viewer with LEFT_CONTRIBUTOR and RIGHT_CONTRIBUTOR this will make the code clearer ok - try find a way to get some of the objects you need in the action without reflection, I'm pretty sure you don't need to use it in all cases. The reflection methods make the code looks ugly, don't you think? Yes, we think. fix. - what does getPath(true) do in org.eclipse.compare.internal.CreatePatchAction.run()? removed - fix copyrights fixed CompareMessages.java: - CompareMessages.Configure_the_options_used_for_the_CVS_diff_command_20 needs to be replace with something more appropriate CompareMessages.properties: - replace "Save Diffs" with "Create Patch..." fixed GenerateDiffFileWizard: - what's the purpose of run(..., boolean unifiedSelectionEnabled) method? Do we really need to parameterize whether the unified option is enabled? It looks like it's always true in our case. this option was in original GenerateDiffFileWizard from org.eclipse.team.cvs.ui, we analyzed it and it doesn't make sense to us too :) - removed. - run methods with 9 and 10 parameters scared me, can we do anything to make the list a little bit shorter? ie you can get left and right label from CompareConfiguration object or a try to get the doc from the DocumentMerger and so on. And again, I'm pretty sure you won't need to use reflection to get to this objects. If you're lacking a getter or if it's not visible as long as it's in an internal class you can add it. This will make the code clearer and afterwards we will decide whether we need to add these getters. list of parameters shortend to 2: DocumentMerger merger, boolean rightToLeft. Labels and docs obtained from merger. - add an info that it's actually a copy of org.eclipse.team.internal.ccvs.ui.wizards.GenerateDiffFileWizard added. - since the above, try to keep class format as close to original as possible ie don't make changes like "// enable (or disable) given buttons" to "// Enable (or disable) given buttons" and don't format the code, don't remove or add empty lines. This make comparing the copy and the original harder since there is now a lot of changes which are irrelevant. we organized the code, so that is easier now to comapre it to original GenerateDiffFileWizard. - what I was able to find is that in org.eclipse.compare.internal.GenerateDiffFileWizard.DirectionSelectionPage you could use values for margins from org.eclipse.jface.dialogs.IDialogConstants instead of hard-coded ones fixed. - there are warning that some variables not used locally fixed. UnifiedDiffFormatter.java: - what do we need FORMAT_* constants for? Propably introduced by Pawel. Removed. - do all constants need to be public? Not all of them. But some of them may be used in future by other classes responsible for reading/writting unified diff format. At this moment only tests uses them. I think it's good idea not to duplicate those strings in tests(and in future in other classes). - generateDiff() methods don't need to be public, right? They have to. They are invoked by GenerateDiffFileWizzard to generate diff(two of them). I added javadocs to all of them. I think that they creates an API of the UnifiedDiffFormatter. - for the purpose of Clipboard creation I think we can get away with new Clipboard(Display.getDefault()); Fixed. - informing that the diff created is not 100% compatible with GNU diff format is great, but it would be better if you place it also in the exact place where the difference occurs. Next to the piece of code which makes the difference. Moreover, I think it could be a global switch for UDF so there is no need to pass it as an argument. Added comments. There is no need to pass it as an argument. You can ommit that argument, and default patch wil be compatible with Eclipse. But with it as an argument You have the opportiniuty to create patch in strict unix format. - I assume not all of the methods from the class need to be public Fixed. Public are only the methods, which creates an API of UnifiedDiffFormatter. - do we need to pass Locale to Calendar.getInstance(Locale.US).getTime() lines 134 and 135 It's not necessery. Fixed. - are we able to add time to the file diffs? No, we aren't. Because comparing process is based on IDocument, I think that information about time is lost. - I didn't look much into the code of the UDF but as long as tests passes I guess there is no such need. I will do this as soon as you address all the other issues. Also Szymon promised to do some extra testing of the formatter, he will ping us if he finds anything unusual Ok. :)
Thanks for the patch guys. Since the code you provide gets bigger and bigger I've branched org.eclipse.compare so you can incrementally create and sent fixes. This also makes them much easier to review. The branch name is branch_20090126_LocalDiff. I made it also on org.eclipse.compare.tests. Please switch to the branch. > - fix copyrights > fixed I changed them a little bit more in some of the classes, double check them if I haven't missed anything. > CompareMessages.properties: > - replace "Save Diffs" with "Create Patch..." > fixed The fix is not in the patch you sent. > GenerateDiffFileWizard: > - add an info that it's actually a copy of > org.eclipse.team.internal.ccvs.ui.wizards.GenerateDiffFileWizard > added. The javadoc sounds much better now, but it's not 100% true: The GenerateDiffFileWizard does not extend the wizard from Compare. Please review the doc once again. > we organized the code, so that is easier now to comapre it to original > GenerateDiffFileWizard. Thanks, I also did some cosmetic changes in the original wizard from CVS/UI. > - informing that the diff created is not 100% compatible with GNU diff format is > great, but it would be better if you place it also in the exact place where the > difference occurs. Next to the piece of code which makes the difference. > Moreover, I think it could be a global switch for UDF so there is no need to > pass it as an argument. > Added comments. > There is no need to pass it as an argument. You can ommit that argument, and > default patch wil be compatible with Eclipse. But with it as an argument You > have the opportiniuty to create patch in strict unix format. Sounds good to me, but imo providing this information in javadoc for the class would be enough. > - are we able to add time to the file diffs? > No, we aren't. Because comparing process is based on IDocument, I think that > information about time is lost. I see, but this is still a (minor) issue. Why don't you open a bug for it, so we won't loose it. I've also noticed you commented out the piece of code that adds F1 help to the wizard, given that you're adding a new feature it would be great to have an assistance available (help). Please file a bug for it, we can address it later on. I will give it more careful review as soon as I can, but so far so good. Few more fixes and I think we are ready to start thinking about releasing the code to HEAD.
(In reply to comment #42) Thanks for the seperate branch Tomasz. It will be most helpful. We added new very small patch, which addressed those few remarks. It is a patch for the new branch_20090126_LocalDiff. > > - fix copyrights > > fixed > > I changed them a little bit more in some of the classes, double check them if > I haven't missed anything. checked, all seems fine. > > CompareMessages.properties: > > - replace "Save Diffs" with "Create Patch..." > > fixed > > The fix is not in the patch you sent. indeed it wasn't, now added. > > GenerateDiffFileWizard: > > - add an info that it's actually a copy of > > org.eclipse.team.internal.ccvs.ui.wizards.GenerateDiffFileWizard > > added. > > The javadoc sounds much better now, but it's not 100% true: The > GenerateDiffFileWizard does not extend the wizard from Compare. Please review > the doc once again. > > > we organized the code, so that is easier now to comapre it to original > > GenerateDiffFileWizard. > > Thanks, I also did some cosmetic changes in the original wizard from CVS/UI. Thanks. > > - informing that the diff created is not 100% compatible with GNU diff format is > > great, but it would be better if you place it also in the exact place where the > > difference occurs. Next to the piece of code which makes the difference. > > Moreover, I think it could be a global switch for UDF so there is no need to > > pass it as an argument. > > Added comments. > > There is no need to pass it as an argument. You can ommit that argument, and > > default patch wil be compatible with Eclipse. But with it as an argument You > > have the opportiniuty to create patch in strict unix format. > > Sounds good to me, but imo providing this information in javadoc for the class > would be enough. Javadocs are provided, the private method with strictUnixFormat switch is covered by another method, which sets the switch to false, and this method is used further in the code. > > - are we able to add time to the file diffs? > > No, we aren't. Because comparing process is based on IDocument, I think that > > information about time is lost. > > I see, but this is still a (minor) issue. Why don't you open a bug for it, so > we won't loose it. Opened (bug 263439). > I've also noticed you commented out the piece of code that adds F1 help to the > wizard, given that you're adding a new feature it would be great to have an > assistance available (help). Please file a bug for it, we can address it later > on. Opened (bug 263440).
Created attachment 124526 [details] patch_v01_branch_20090126_LocalDiff
The latest patch has been released to the branch with some minor mods. I will do smoke testing this week and see if we can start thinking about merging to HEAD.
Created attachment 126277 [details] patch_v02_branch_changesTree Tomasz, this is a patch with initial implementation of component which is responsible for displaying and choosing files which are going to be included in local patch. The component is displayed on the second page of Local Wizard. It is supposed to simulate the same component from GenerateDiffFileWizard from org.eclipse.team.ui. Our first idea was to subclass classes from team plugin, to create this component, so that it would be exactly the same component that is used in GenerateDiffFileWizard from team. Especially, we wanted to create our own SynchronizeParticipant, to use the whole synchronize mechanism from team plugin. The main advantage of this idea, was that with very small effort (no creating code that is already written), we had exactly what we wanted - a component to synchronize two sets of resources, which is checked, tested and already there. And additinally it looked the same as cvs wizard, which is also our goal. However, we are unable to use this solution, due to cycle dependencies (org.eclipse.team.ui already has org.eclipse.comapre in its dependencies). So far we were unable to find any possible way of using classes from org.eclipse.team.ui, without creating a cycle. We also weren't able to use interfaces and its runtime implementation from bundle team (maybe we are doing something the wrong way), like you suggested us in your email. Tomasz, if we have any ideas how to resolve this issue, please let us now. So far, we decided to create this component from scratch, using pure swt and jface components and a DiffNode structure. It is at a very early stage, the component just display files, the chosen files are not propagated further in the patch creation process. New action has been added to context menu, to section "Compare With", on package explorer, so that it is possible now to compare folders. We also have same prototype code(not published), which we wanted to complete and post on bugzilla after the weekend. Tomasz, if you could just take a look at the code and tell us if we are going in the right direction, it would be great.
I haven't looked at the patch yet, but from you description it sounds more like a fix for bug 265032. Please correct me if I'm wrong.
You are right Tomasz. Exacly it is patch for bug 265030, bug 265032 and bug 257273. Right now I'm sending new version of patch to the 265030 bug, which is the core of those changes.
I was wondering if while working on this enhancement you could also have a look at bug #215360, which I think is quite important. Thanks to all of you guys for the great effort spent on this!
Mauro, thanks for the support ! We will surely take a look at the problem you have pointed out. But from the first view at the bug, I can see it refers to the Create Patch action from the CVS eclipse plugin, which we are not working on now. We are currently working on completly new Create Patch action, called from any open comparison (compare editor). It is not associated with any CVS part of the Eclipse. We'll see if we can do anything. Greetings.
Tomasz, there seems to be a problem on the branch_20090126_LocalDiff. Not all the classes compile, due to the problems with some dependencies from org.eclipse.compare.core plugin. It looks like the code from the branch_20090126_LocalDiff uses some old classes from org.eclipse.compare.core, which are no longer there. Could you please move the changes from head to the branch, so the branch compiles completely ? These are the classes from branch_20090126_LocalDiff I've been having problem with: FileDiffWrapper.java InputPatchPage.java PatchCompareEditorInput.java Patcher.java PatchFileDiffNode.java PatchFileTypedElement.java PatchTargetPage.java PreviewPatchPage2.java RetargetPatchElementDialog.java UnmatchedHunkTypedElement.java WorkspaceFileDiffResult.java WorkspacePatcher.java
(In reply to comment #50) > Mauro, thanks for the support ! > > We will surely take a look at the problem you have pointed out. > > But from the first view at the bug, I can see it refers to the Create Patch > action from the CVS eclipse plugin, which we are not working on now. > > We are currently working on completly new Create Patch action, called from any > open comparison (compare editor). It is not associated with any CVS part of the > Eclipse. > > We'll see if we can do anything. Greetings. Hi Kacper, thanks for the answer. I thought the patch generation code was shared amongst the CVS component and this new compare editor feature, this is why I posted that comment. Mauro.
(In reply to comment #51) > Could you please move the changes from head to the branch, so the branch compiles completely ? Done, the branch has been rebased on 20090318.
Maybe this is obvious, but we need to fix all blocking issues before the code is moved from the branch to HEAD. I think that it is the time and the last chance to do the merge is the next IBuild. This means that we need the fixes ready by Friday. Ready means created and reviewed... Looking at bug 270473 & bug 270474, this seems to be doable. I'm looking forward to your quick response.
Since there are still blocking bugs opened there is no chance to have this completely fixed and released during 3.5. Nevertheless, we will consider this bug to be addressed during 3.6 (it's on our draft dev plan for 3.6, http://wiki.eclipse.org/Workspace3.6).
(In reply to comment #55) > Since there are still blocking bugs opened there is no chance to have this > completely fixed and released during 3.5. Nevertheless, we will consider this > bug to be addressed during 3.6 (it's on our draft dev plan for 3.6, > http://wiki.eclipse.org/Workspace3.6). This is very bad news :-(((
*** Bug 250421 has been marked as a duplicate of this bug. ***
Any love on this one?
We do not plan to address this issue in 3.6 ie it's not a part of our official 3.6 plan[1]. However it is mentioned on our 3.6 draft plan on wiki[2] which means that there is a chance we give it a second try once we're done with all the things from [1] (NST early 2010). [1] http://www.eclipse.org/eclipse/platform-team/team3.6/plan.php [2] http://wiki.eclipse.org/Workspace3.6
Please give it a second try!!! This feature would be extremely useful!
Yes, It would be really helpful, for example, I'm working on a website, and after downloading the site to my local machine through ssh and fixing some bugs I have to report the fixed bug accompanying the report with a .patch, I know it would work if I just commit these changes in a svn repo, but I just want to create a .patch without commiting.
*** Bug 282126 has been marked as a duplicate of this bug. ***
I needed to create a patch from a diff between two files. +1
Any news on this? Could this be targeted for 3.8?
(In reply to comment #64) > [...] Could this be targeted for 3.8? No, this is not part of our 3.8 plan[1]; notice there is no Target Milestone set. Unless someone provides a patch there very little chance someone from our team will start working on this. It doesn't mean I'm not willing to help if we find a volunteer, I always do. We simply don't have the manpower to address it at this time. [1] http://wiki.eclipse.org/Workspace3.8
Dear Tomasz, first of all thanks for your feedback. I had seen the 3.8 plan, however I always find myself "not in time" for this kind of requests. I mean, periodically I do a check on the open bugs I would be interested in and, if a long time has passed, I "ping" them in some way. I was told this is the only way for the community to express interest in bug resolutions. However, almost every time I get answered that the fix is not in the next-version plan. I'm sad to know that the 3.8 plan is already frozen... we're still in October and 3.8 is going to be released in next June... On the other hand, this bug is 7 years (!!!) old, has 30 votes on it and a patch for a partial work that was left at a certain point. How is that it was not considered for 3.6, 3.7 and 3.8 plans, while it was previously targeted for 3.5? Not to say that this feature is so valuable that it's a real pity that it wasn't implemented in the origins...
A big agreement from the guy who stated the bug years ago.
I just spent quite a while searching the web for this feature (i.e. create a patch from "compare with" > "each other" result) and finally found this bug - only to discover that ist is more or less implemented but still not available. Would it not be possible to at least publish the changes as a plugin/dropin for the current Eclipse versions on one of the involved developer's websites, so that the work being done is not lost? +1 for this to be included in Eclipse 4.3!
(In reply to comment #68) > Would it not be possible to at least publish the changes as a plugin/dropin > for the current Eclipse versions on one of the involved developer's > websites, [...] imo it doesn't make much sense to create a patch for an uncompleted, potentially broken feature. > so that the work being done is not lost? The work is not lost, all the patches are still here. Having said that, I think it'd be better to start from scratch.
You need to push updated patches to Gerrit. Unfortunately I think it is too late for Kepler now.
