Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abort repeat with evil-undo #1405

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

aaronjensen
Copy link
Contributor

Without this, . will repeat undo, at least when using undo-tree.

@aaronjensen
Copy link
Contributor Author

Actually, the same needs to apply to evil-redo

@aaronjensen aaronjensen force-pushed the evil-undo-abort-repeat branch from 2099377 to 4aa3c5a Compare January 10, 2021 18:01
@aaronjensen
Copy link
Contributor Author

Updated

@aaronjensen
Copy link
Contributor Author

I should mention, that, at least on my machine, some very bad things (like emacs 100% cpu spins) happen in some situations without this change when undoing/redoing and repeating things.

@xsrvmy
Copy link

xsrvmy commented Mar 4, 2021

Ran into this on spacemacs. Someone please merge.

@tomdl89
Copy link
Member

tomdl89 commented Mar 5, 2021

I'm not the maintainer (he's a bit busy right now) but I do have merge permissions. Happy to do so, but feel I need some explanation first:

  • Why 'abort rather than 'ignore or nil?
  • Is this a recent regression? How has it not come up before?
  • It's not a problem for me using undo-fu - is it specific to undo-tree, and if so, why?
  • Could we add a test for this?

@aaronjensen
Copy link
Contributor Author

  • Why 'abort rather than 'ignore or nil?

The :repeat property of both undo and redo is 'abort. These were the commands u and and C-r were previously bound to

Furthermore, from evil-repeat.el:

;; Not all commands are recorded. There are several commands that are
;; completely ignored and other commands that even abort the currently
;; active recording, e.g., commands that change the current buffer.

Undoing changes the current buffer. It would be odd if one was in the middle of typing in a buffer in insert mode, issued an undo with something like c-/, then kept typing if a repeat would insert both the pre-undo and the post-undo typed characters.

Is this a recent regression? How has it not come up before?

Yes. It was introduced when the configurable undo system was introduced, i.e. when #1360 was merged.

It's not a problem for me using undo-fu - is it specific to undo-tree, and if so, why?

When using undo-tree, evil-repeat sees the command as evil-undo which has a default repeat of t. When using undo-fu, it sees it as undo-fu-only-redo. I do not know why this is, the workings of this-command are a bit of a mystery to me.

Could we add a test for this?

Someone possibly could, but I don't have the time to get into it. Doing the research to answer your other questions was more time than I had to spend on this at the moment, unfortunately.

@aaronjensen
Copy link
Contributor Author

By the way, as an undo-fu user, you may want to try out binding C-/ to undo-fu-only-undo then typing in insert mode:

aaa C-/ bbb

Then leave insert mode and press .. See if aaabbb is what you really expect to happen there.

@tomdl89
Copy link
Member

tomdl89 commented Mar 5, 2021

@aaronjensen thanks for taking the time to explain. I had a feeling it would have been a regression from the recent introduction of the configurable undo system. I'm satisfied this should be merged. Please keep an eye out for any unexpected behaviour.

@tomdl89 tomdl89 merged commit f5ab7ff into emacs-evil:master Mar 5, 2021
@aaronjensen aaronjensen deleted the evil-undo-abort-repeat branch March 5, 2021 13:42
@aaronjensen
Copy link
Contributor Author

Sure thing, thank you for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants