Alexey-T / CudaText

Cross-platform text editor, written in Free Pascal
Mozilla Public License 2.0
2.39k stars 166 forks source link

Previous undos unexpectdly get merged #5516

Closed pintassilgo closed 3 weeks ago

pintassilgo commented 1 month ago

I was removing the unneeded entries from my user.json after the changing in default values, then I noticed this issue.

  1. Place the caret at the end of a random line in the middle of user.json.
  2. Shift+arrow up, then Delete, to select and remove that line.
  3. Repeat steps 1 and 2 in other line.
  4. Press Ctrl+Z (undo). The last line you removed is reinserted, OK.
  5. Press Ctrl+Z again. The first line you removed is reinserted, OK.
  6. Press Ctrl+Shift+Z (redo).

Expected result:

The first line you removed should be removed once again. To remove the other one, you would need to press Ctrl+Shift+Z once more.

Actual result:

Both lines were merged to a single undo/redo entry, so both were removed when you first pressed Ctrl+Shift+Z.

Bonus: if you press Ctrl+Z again, both lines are reinserted but only one is selected, making user think only that one was reinserted. You need to look carefully to notice both were reinserted.

Alexey-T commented 1 month ago

That part with undo/redo merging steps is difficult, I wont promise the fix. Will see.

Alexey-T commented 3 weeks ago

Reproduced on this text, when caret is at the end of line 3

dddddddddd
ddddddddddd
dddddddddddddd
ddddddddddddddd

we must move to shorter line when doing Shift+Up.

Alexey-T commented 3 weeks ago

We have variable PauseForMakingGroup=1500 (in msec). if pause between actions is smaller, actions are later undone as a whole. I reduced this pause to 400. now if I press Undo / Undo with pause at least 400, Redo will revert them as separate actions, as you want. this pause is not the global option yet. not sure it's needed.

Alexey-T commented 3 weeks ago

beta linux-qt5, rename to *.xz. cudatext.xz.zip

pintassilgo commented 3 weeks ago

Thanks.

I think this can be a OK solution, but an important improvement should be made: undo/redo must NOT be affected by this var.

Example:

I'm editing user.json. I delete some lines one by one, like line 5, line 10, line 15, line 20 and line 25. Then, if I press Ctrl+Z multiple times, I can reinsert these lines one by one, good.

But if I HOLD Ctrl+Z to run many undos fast, my undo history becomes a complete mess, with all the very separated lines merged to a single redo action. It shouldn't be that way. Having the proper undo/redo history intact can be important sometimes.

So: there should be an exception so that running consecutive undo/redo shouldn't be able to merge multiple undo items into one.

pintassilgo commented 3 weeks ago

Other real scenario affected if you don't fix what I described in previous comment:

I'm writing a long text or code, working hours on it, with many typing and deleting. At some point, I feel I'm not satisfied with current content and decide to return to a previous state and continue to write from there. Instead of holding Ctrl and tapping Z many times until my finger hurts, I simply hold Ctrl+Z for fast multiple undos, looking at when it's near the point I will choose to restart.

When I release the keys, I pause to look carefully, then I decide I don't want to stop there, but a bit later. So now I need to run some redo's (Ctrl+Shift+Z). But now my undo history was already lost, because while I was holding Ctrl+Z my many undo entries were merged. So I no longer have the middle states to redo.

You don't see this issue in any other editor, it's clearly a bug.

Alexey-T commented 3 weeks ago

I agree. I will try to fix that.

Alexey-T commented 3 weeks ago

next attempt, beta.

cudatext.xz.zip

pintassilgo commented 3 weeks ago

Looks good, thanks.