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

Add option to switch to undo-fu #1229

Closed
wants to merge 1 commit into from
Closed

Conversation

TheBB
Copy link
Member

@TheBB TheBB commented Dec 22, 2019

Just a draft branch that I'll be playing around with for a while. Opening this as a heads up.

Hopefully this can end up closing #1074, #418 and #522.

@edkolev
Copy link
Member

edkolev commented Dec 22, 2019

Nice initiative!

A couple of notes:

  • let me know if you need need me to dogfood this feature - I'd be happy to test
  • supporting both undo-tree and undo-fu might be a stretch (in terms of implementation cost, and maintenance cost). I suggest supporting just one one them, i.e. no need for evil-undo-provider

@TheBB
Copy link
Member Author

TheBB commented Dec 22, 2019

Yeah I'd love for others to test.

Wasn't sure yet whether I wanted to rip out undo-tree completely or what. If testing goes well we can take that into account. One problem is that undo-fu has very strict Emacs version requirement. I'll check with the author if that's just because it's untested on older versions.

@TheBB
Copy link
Member Author

TheBB commented Dec 22, 2019

Also goto-chg depends on undo-tree but I think it's a soft dependency.

@TheBB TheBB force-pushed the undo-fu branch 2 times, most recently from f893133 to 54278f2 Compare December 30, 2019 10:19
@TheBB
Copy link
Member Author

TheBB commented Jan 8, 2020

There's been a new release to undo-tree, which might mitigate some of the problems people have been seeing. I'll leave this PR in limbo for a while.

@jixiuf
Copy link
Contributor

jixiuf commented Jan 24, 2020

I would got on emacs28

Symbol's function definition is void: undo-tree-save-history-from-hook

I think this hooks should be added after function is defined

(add-hook 'write-file-functions #'undo-tree-save-history-from-hook)
(add-hook 'kill-buffer-hook #'undo-tree-save-history-from-hook)
(add-hook 'find-file-hook #'undo-tree-load-history-from-hook)

can not reach the author of undo-tree

@aaronjensen
Copy link
Contributor

@jixiuf he responds to email, which you can find on his website: http://www.dr-qubit.org/undo-tree.html

That said, I think he is unlikely to make any changes for Emacs28 unless they are totally innocuous for current emacs versions.

The error you're seeing is surprising... they're changing the semantics of #'symbol or add-hook is doing validation now...

@TheBB
Copy link
Member Author

TheBB commented Jan 24, 2020 via email

@TheBB
Copy link
Member Author

TheBB commented Jan 27, 2020

Undo-tree 0.7.3 is now out and it should have fixed the above mentioned issue.

@ideasman42
Copy link

ideasman42 commented Feb 8, 2020

Any update on this?

Even in the case undo-tree is has been fixed, it's a fairly large and complex package, it installs hooks even when it's disabled.
It would be nice if there was a way to fully disable undo-tree in evil mode.

(add-hook 'post-gc-hook #'undo-tree-post-gc nil)
;; --- snip --- 
(add-hook 'write-file-functions #'undo-tree-save-history-from-hook)
(add-hook 'kill-buffer-hook #'undo-tree-save-history-from-hook)
(add-hook 'find-file-hook #'undo-tree-load-history-from-hook)

Undo-fu by comparison:

  • doesn't install any hooks/advice/timers.
  • it's 213-SLOC compared to 2740-SLOC.
  • no GC / undo-data manipulation.
  • no extra mode/state needed It just exposes undo and redo which operate on Emacs undo information.

Since this PR, I've published undo-fu-session a companion package that can save/load undo data from disk. (to match undo-tree's functionality)

@TheBB
Copy link
Member Author

TheBB commented Feb 8, 2020

No updates yet. I wrote this to try undo-fu but at that time there were still bugs. Since then undo-tree received a promising update, so I decided I could focus on other things for the moment.

I'll get around to it eventually, but of course if someone else wants to implement it I'd be happy to review and merge.

I'm pretty sure I actually do want the provider variable though. 😄

@TheBB
Copy link
Member Author

TheBB commented Feb 8, 2020

Note: we also need to double check that goto-chg works fine when undo-tree isn't present. It's supposed to, but since evil for the moment requires undo-tree I'm not sure how much those code paths have been tested.

@ideasman42
Copy link

ideasman42 commented Feb 11, 2020

Testing goto-last-change and goto-last-change-reverse, they both seem to be working properly, although the navigation isn't linearized (it has the behavior of Emacs undo, not undo-only).

This seems more like a feature that could be supported in goto-chg.el than a bug.

@condy0919
Copy link
Contributor

Any updates?

@TheBB
Copy link
Member Author

TheBB commented Apr 13, 2020

If there's significant interest in an option for choosing undo provider please feel free to clean up the PR and resubmit. I don't have much time for evil at the moment. I've received no reports of bugs in the newer undo-tree versions so I don't consider it a priority.

@wasamasa
Copy link
Member

wasamasa commented May 8, 2020

@jixiuf @aaronjensen Do you have a minimal reproduction example for that change in hook functions? I've tried the following on Emacs built from master and failed to reproduce it with the following in a file:

(defun my-test () (message "Test"))
(add-hook 'post-command-hook 'my-test)

@TheBB
Copy link
Member Author

TheBB commented Oct 8, 2020

Closed in favor of #1360

@TheBB TheBB closed this Oct 8, 2020
@TheBB TheBB deleted the undo-fu branch October 8, 2020 07:58
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.

7 participants