Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 352576 - Compare editor doesn't seem to recalculate the diff after copying change from right to left and saving
Summary: Compare editor doesn't seem to recalculate the diff after copying change from...
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: 1.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 1.1-M3   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 353537 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-07-20 09:29 EDT by Remy Suen CLA
Modified: 2011-08-28 19:44 EDT (History)
3 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Suen CLA 2011-07-20 09:29:30 EDT 
I compare a change in the 'Git Staging' view and after I undo some changes by copying a change from right to left and save, no diff recalculation is performed so the compare editor still think there is a change around those lines in the file.

The CVS compare doesn't seem to have this problem.
Comment 1 Tomasz Zarna CLA 2011-07-20 11:21:06 EDT 
In CVS, but it's also true for local files, when save is made SavebaleHelper calls doSaveModel[1] for each saveable[2] in the editor. In most cases, it's a single iteration because the Compare Editor has been opened for SaveableCompareEditorInput[3], which has only left side can be edited (saved).

I don't know what kind of input EGit uses for their Compare Editor. I would start from finding that out.

[1] org.eclipse.ui.internal.SaveableHelper.doSaveModel(Saveable, IProgressMonitor, IShellProvider, boolean)
[2] org.eclipse.ui.Saveable
[3] org.eclipse.team.ui.synchronize.SaveableCompareEditorInput
Comment 2 Remy Suen CLA 2011-07-20 12:55:15 EDT 
Thank you for your research, Tomasz!

The problem seems to be because GitCompareFileRevisionEditorInput does not have an implementation for the abstract fireInputChange() method. If I force the compare result to fire a change like what it's doing in CompareFileRevisionEditorInput then it works. Tomasz, is this the expected pattern? That we subclass DiffNode and then provide a public version of fireChange() to invoke within fireInputChange()?

You can find our implementation of prepareCompareInput(IProgressMonitor) below:
http://egit.eclipse.org/w/?p=egit.git;a=blob;f=org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/GitCompareFileRevisionEditorInput.java;h=f3c23097ad167b5d8ee6e03f5d66d972a97144b0;hb=HEAD
Comment 3 Tomasz Zarna CLA 2011-07-25 07:57:30 EDT 
(In reply to comment #2)
> Tomasz, is this the expected
> pattern? That we subclass DiffNode and then provide a public version of
> fireChange() to invoke within fireInputChange()?

From what I can see looking at implementations of SaveableCompareEditorInput in Team, it seems to be right approach. All those inputs have a meaningful implementation of the #fireInputChange method. However, if you take a look at comment for org.eclipse.team.ui.synchronize.SyncInfoCompareInput.MyDiffNode you will read "This class exists so that we can force the text merge viewers to update by calling #fireChange when we save the compare input to disk. The side effect is that the compare viewers will be updated to reflect the new changes that have been made. Compare doesn't do this by default.". So, to me, it sounds like an optional thing. Not sure if this answers your question.
Comment 4 Remy Suen CLA 2011-08-25 17:27:51 EDT 
*** Bug 353537 has been marked as a duplicate of this bug. ***
Comment 5 Remy Suen CLA 2011-08-26 17:50:32 EDT 
A change set has been attached to Gerrit for review.
http://egit.eclipse.org/r/#change,4073