Undo cursor move
Needs ReviewPublic

Authored by mickaelbo on Wed, Mar 25, 4:07 PM.

Details

Reviewers
None
Group Reviewers
Kate
VDG
Summary

Undo/redo cursor moves in a view.
This implements Bug 409940 from the KDE bug tracker.
A cursor move must be at least 10 lines long to be recorded. Should we allow to customize this hardcoded value ?
For now, the shortcut to undo a move is shift + mouse Qt::BackButton, redo is shift + mouse Qt::ForwardButton. Should we add an action in another commit to trigger undo/redo from a user defined shortcut?

Test Plan

Open a document with more than 10 lines. The default cursor position is 0.
Click inside line 11.
Trigger the undo action, the cursor goes back to line 0.
Trigger the redo action, the cursor goes back to line 11.
Click inside line 5 and click undo (the history is not affected).
Trigger the undo action, the cursor goes back to line 0.

Diff Detail

Repository
R40 Kate
Lint
Lint Skipped
Unit
Unit Tests Skipped
mickaelbo created this revision.Wed, Mar 25, 4:07 PM
Restricted Application added a project: Kate. · View Herald TranscriptWed, Mar 25, 4:07 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
mickaelbo requested review of this revision.Wed, Mar 25, 4:07 PM
ngraham added a reviewer: VDG.Wed, Mar 25, 7:28 PM
ngraham added a subscriber: ngraham.

Interesting idea. I don't know if I've ever used another editor with this feature, but I thought I ought to give it a try before judging. However I'm not able to get it working consistently. Sometimes moving the cursor more than 10 lines adds an item to the undo stack, sometimes it doesn't.

Thanks for the feedback, I managed to reproduce this using "go to definition" and will investigate.
I only tried to manage long jumps (page end, mouse click, etc.): pressing 77 times the up arrow was not meant to add any item to the undo stack but reading the code, I realized that it actually adds 7 items of 11 lines jumps. This is probably not what we want.
It is possible to discard small jumps by comparing the new position to the last *view* position instead of the last *undo stack* position. Any suggestion ?

I am currently not convinced for several reasons:

  1. There is already a way to do this on KTextEditor level, see: https://github.com/KDE/ktexteditor/blob/master/src/document/katedocument.h#L406
  2. I believe this should be a plugin, since the current implementation makes the current code more complex, and it's a feature we have not needed for > 15 years.

As also commented on the bug report, maybe the existing solution can be improved?

brauch added a subscriber: brauch.Wed, Mar 25, 11:18 PM

The 10 line limit also seems kind of arbitrary to me. KDevelop has a similar navigation feature and navigates by "contexts". A similar concept exists in KTextEditor in terms of the folding ranges; maybe that could be used to classify "meaningful" cursor moves to navigate between?