Community
Participate
Working Groups
Using latest from HEAD: 1. Select a method of a Java source file with recent editing. 2. Compare with Element from Local History 3. Select a version -> Left pane show the whole current file, right pane correctly focuses to the selected method. 4. Make a small edit in the left pane and save -> Left pane, too, focuses on the selected method. I assume the behavior after 3 is a bug.
Created attachment 149072 [details] propose patch This seems to be related to conflicting calls to SourceViewer.setDocument(..). 2 paths to this method are originating from TextMergeViewer.updateContent(..): via fLeftContributor.setDocument(fLeft, ...) -> this call correctly sets a ProjectionDocument that knows about the focus to the method's source range via configureSourceViewer(fLeft.getSourceViewer(), ...) -> this call is agnostic of source ranges and sets a plain Document By the order of these statements the unpositioned plain Document overwrites the positioned ProjectionDocument. After editing and saving, the same method updateContents is executed again, but now, isConfigured is true and the bogus second call is skipped, that's why after save the focus is correct. Thus, my proposed patch simply swaps statement order in said method. This fixed the problem for me.
(In reply to comment #0) > I assume the behavior after 3 is a bug. You're right, it is a bug. (In reply to comment #1) > Thus, my proposed patch simply swaps statement order in said method. Reviewing...
(In reply to comment #1) > This fixed the problem for me. It does fix this particular issue indeed, however it breaks Content Assist (Ctrl+Space) and Quick Fix (Ctrl+1) at the same time. We will need to prepare a safer solution.
(In reply to comment #3) > It does fix this particular issue indeed, however it breaks Content Assist > (Ctrl+Space) and Quick Fix (Ctrl+1) at the same time. We will need to prepare a > safer solution. Good point :( It looks like some circular dependency between those blocks of statements, each needs to be called after the other? I inspected whether the JavaSourceViewerConfiguration was correctly installed: yes. Also editor.makeActions() seems to yield the same results in both versions. Do you have a quick guess, where in the code I could find the place where content assist / quickfix are installed?
Thanks for your time Stephan, I will take a look at it next week. My task list scheduled for this week is full, sorry.
Created attachment 149655 [details] improved patch I gave it another shot. What's funny: it seems to be another issue of statement ordering: TextMergeViewer.configureSourceViewer(..) first configured the source viewer and only then marked it as editable. Unfortunately, configuring the viewer while it was still marked as not-editable caused the ContentAssistAction and others to NOT install themselves. Again a simple swap of statements solved the issue for me. I hope I didn't break anything else, this time ;-)
Good job Stephan! I have done some additional testing and so far so good. I did filed bug 293144 and bug 293153, but these are not caused by your fix. I will give it another try next week and if satisfied with the results I think I could release in 3.5M4. It's a little bit to late for M3. How does it sound? Thanks!
(In reply to comment #7) > Good job Stephan! I have done some additional testing and so far so good. I did > filed bug 293144 and bug 293153, but these are not caused by your fix. Another one while we're at it: bug 293674 > I will > give it another try next week and if satisfied with the results I think I could > release in 3.5M4. It's a little bit to late for M3. How does it sound? sounds fair. > Thanks! welcome :)
*** Bug 293663 has been marked as a duplicate of this bug. ***
The latest patch has been released to HEAD, available in builds >N20091117-2000. Credit goes to Stephan, so I marked the patch with "iplog+" flag and added you to the "Contributors" section in the class. The fix may seem trivial, but I'm sure debugging and looking for the root cause was a hard nut to crack. Thanks again!
Please revert for M4 or provide a better fix as this one results in corrupted files, see bug 296580.
(In reply to comment #11) > Please revert for M4 or provide a better fix as this one results in corrupted > files, see bug 296580. I can reproduce and I'm currently trying to find the root cause. I observed that applying one diff fails to update the diff-regions in the modified editor, i.e., even before applying the second diff you can see that it will insert into a wrong position. Perhaps this is also related to bug 293674?
I've proposed a patch at bug 296580. Closing because I think that this bug's patch should not be reverted or changed.
>Closing because I think that this bug's patch should not be reverted >or changed. Actually, I don't care how but just that bug 296580 gets fixed for M4 and if reverting is the only choice then it will have to be reverted, but let's focus on the patch you attached to bug 296580 for now.
Verified in I20091208-0100, but filed bug 297316. There is also bug 293674 opened by Stephan in comment 8.
