Undo cursor move
AbandonedPublic

Authored by mickaelbo on Mar 25 2020, 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.Mar 25 2020, 4:07 PM
Restricted Application added a project: Kate. · View Herald TranscriptMar 25 2020, 4:07 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
mickaelbo requested review of this revision.Mar 25 2020, 4:07 PM
ngraham added a reviewer: VDG.Mar 25 2020, 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.Mar 25 2020, 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?

  • "There is already a way to do this on KTextEditor". The existing code seems to update the position stack only after editing a document but I will give it a try, thanks . It is a feature I needed for a long time and according to the Bug 409940, I'm not the only one, I just never took the time to dig into it. The "delete" and "page end" keys are too close on my keyboard...

BTW, is the KDE Bugtracking System linked to phabricator in any way ? This mailing list is very reactive and I feel like the bug tracker could benefit from this. This discussion is a perfect example, the bug 409940 received no answer until recently but this review request received very good answers.

Patches naturally get more attention than feature requests in bug reports, because even if in the feature request everyone agrees it's a nice idea you still have to find somebody to actually do it -- which is often not the case ;) and then it's just wasted time to discuss it in the first place.

mickaelbo abandoned this revision.Apr 4 2020, 7:29 PM

This feature is already implemented in kwrite. The default undo/redo shortcuts in Kate are Ctrl+E / Ctrl+Shift+E.