emacs-evil / evil

The extensible vi layer for Emacs.

GNU General Public License v3.0

3.35k stars 283 forks source link

`evil-redo` won't redo `evil-undo`, unless the latter was called in "normal mode" #1883

Closed Hi-Angel closed 5 months ago

Hi-Angel commented 5 months ago

Issue type

Bug report

Environment

Emacs version: GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.41, cairo version 1.18.0) of 2024-02-21 Operating System: Archlinux Evil version: 1.15.0 and latest Evil as of commit 0d40434 Evil installation type: Melpa Graphical/Terminal: Graphical Tested in a make emacs session: Yes

Reproduction steps

In Evil repo: make emacs
Press n to cancel tests

Evaluate with M-: the following code:

(progn
 (evil-set-undo-system 'undo-redo)
 (bind-key "C-/" 'evil-undo))

Type in the buffer test
While still being in "insert mode" press C-/ to undo (the text should disappear)
Switch to "normal mode" and redo with C-r

Expected behavior

The test text should appear

Actual behavior

Nothing happens

Further notes

This is a very old annoying problem, but I never reported it because didn't have minimal steps. That is until now.

Hi-Angel commented 5 months ago

This may be a bug in Emacs, because the problem is reproduced if I bind C-/ and C-r (the latter in "normal mode") to undo-only and undo-redo directly.

However, I'd need some help in reducing this to a testcase that detaches the problem from Evil.

Hi-Angel commented 5 months ago

Idk if relevant, but I found out that reproducing the bug leaves a marker in no buffer entry in the buffer-undo-list list.

Evil somehow causes it, that won't appear in emacs -Q. As a matter of fact, Evil leaves around a lot of marker entries while following the steps (albeit only one of them is a "no buffer"). That does not happen with emacs -Q which has only one marker.

tomdl89 commented 5 months ago

Thanks @Hi-Angel - I can reproduce. If evil-want-fine-undo is set to t then I can't, so it does indeed look like evil is clobbering the undo info for what was typed, most likely as part of its evil-start-undo-step grouping. Will investigate when I have some time. The proliferation of marker entries looks like it's worth investigating too.

Hi-Angel commented 5 months ago

Thank you @tomdl89. Makes me wonder if that's somehow connected to the infamous undo-tree "unrecognized entry in undo list" error.

tomdl89 commented 5 months ago

I have evil-want-fine-undo set to t and I think that is the best solution to this problem. But I have also made a commit which does provide a fix, of sorts, without doing that: https://github.com/tomdl89/evil/tree/insert-state-undo-in-normal-state. @Hi-Angel can you check out the branch and let me know if it works as expected? It drops you back to normal state, undoes the (entire) insert, and then puts you back into insert state. A jumping-off point, at least, hopefully.

Hi-Angel commented 5 months ago

Yay, cool, that works! Thanks!