Community
Participate
Working Groups
branch_20090126_LocalDiff Couple of things that would be good to have done before merging the code from branch_20090126_LocalDiff to HEAD: 1. UnifiedDiffFormater: create a constant that indicates that we're not using strict unix format when creating local diffs 2. UnifiedDiffFormater: remove the last argument from UDF's constructor, I guess passing docs and paths in a particular order could do the job here, ie: when in "left-to-right" mode pass the left doc as first, when in "right-to-left" pass the right one first 3. UnifiedDiffFormater: instead of passing the whole DocumentMerger you could just provide a list of all changes when creating the UDF 4. GenerateDiffFileWizard: the isRightToLeft() method is not clear to me, it checks whether the LEFT_OPTION is selected which could indicate that the patch contain changes contributed from left to right... but I may be wrong here. Why don't put a short javadoc there to make things crystal clear 5. TextMergeViewer: removing extra whitespaces was a bad idea, my fault. Please revert that change, this will make merging much easier 6. In case we would like to use patches generated by CVS to test the ones generated locally you would need to ignore other markers that could be found in such a patch eg. PatchReader.MULTIPROJECTPATCH_HEADER
Created attachment 128554 [details] Patch v1 About Your requests, Tomasz: 1. Done. 2. and 3. We changed formatter constructor, to: public UnifiedDiffFormatter(DocumentMerger merger, String oldPath, String newPath, boolean rightToLeft) We're passing the whole merger, because now we take leftDoc, rightDoc and allDiffs from it, so it's reasonable to pass one merger instead of three other parameters. We also left rightToLeft parameter, because it is essential to interpret diffs from merger in a proper way. The merger, (taken from comapre editor), produceed diffs using left and right side of comaprison from editor. So if we change the contributing side using wizard, we have to somhow notify about it the formatter, so it may process the diffs in right order. 4. We explaind in javadoc what this method does exactly. 5. Done. 6. It was done already. Method getContentFromLines(..) is responsible for that.
Created attachment 128555 [details] Tests_Patch v1 Due to Patch v1 changes, minor change in tests wes required.
Krzysztof I've just updated the branch with all the changes from HEAD. The problem is that it made the patch not appliable (conflicts in TextMergeViewer), could you update it please?
Tomasz, the only changes made to TextMergeViewer in patch to this bug, were corrected whitespaces (nr 5 in the bug description). Since you have already moved all the changes from head to branch, we don't need these changes of whitspaces anymore, is that correct ?
(In reply to comment #4) > Tomasz, the only changes made to TextMergeViewer in patch to this bug, were > corrected whitespaces (nr 5 in the bug description). This is not quite true, in the patch I can see that you commented out contributeChangeEncodingAction(MergeSourceViewer) method body and did the same for getTextEditorAdapter(). Could you explain what's the meaning of this? Or did this happen by accident? > Since you have already moved all the changes from head to branch, we don't need > these changes of whitspaces anymore, is that correct ? No, if you had looked at the code you would have seen that I moved only logical changes.
Created attachment 129604 [details] Patch v2 Patch is now appliable to current version of branch LocalDiff. > This is not quite true, in the patch I can see that you commented out > contributeChangeEncodingAction(MergeSourceViewer) method body and did the same > for getTextEditorAdapter(). Could you explain what's the meaning of this? Or > did this happen by accident? That's our fault, it got there by accident, sorry.
Patch v2 is not applyable i.e. there are conflicts in TMV and GDFW. Guys, could you update it? Thanks.
(In reply to comment #7) > Patch v2 is not applyable i.e. there are conflicts in TMV and GDFW. Guys, could > you update it? Thanks. > Tomasz, I've just checked the Patch v2 on the verion straight from the branch (no other changes made by other patches) and it applied without any conflicts. Could you please check if your project have any other changes, before applying Patch v2 ? Thank you.
My fault, missed the fact that Szymon made some changes on the branch in the meantime. The patch applies smoothly now, but still I've got two questions: 1. Didn't we agree that STRICT_UNIX_FORMAT constant should be documented? This way, you could remove the info about how the current implementation differs from the GNU one. 2. You didn't mention that you commented out a line in TMV. To be more precise, the one that set Command ID the Find action. This is unrelated to this bug, you should touch it.
Created attachment 130252 [details] Patch v3 > 1. Didn't we agree that STRICT_UNIX_FORMAT constant should be documented? This > way, you could remove the info about how the current implementation differs > from the GNU one. all comments on implementation differences moved to javadoc of STRICT_UNIX_FORMAT constant. > 2. You didn't mention that you commented out a line in TMV. To be more precise, > the one that set Command ID the Find action. This is unrelated to this bug, you > should touch it. Sorry, our fault, it shouldn't be commented out, fixed.
Both patches released to the branch. I removed few more whitespace differences from TMV.
