[ELPA] propose to add vundo.el to ELPA

[ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

Hi,

I wrote vundo.el a while ago and it’s now quite stable. It displays the undo history in a tree form like undo-tree. Unlike undo-tree, vundo doesn’t require you to replace undo commands and track changes itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do you think vundo would be a good addition to ELPA?

https://github.com/casouri/vundo

Yuan

Re: [ELPA] propose to add vundo.el to ELPA

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

Yuan Fu <casouri@gmail.com> writes:

> I wrote vundo.el a while ago and it’s now quite stable. It displays
> the undo history in a tree form like undo-tree. Unlike undo-tree,
> vundo doesn’t require you to replace undo commands and track changes
> itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do
> you think vundo would be a good addition to ELPA?

Tried it out shortly.  Looks very useful and cool and easy to use and
well done.

Some minor comments:

- Maybe mention the arrow keys in the introduction (that they work)?

- In undo-tree, one had to select a different branch before one could
"walk" into it.  For vundo it's the other way round: you first have to
select the first element of the active branch, after the root, and only
after that you can activate a different (sibling) branch (or did
undo-tree add virtual roots or so?).  Dunno what interface approach I
like more.  But because I had tried undo-tree, your approach feels
unfamiliar.  Dunno how others see that (AFAIR, undo-tree felt strange
in the same aspect when I had tried it first, too=.

- Question: here in the comment:

;; update. Vundo catches up the next time you invoke any command:
;; instead of performing that command, it updates the tree.

do you mean "next time you invoke any _vundo_ command"?  Otherwise I
don't understand the sentence - buffer changes happen via commands - so
what command actually do update?

- And here are some typos I think (without warranty, I'm not perfectly
speaking English):


[-- Attachment #2: 0001-Some-typos.patch --]
[-- Type: text/x-diff, Size: 2477 bytes --]

From 93248a5f359153ec272871dbbe41a5a450c9ef87 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Tue, 5 Apr 2022 05:32:42 +0200
Subject: [PATCH] Some typos

---
 vundo.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/vundo.el b/vundo.el
index 27a6368..f16df2e 100644
--- a/vundo.el
+++ b/vundo.el
@@ -27,7 +27,7 @@
 ;;; Commentary:
 ;;
 ;; To use vundo, type M-x vundo RET in the buffer you want to undo.
-;; A undo tree buffer should pop up. To move around, type:
+;; An undo tree buffer should pop up. To move around, type:
 ;;
 ;;   f   to go forward
 ;;   b   to go backward
@@ -36,7 +36,7 @@
 ;;   q   to quit, you can also type C-g
 ;;
 ;; By default, you need to press RET to “commit” your change and if
-;; you quit with q or C-g, the change made by vundo are rolled back.
+;; you quit with q or C-g, the changes made by vundo are rolled back.
 ;; You can set ‘vundo-roll-back-on-quit’ to nil to disable rolling
 ;; back.
 ;;
@@ -66,7 +66,7 @@
 ;;
 ;; Your default font needs to contain these Unicode characters, otherwise
 ;; they look terrible and don’t align. You can find a font that covers
-;; these characters (eg, Symbola, Unifont), and set ‘vundo-default’ face
+;; these characters (e.g. Symbola, Unifont), and set ‘vundo-default’ face
 ;; to use that font:
 ;;
 ;;     (set-face-attribute 'vundo-default nil :family "Symbola")
@@ -114,14 +114,14 @@
 ;; Position-only records
 ;;
 ;; We know how undo works: when undoing, ‘primitive-undo’ looks at
-;; each record in ‘pending-undo-list’ and modify the buffer
+;; each record in ‘pending-undo-list’ and modifies the buffer
 ;; accordingly, and that modification itself pushes new undo records
 ;; into ‘buffer-undo-list’. However, not all undo records introduce
 ;; modification, if the record is an integer, ‘primitive-undo’ simply
 ;; ‘goto’ that position, which introduces no modification to the
 ;; buffer and pushes no undo record to ‘buffer-undo-list’. Normally
 ;; position records accompany other buffer-modifying records, but if a
-;; particular record consist of only position records, we have
+;; particular record consists of only position records, we have
 ;; trouble: after an undo step, ‘buffer-undo-list’ didn’t grow, as far
 ;; as vundo tree-folding algorithm is concerned, we didn’t move.
 ;; Assertions expecting to see new undo records in ‘buffer-undo-list’
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 111 bytes --]


but I only read the commentary partly and quickly.  Also only tried
quickly, but looks really cool.

Michael.

Re: [ELPA] propose to add vundo.el to ELPA

[Philip Kaludercic]

Yuan Fu <casouri@gmail.com> writes:

> Hi,
>
> I wrote vundo.el a while ago and it’s now quite stable. It displays
> the undo history in a tree form like undo-tree. Unlike undo-tree,
> vundo doesn’t require you to replace undo commands and track changes
> itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do
> you think vundo would be a good addition to ELPA?

I certainly think it would be, and would be glad to help with any
packaging needs.

> https://github.com/casouri/vundo
>
> Yuan
>

-- 
	Philip Kaludercic

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

> On Apr 4, 2022, at 8:41 PM, Michael Heerdegen <michael_heerdegen@web.de> wrote:
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>> I wrote vundo.el a while ago and it’s now quite stable. It displays
>> the undo history in a tree form like undo-tree. Unlike undo-tree,
>> vundo doesn’t require you to replace undo commands and track changes
>> itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do
>> you think vundo would be a good addition to ELPA?
> 
> Tried it out shortly.  Looks very useful and cool and easy to use and
> well done.
> 

Thanks for you kind words and suggestions!

> Some minor comments:
> 
> - Maybe mention the arrow keys in the introduction (that they work)?
> 
> - In undo-tree, one had to select a different branch before one could
> "walk" into it.  For vundo it's the other way round: you first have to
> select the first element of the active branch, after the root, and only
> after that you can activate a different (sibling) branch (or did
> undo-tree add virtual roots or so?).  Dunno what interface approach I
> like more.  But because I had tried undo-tree, your approach feels
> unfamiliar.  Dunno how others see that (AFAIR, undo-tree felt strange
> in the same aspect when I had tried it first, too=.

I now added further explanations.

> - Question: here in the comment:
> 
> ;; update. Vundo catches up the next time you invoke any command:
> ;; instead of performing that command, it updates the tree.
> 
> do you mean "next time you invoke any _vundo_ command"?  Otherwise I
> don't understand the sentence - buffer changes happen via commands - so
> what command actually do update?

Yes, any vundo command. Pressing f/b/n/p will not move in history but update the tree.

> 
> - And here are some typos I think (without warranty, I'm not perfectly
> speaking English):

Thanks, applied!

Yuan

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]



> On Apr 5, 2022, at 12:23 AM, Philip Kaludercic <philipk@posteo.net> wrote:
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>> Hi,
>> 
>> I wrote vundo.el a while ago and it’s now quite stable. It displays
>> the undo history in a tree form like undo-tree. Unlike undo-tree,
>> vundo doesn’t require you to replace undo commands and track changes
>> itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do
>> you think vundo would be a good addition to ELPA?
> 
> I certainly think it would be, and would be glad to help with any
> packaging needs.

Thanks. Here is a patch for ELPA.

Yuan


[-- Attachment #2: vundo.patch --]
[-- Type: application/octet-stream, Size: 870 bytes --]

From 26c0aaf44d96e3a8b74b6f55421f791a39cf3ad9 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Tue, 5 Apr 2022 21:21:07 -0700
Subject: [PATCH] * elpa-packages ("vundo"): New package.

---
 elpa-packages | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index e218968e87..8fb17c4baf 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -630,6 +630,12 @@
  ("vertico-posframe"
   :url "https://github.com/tumashu/vertico-posframe"
   :auto-sync t)
+ ("vundo"
+  :url "https://github.com/casouri/vundo"
+  :ignored-files ("test")
+  :doc "README.txt"
+  :news "NEWS.txt"
+  :auto-sync t)
  ("wcheck-mode"		:url "https://github.com/tlikonen/wcheck-mode")
  ("wconf"		:url "https://github.com/ilohmar/wconf")
  ("web-server"		:url "https://github.com/eschulte/emacs-web-server.git")
-- 
2.33.1

Re: [ELPA] propose to add vundo.el to ELPA

[Philip Kaludercic]

Yuan Fu <casouri@gmail.com> writes:

>> On Apr 5, 2022, at 12:23 AM, Philip Kaludercic <philipk@posteo.net> wrote:
>> 
>> Yuan Fu <casouri@gmail.com> writes:
>> 
>>> Hi,
>>> 
>>> I wrote vundo.el a while ago and it’s now quite stable. It displays
>>> the undo history in a tree form like undo-tree. Unlike undo-tree,
>>> vundo doesn’t require you to replace undo commands and track changes
>>> itself, rather it generates the tree fresh from ‘buffer-undo-list’. Do
>>> you think vundo would be a good addition to ELPA?
>> 
>> I certainly think it would be, and would be glad to help with any
>> packaging needs.
>
> Thanks. Here is a patch for ELPA.

I would apply this then, given no other objections.

> Yuan
>
> From 26c0aaf44d96e3a8b74b6f55421f791a39cf3ad9 Mon Sep 17 00:00:00 2001
> From: Yuan Fu <casouri@gmail.com>
> Date: Tue, 5 Apr 2022 21:21:07 -0700
> Subject: [PATCH] * elpa-packages ("vundo"): New package.
>
> ---
>  elpa-packages | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index e218968e87..8fb17c4baf 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -630,6 +630,12 @@
>   ("vertico-posframe"
>    :url "https://github.com/tumashu/vertico-posframe"
>    :auto-sync t)
> + ("vundo"
> +  :url "https://github.com/casouri/vundo"
> +  :ignored-files ("test")
> +  :doc "README.txt"
      ^
      I am assuming this is supposed to be a :readme, not a :doc.
      Or do you actually want to create a manual out of the README?

      As your commentary section seems to intentionally mirror the
      README (if this is the right thing is a different question.  I
      don't know why a package.el user is interested in testing if the
      testing code isn't bundled in the package), this could just be
      dropped.

> +  :news "NEWS.txt"

I see that your README also contains a news section.  It might make
sense only maintain the news in one of the two, to avoid duplication on
elpa.gnu.org.

Besides that, it seems to be that the introduction in README.txt doesn't
immediately explain what the package is about.  This might confuse
people using C-h P to preview the package before installing it.

> +  :auto-sync t)
>   ("wcheck-mode"		:url "https://github.com/tlikonen/wcheck-mode")
>   ("wconf"		:url "https://github.com/ilohmar/wconf")
>   ("web-server"		:url "https://github.com/eschulte/emacs-web-server.git")

-- 
	Philip Kaludercic

Re: [ELPA] propose to add vundo.el to ELPA

[Stefan Monnier]

> + ("vundo"
> +  :url "https://github.com/casouri/vundo"
> +  :ignored-files ("test")
> +  :doc "README.txt"
> +  :news "NEWS.txt"
> +  :auto-sync t)

`:doc` is for Texinfo docs.  I think you meant `:readme` instead.


        Stefan

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

>> 
>> diff --git a/elpa-packages b/elpa-packages
>> index e218968e87..8fb17c4baf 100644
>> --- a/elpa-packages
>> +++ b/elpa-packages
>> @@ -630,6 +630,12 @@
>> ("vertico-posframe"
>> :url "https://github.com/tumashu/vertico-posframe"
>> :auto-sync t)
>> + ("vundo"
>> + :url "https://github.com/casouri/vundo"
>> + :ignored-files ("test")
>> + :doc "README.txt"
> ^
> I am assuming this is supposed to be a :readme, not a :doc.
> Or do you actually want to create a manual out of the README?

That should be README.

> 
> As your commentary section seems to intentionally mirror the
> README (if this is the right thing is a different question. I
> don't know why a package.el user is interested in testing if the
> testing code isn't bundled in the package), this could just be
> dropped.

Should I let ELPA ignore README.txt and only use Commentary then?

> 
>> + :news "NEWS.txt"
> 
> I see that your README also contains a news section. It might make
> sense only maintain the news in one of the two, to avoid duplication on
> elpa.gnu.org.
> 
> Besides that, it seems to be that the introduction in README.txt doesn't
> immediately explain what the package is about. This might confuse
> people using C-h P to preview the package before installing it.

Thanks. I fixed those points and here is the new patch. In this patch I removed README.txt. If it turns out to be not desirable, I can make another patch.

Yuan


[-- Attachment #2: vundo.patch --]
[-- Type: application/octet-stream, Size: 860 bytes --]

From 8fac9d0caf324f1320ee99d9cdf421c50e30ac4d Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Tue, 5 Apr 2022 21:21:07 -0700
Subject: [PATCH] * elpa-packages ("vundo"): New package.

---
 elpa-packages | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/elpa-packages b/elpa-packages
index e218968e87..c6970f77b6 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -630,6 +630,11 @@
  ("vertico-posframe"
   :url "https://github.com/tumashu/vertico-posframe"
   :auto-sync t)
+ ("vundo"
+  :url "https://github.com/casouri/vundo"
+  :ignored-files ("test" "README.txt")
+  :news "NEWS.txt"
+  :auto-sync t)
  ("wcheck-mode"		:url "https://github.com/tlikonen/wcheck-mode")
  ("wconf"		:url "https://github.com/ilohmar/wconf")
  ("web-server"		:url "https://github.com/eschulte/emacs-web-server.git")
-- 
2.33.1


[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

I forgot to mention that all significant contributors (Campbell Barton and philipk@posteo.net) have signed the copyright assignment.

Yuan

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

> On Apr 6, 2022, at 11:08 AM, Yuan Fu <casouri@gmail.com> wrote:
> 
> I forgot to mention that all significant contributors (Campbell Barton and philipk@posteo.net) have signed the copyright assignment.

Oh you are the same Pillip, just noticed :-)

Yuan

Re: [ELPA] propose to add vundo.el to ELPA

[Philip Kaludercic]

Yuan Fu <casouri@gmail.com> writes:

>>> 
>>> diff --git a/elpa-packages b/elpa-packages
>>> index e218968e87..8fb17c4baf 100644
>>> --- a/elpa-packages
>>> +++ b/elpa-packages
>>> @@ -630,6 +630,12 @@
>>> ("vertico-posframe"
>>> :url "https://github.com/tumashu/vertico-posframe"
>>> :auto-sync t)
>>> + ("vundo"
>>> + :url "https://github.com/casouri/vundo"
>>> + :ignored-files ("test")
>>> + :doc "README.txt"
>> ^
>> I am assuming this is supposed to be a :readme, not a :doc.
>> Or do you actually want to create a manual out of the README?
>
> That should be README.

Ok.

>> As your commentary section seems to intentionally mirror the
>> README (if this is the right thing is a different question. I
>> don't know why a package.el user is interested in testing if the
>> testing code isn't bundled in the package), this could just be
>> dropped.
>
> Should I let ELPA ignore README.txt and only use Commentary then?

Sure that can be done.  I personally like this approach because a README
in my eyes is what a contributor is interested in while the commentary
section is what a package user wants to know.

>> 
>>> + :news "NEWS.txt"
>> 
>> I see that your README also contains a news section. It might make
>> sense only maintain the news in one of the two, to avoid duplication on
>> elpa.gnu.org.
>> 
>> Besides that, it seems to be that the introduction in README.txt doesn't
>> immediately explain what the package is about. This might confuse
>> people using C-h P to preview the package before installing it.
>
> Thanks. I fixed those points and here is the new patch. In this patch
> I removed README.txt. If it turns out to be not desirable, I can make
> another patch.


Looks good.  BTW, you can also add a .elpaignore file to your repository
if you want to maintain what files to ignore on your end.

-- 
	Philip Kaludercic

Re: [ELPA] propose to add vundo.el to ELPA

[Michael Heerdegen]

[-- Attachment #1: Type: text/plain, Size: 77 bytes --]

Yuan Fu <casouri@gmail.com> writes:

> Thanks, applied!

Ok, 3 little more:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Three-more-tyos.patch --]
[-- Type: text/x-diff, Size: 1248 bytes --]

From 2820d87b9d10f87921677560aae141a8dd43f6d4 Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Fri, 8 Apr 2022 00:43:20 +0200
Subject: [PATCH] Three more tyos

---
 vundo.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/vundo.el b/vundo.el
index f16df2e..4de45de 100644
--- a/vundo.el
+++ b/vundo.el
@@ -91,12 +91,12 @@
 ;; `vundo-m' object corresponding to it. Once we have the mod-list and
 ;; hash table, we connect the nodes in mod-list to form a tree in
 ;; `vundo--build-tree'. We build the tree by a simple observation:
-;; only non-undo modifications creates new unique buffer states and
+;; Only non-undo modifications create new unique buffer states and
 ;; need to be drawn in the tree. For undo modifications, they
-;; associates equivalent nodes.
+;; associate equivalent nodes.
 ;;
 ;; Once we have generated the data structure and drawn the tree, vundo
-;; commands can move around on that tree by calling
+;; commands can move around in that tree by calling
 ;; `vundo--move-to-node'. It will construct the correct undo-list and
 ;; feed it to `primitive-undo'. `vundo--trim-undo-list' can trim the
 ;; undo list when possible.
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 946 bytes --]



Some more ideas/thoughts:

(1) If you use `vundo' for the first time in a buffer with an already
very large undo list, do you expect a delay?  Would it make sense to
limit the processing of `buffer-undo-list' to the last N entries to
avoid such a delay (N would probably be a user option)?

(2) I wonder which setup I will prefer in the future.  I'm using the
commands that already "fold" the undo list (AFAIU, in the same or a
similar sense as vundo does): `undo-only' and `undo-redo'.  Do these
commands directly correspond to hitting right and left in the vundo
buffer?

If yes, I think it would be cool to setup stuff that the keys I use for
`undo-only' and `undo-redo' would work like now but additionally start
vundo.  And the same keys would still work in vundo and call
vundo-backward and vundo-forwar, additionally to the existing bindings.
I think that could feel convenient.  Would such a setup make sense to
you?

Thanks,

Michael.

Re: [ELPA] propose to add vundo.el to ELPA

[Yuan Fu]

> 
> Ok, 3 little more:
> 

Thanks, merged.

> 
> Some more ideas/thoughts:
> 
> (1) If you use `vundo' for the first time in a buffer with an already
> very large undo list, do you expect a delay?  Would it make sense to
> limit the processing of `buffer-undo-list' to the last N entries to
> avoid such a delay (N would probably be a user option)?

That depends on exactly how large the undo list is. For any daily workload vundo is pretty fast. For super long undo lists there is a delay but not too bad. I just tried 1k and 10k entries, 1k took ~0.3s, 10k took ~3s.

> 
> (2) I wonder which setup I will prefer in the future.  I'm using the
> commands that already "fold" the undo list (AFAIU, in the same or a
> similar sense as vundo does): `undo-only' and `undo-redo'.  Do these
> commands directly correspond to hitting right and left in the vundo
> buffer?

Pretty much. They don’t run the same code but the semantics should be the same. I use undo-only and undo-redo myself.

> 
> If yes, I think it would be cool to setup stuff that the keys I use for
> `undo-only' and `undo-redo' would work like now but additionally start
> vundo.  And the same keys would still work in vundo and call
> vundo-backward and vundo-forwar, additionally to the existing bindings.
> I think that could feel convenient.  Would such a setup make sense to
> you?

I never feel the need to see the undo tree when I’m just doing simple undo. You can definitely try it using transient keymaps

Yuan

Re: [ELPA] propose to add vundo.el to ELPA

[Michael Heerdegen]

Yuan Fu <casouri@gmail.com> writes:

> I never feel the need to see the undo tree when I’m just doing simple
> undo.

You are probably right.  At least the captured bindings irritate when
you expect e.g. n to self-insert after (v)undo.

So IF I wanted something like that automatically then only the display
without the vundo commands.  But I'm just experimenting.  It's easy
enough to invoke vundo when one needs it.

BTW, so far vundo works well in real life.

Michael.

Re: [ELPA] propose to add vundo.el to ELPA

[Philip Kaludercic]

I would then want to add vundo to ELPA, unless there are any objections?

Philip Kaludercic <philipk@posteo.net> writes:

> Yuan Fu <casouri@gmail.com> writes:
>
>>>> 
>>>> diff --git a/elpa-packages b/elpa-packages
>>>> index e218968e87..8fb17c4baf 100644
>>>> --- a/elpa-packages
>>>> +++ b/elpa-packages
>>>> @@ -630,6 +630,12 @@
>>>> ("vertico-posframe"
>>>> :url "https://github.com/tumashu/vertico-posframe"
>>>> :auto-sync t)
>>>> + ("vundo"
>>>> + :url "https://github.com/casouri/vundo"
>>>> + :ignored-files ("test")
>>>> + :doc "README.txt"
>>> ^
>>> I am assuming this is supposed to be a :readme, not a :doc.
>>> Or do you actually want to create a manual out of the README?
>>
>> That should be README.
>
> Ok.
>
>>> As your commentary section seems to intentionally mirror the
>>> README (if this is the right thing is a different question. I
>>> don't know why a package.el user is interested in testing if the
>>> testing code isn't bundled in the package), this could just be
>>> dropped.
>>
>> Should I let ELPA ignore README.txt and only use Commentary then?
>
> Sure that can be done.  I personally like this approach because a README
> in my eyes is what a contributor is interested in while the commentary
> section is what a package user wants to know.
>
>>> 
>>>> + :news "NEWS.txt"
>>> 
>>> I see that your README also contains a news section. It might make
>>> sense only maintain the news in one of the two, to avoid duplication on
>>> elpa.gnu.org.
>>> 
>>> Besides that, it seems to be that the introduction in README.txt doesn't
>>> immediately explain what the package is about. This might confuse
>>> people using C-h P to preview the package before installing it.
>>
>> Thanks. I fixed those points and here is the new patch. In this patch
>> I removed README.txt. If it turns out to be not desirable, I can make
>> another patch.
>
> Looks good.  BTW, you can also add a .elpaignore file to your repository
> if you want to maintain what files to ignore on your end.

-- 
	Philip Kaludercic