Community
Participate
Working Groups
Implementation of a compare editor input that supports editing on both sides introduced in bug 193324 has unintentionally introduced several issues exposed by bug 262557 (see bug 262557 comment 7). The issues are: 1. the worker that computes the AST is canceled by Compare and hence the exception ==> the StructureCreate has to be prepared that an OperationCanceledException is thrown by anyone that check the monitor 2. createStructure is called twice for both sides on save: - once in UI thread via: org.eclipse.team.ui.mapping.SaveableComparison.doSave(IProgressMonitor) - once via org.eclipse.compare.internal.WorkerJob 3. in SaveableHelper.saveModels only left side is saved with Saveable.doSave logic ==> when SaveableHelper loop moves to the second saveable (right side) it turns out that the model is not dirty thus saving cane be is skipped[1]. This situation is caused by the way saving the left side is done[2] ie flushing viewers results in saving the content of the right side as well[3]. 4. TextMergeViewer.flush is called twice for the right side contributor - firstly, when flushing content of TextMergeViewer[3] in order to save left side saveable (first iteration in SaveableHelper[1]) - secondly, in reaction[4] to content changed event triggered by the first call. Each LocalResourceSaveableComparison listens for content changes[5] and when such occurs save is performed, but in this particular case the change is made when saving the left side. 1 and 2 are taken from bug 262557 comment 7, for more info see also bug 262557 comment 9. Among the things mentioned above, there is also bug 256805 addressing general polishing of SaveablesCompareEditorInput. [1] SaveableHelper.saveModels [2] LocalResourceSaveableComparison.performSave(IProgressMonitor) [3] TextMergeViewer/ContentMergeViewer.flushContent(Object, IProgressMonitor) [4] LocalResourceSaveableComparison.fireInputChange() [5] LocalResourceSaveableComparison.initializeContentChangeListeners().new IContentChangeListener() {...}.contentChanged(IContentChangeNotifier)
5. SaveableComparison.doSave sets dirty indicator on CompareEditorInput to false, which is wrong as there still may be other Saveables from the same Compare Editor waiting to be saved. LocalResourceSaveableComparison does the same thing in performSave, sets dirty flag on the editor input to false.
5. is another reason for 3. to happen.
We will investigate these issues during 3.6, potentially breaking up this bug into smaller pieces and fixing them one by one. Hopefully we will manage to address all the regressions listed here and others revealed during the investigation or reported by the community. Some of the fixes may be even considered for bakckporting to 3.5.x stream.
SaveablesCompareEditorInput holds 2 Saveable objects, when they are both dirty and the editor is being saved SaveableHelper calls doSave for each of them. The problem is when saving the first saveable, all compare editor viewers are flushed saving both sides of the comparison. This looks like a direct consequence of how IFlushable is designed and a reason for 3. and 4. to happen (comment 0). The good news is that it's being addressed by bug 273450 (see bug 273450, comment 6, last bullet).
Bug 312893 is another issue which showed up while working on bug 273450.
Regressions number 3, 4 and 5 will be addressed at bug 273450. At least, the current shape of the patch there not only fixes the wrong labels in the Save dialog but also fixes the former issues. However, given the possible impact of releasing the patch at this stage I would hold off committing it. Thus I'm removing the target milestone on this bug to better match the reality. Hopefully we will find more time for it in 3.7 (the item has been added to Workspace 3.7 draft plan[1]). The other two regressions described in comment 0 (1 and 2) are still valid and need to be investigated. [1] http://wiki.eclipse.org/Workspace3.7
Some issues described here were addressed by bug 273450. We will continue working on 1) and 2) from comment 0 during 3.8.
(In reply to comment #7) > We will continue working on 1) and 2) from comment 0 during 3.8. 1) is bug 358911 2) is bug 358909 From now on this bug serves only as a container for regressions introduced by SaveablesCompareEditorInput. They are represented as blocking bugs. As soon as all of them are fixed, this bug can be closed.
Another regression: bug 347560
All underlying regressions are targeted to M6 the latest so hopefully we'll be done with all by M6.
All remaining regressions have been postponed, moving master bug to m7.
Moving to 4.3.
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. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. 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. -- The automated Eclipse Genie.
Maybe this should be reopened?
