Bug 287421 - 'Copy Current Change' should not go to next difference
Summary: 'Copy Current Change' should not go to next difference
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Compare (show other bugs)
Version: 3.6   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M5   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard:
Keywords: accessibility
Depends on:
Blocks:
 
Reported: 2009-08-24 06:32 EDT by Markus Keller CLA
Modified: 2009-12-23 08:26 EST (History)
1 user (show)

See Also:


Attachments
Fix v01 (1.05 KB, patch)
2009-12-15 10:13 EST, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (8.42 KB, application/octet-stream)
2009-12-15 10:13 EST, Tomasz Zarna CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2009-08-24 06:32:17 EDT
I20090818-0800

When I manually inspect each incoming change in a compare editor for a file with conflicts, I always want to verify that the updated code is correct.

Currently, this is very cumbersome, because the compare editor automatically jumps to the next change after I clicked the 'Copy Current Change' button in the column between the two text editors. The command should just do what its name says, namely copy the change.

The added smartness to automatically jump to the next change could be left as an option, but it should be off by default.

The current smartness also has a bug which makes this even worse: It jumps to the next change based on the cursor position. This is completely wrong when I just used the mouse to scroll down and then clicked the copy button in the middle column. The search for the next change should start at the last edit position and not at the old caret location.
Comment 1 Markus Keller CLA 2009-11-11 07:11:22 EST
Loss of context is evil.
Comment 2 Tomasz Zarna CLA 2009-11-20 07:30:15 EST
(In reply to comment #0)
> Currently, this is very cumbersome, because the compare editor automatically
> jumps to the next change after I clicked the 'Copy Current Change' button in
> the column between the two text editors. 

...unless the change you copied is a conflicting change. I've just checked it in the code - I've never used that button myself. It's something Michael added on bug  202944. In addition what you just said, when doing the same thing (ie copying a change) with toolbar buttons the editor won't jump to the next change. You're absolutely right, this is cumbersome.

> The added smartness to automatically jump to the next change could be left as
> an option, but it should be off by default.

Should the option affect "Copy Current Change" toolbar buttons as well? Should the editor jump to the next diff when the option is set to "jump" even if we copied the change using a toolbar button? This would make it consistent I think.
 
> The current smartness also has a bug which makes this even worse: It jumps to
> the next change based on the cursor position. This is completely wrong when I
> just used the mouse to scroll down and then clicked the copy button in the
> middle column. The search for the next change should start at the last edit
> position and not at the old caret location.

If I understood you correctly, "the last edit position" in this scenario actually stands for the position of the change you've just copied. Should that be configurable as well? I guess not.
Comment 3 Markus Keller CLA 2009-11-30 12:25:30 EST
> Should the option affect "Copy Current Change" toolbar buttons as well?
I'd say yes. To be honest, I don't even need an option. I just said that to improve the possibility that this gets fixed ;-). You could also just remove the jumping and only add an option if somebody actually speaks up and wants the old behavior back.

> If I understood you correctly, "the last edit position" in this scenario
> actually stands for the position of the change you've just copied.
Yes. I don't know how the search is implemented, but you probably have to set the caret in both (left & right) text viewers to make sure it continues from there in all cases.

> Should that be configurable as well? I guess not.
No, I don't think anyone wants that.
Comment 4 Tomasz Zarna CLA 2009-12-15 10:13:21 EST
Created attachment 154483 [details]
Fix v01

Without the option to trigger the jumping fixing it is pretty straightforward. However, I do not want to have it on my conscience so I guess I'll add the option.
Comment 5 Tomasz Zarna CLA 2009-12-15 10:13:26 EST
Created attachment 154484 [details]
mylyn/context/zip
Comment 6 Tomasz Zarna CLA 2009-12-23 06:08:41 EST
On second thought, I agree with Markus to leave out the option. I'm pretty sure we can live without it and as Markus suggested I could easily add it if somebody asks for it.

Fix v01 applied to HEAD, available in builds >N20091217-2000.