bug#6419: A suggestion for easier editing of lists in Custom buffers

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Lennart Borgman]

Moving things in a list in a custom buffer is not very easy currently,
especially not if it contains nested lists. I have been thinking of
implementing some up/down arrows to permit moving of subtrees, but
that is also rather clumsy so I have refrained from it.


Here is another idea which perhaps both is more useful and easier to implement:

A) When clicking on a [-] do delete a subtree save (a print
representation) of it to the clipboard/kill ring (beside deleting the
subtree, of course ;-)

B) When on a [+] do add a new subtree give the possibility to paste
the deleted subtree in this position.


A is of course no problem to implement. B is probably not that hard to
implement either if one has the (sub)type for the subtree available on
the [+] button.

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

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

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Moving things in a list in a custom buffer is not very easy currently,
> especially not if it contains nested lists. I have been thinking of
> implementing some up/down arrows to permit moving of subtrees, but
> that is also rather clumsy so I have refrained from it.
>
>
> Here is another idea which perhaps both is more useful and easier to
implement:
>
> A) When clicking on a [-] do delete a subtree save (a print
> representation) of it to the clipboard/kill ring (beside deleting the
> subtree, of course ;-)
>
> B) When on a [+] do add a new subtree give the possibility to paste
> the deleted subtree in this position.
>
>
> A is of course no problem to implement. B is probably not that hard to
> implement either if one has the (sub)type for the subtree available on
> the [+] button.

This idea didn't get any feedback back then, but I think there might be
value in it.  I'd tweak it to delete/insert widgets, rather than using
the kill ring.

Any opinions?

[-- Attachment #2: Type: text/html, Size: 1243 bytes --]

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> This idea didn't get any feedback back then, but I think there might be
> value in it.  I'd tweak it to delete/insert widgets, rather than using
> the kill ring.
>
> Any opinions?

I think it sounds like a good idea.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 368 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> This idea didn't get any feedback back then, but I think there might be
>> value in it.  I'd tweak it to delete/insert widgets, rather than using
>> the kill ring.
>>
>> Any opinions?
>
> I think it sounds like a good idea.

Thanks.

Here's my shot at it, please review.

[-- Attachment #1.2: Type: text/html, Size: 583 bytes --]

[-- Attachment #2: 0001-Allow-moving-members-of-editable-list-widget-via-del.patch --]
[-- Type: text/x-patch, Size: 6003 bytes --]

From 8277637c9ff1fdf1e62140700e27a1c8e65d39bf Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Wed, 21 Oct 2020 11:07:49 -0300
Subject: [PATCH] Allow moving members of editable-list widget, via
 delete+insert

Suggested by Lennart Borgman in Bug#6419.

* lisp/wid-edit.el (widget-editable-list-delete-at): Save into a new
widget property, :last-deleted, the WIDGET to be deleted.  Add
docstring.
(widget-editable-list-insert-before): If there is a recently deleted
child for the editable list, insert that one, instead of a new default
widget.  Add docstring.
(insert-button widget, delete-button widget): Tweak the :help-echo, to
document this behavior.

* test/lisp/wid-edit-tests.el (widget-test-moving-editable-list-item):
Test the feature.

* etc/NEWS (Widget): Announce the feature.
---
 etc/NEWS                    |  7 +++++++
 lisp/wid-edit.el            | 29 +++++++++++++++++++++++++----
 test/lisp/wid-edit-tests.el | 19 +++++++++++++++++++
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f3e3d9a1b6..6e20530f6d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1166,6 +1166,13 @@ window after starting).  This variable defaults to nil.
 +++
 *** 'widget-choose' now supports menus in extended format.
 
+---
+*** The 'editable-list' widget now supports moving items up and down.
+You can now move items up and down by deleting and then reinserting
+them, using the DEL and INS buttons respectively.  This is useful in
+Custom buffers, for example, to change the order of the elements in a
+list.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 6568cd2c8f..37a04cb1cd 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2721,7 +2721,7 @@ widget-radio-action
 (define-widget 'insert-button 'push-button
   "An insert button for the `editable-list' widget."
   :tag "INS"
-  :help-echo "Insert a new item into the list at this position."
+  :help-echo "Insert a new item (or a previously deleted one) into the list at this position."
   :action 'widget-insert-button-action)
 
 (defun widget-insert-button-action (widget &optional _event)
@@ -2734,7 +2734,7 @@ widget-insert-button-action
 (define-widget 'delete-button 'push-button
   "A delete button for the `editable-list' widget."
   :tag "DEL"
-  :help-echo "Delete this item from the list."
+  :help-echo "Delete this item from the list, saving it for later reinsertion."
   :action 'widget-delete-button-action)
 
 (defun widget-delete-button-action (widget &optional _event)
@@ -2824,9 +2824,17 @@ widget-editable-list-match-inline
     (cons found value)))
 
 (defun widget-editable-list-insert-before (widget before)
-  ;; Insert a new child in the list of children.
+  "Insert a new widget as a child of WIDGET.
+
+If there is a recently deleted child, the new widget is that deleted child.
+Otherwise, the new widget is the default child of WIDGET.
+
+The new widget gets inserted at the position of the BEFORE child."
   (save-excursion
     (let ((children (widget-get widget :children))
+          (last-deleted (let ((lst (widget-get widget :last-deleted)))
+                          (and lst (prog1 (pop lst)
+                                     (widget-put widget :last-deleted lst)))))
 	  (inhibit-read-only t)
 	  (inhibit-modification-hooks t))
       (cond (before
@@ -2834,7 +2842,11 @@ widget-editable-list-insert-before
 	    (t
 	     (goto-char (widget-get widget :value-pos))))
       (let ((child (widget-editable-list-entry-create
-		    widget nil nil)))
+                    widget (and last-deleted
+                                (widget-apply last-deleted
+                                              :value-to-external
+                                              (widget-get last-deleted :value)))
+                    last-deleted)))
 	(when (< (widget-get child :entry-from) (widget-get widget :from))
 	  (set-marker (widget-get widget :from)
 		      (widget-get child :entry-from)))
@@ -2847,6 +2859,15 @@ widget-editable-list-insert-before
   (widget-apply widget :notify widget))
 
 (defun widget-editable-list-delete-at (widget child)
+  "Delete the widget CHILD from the known childrens of widget WIDGET.
+
+Save CHILD into the :last-deleted list, so it can be inserted later."
+  ;; Save the current value of CHILD, to use if the user later inserts the
+  ;; widget.
+  (widget-put child :value (widget-apply child :value-get))
+  (let ((lst (widget-get widget :last-deleted)))
+    (push child lst)
+    (widget-put widget :last-deleted lst))
   ;; Delete child from list of children.
   (save-excursion
     (let ((buttons (copy-sequence (widget-get widget :buttons)))
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index df49ffc822..4508b68023 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -129,4 +129,23 @@ widget-test-editable-field-widget-value
       (widget-insert "And some non-widget text.")
       (should (string= (widget-apply wid :value-get) "")))))
 
+(ert-deftest widget-test-moving-editable-list-item ()
+  "Check that we can move an editable list item up or down, via delete+insert."
+  (with-temp-buffer
+    (widget-insert "Testing editable-list.\n\n")
+    (let ((lst (widget-create 'editable-list
+                              :value '("beg" "end" "middle")
+                              '(editable-field :value "unknown"))))
+      (use-local-map widget-keymap)
+      (widget-setup)
+      ;; Go to the DEL button for the 2nd element and action it.
+      (goto-char (widget-get (nth 2 (widget-get lst :buttons)) :from))
+      (widget-apply-action (widget-at))
+      ;; Go to the INS button and action it.
+      (goto-char (widget-get lst :to))
+      (widget-backward 1)
+      (widget-apply-action (widget-at))
+      ;; Check that we effectively moved the item to the last position.
+      (should (equal (widget-value lst) '("beg" "middle" "end"))))))
+
 ;;; wid-edit-tests.el ends here
-- 
2.28.0

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Drew Adams]

"Insert a new item (or a previously deleted one) into the list at this position."

Should "a previously deleted one" be "the last-deleted one"?

If you use INS in one customize buffer after using
DEL in another, or after using DEL in another part
of the same buffer (but not part of the same sequence),
is that last-deleted item inserted?  Or is it only the
last-deleted (if any) from the same sequence?

"If there is a recently deleted child, the new widget
is that deleted child."

Same question/problem.  Any recently deleted child?
What if there are several?  Is it the last-deleted
that's used?  (I see that the code says "last-deleted".)

"from the known childrens" -> "from the known children"

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

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

Drew Adams <drew.adams@oracle.com> writes:

> "Insert a new item (or a previously deleted one) into the list at this
> position."
>
> Should "a previously deleted one" be "the last-deleted one"?

Not necessarily.  It works like a stack.

> If you use INS in one customize buffer after using
> DEL in another, or after using DEL in another part
> of the same buffer (but not part of the same sequence),
> is that last-deleted item inserted?  Or is it only the
> last-deleted (if any) from the same sequence?

The latter.  This only implements moving items within the same
sequence.

> "If there is a recently deleted child, the new widget
> is that deleted child."
>
> Same question/problem.  Any recently deleted child?
> What if there are several?  Is it the last-deleted
> that's used?  (I see that the code says "last-deleted".)

Hopefully what I said above made it clearer.  I guess it's clear that I
had trouble with making it clear and keeping the help-echo short at the
same time, so I'll be glad if someone can improve the wording, or
suggest improvements.

> "from the known childrens" -> "from the known children"

I'll fix that.

Thanks.

[-- Attachment #2: Type: text/html, Size: 1466 bytes --]

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Drew Adams]

>> Should "a previously deleted one" be "the last-deleted one"?
>
> Not necessarily.  It works like a stack.

How is that different - LIFO, no?
In any case, how it works needs to be made clear
somewhere.

>> If you use INS in one customize buffer after using
>> DEL in another, or after using DEL in another part
>> of the same buffer (but not part of the same sequence),
>> is that last-deleted item inserted?  Or is it only the
>> last-deleted (if any) from the same sequence?
>
> The latter.  This only implements moving items within the same
sequence.

So maybe make that clear somehow.

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

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

Drew Adams <drew.adams@oracle.com> writes:

>>> Should "a previously deleted one" be "the last-deleted one"?
>>
>> Not necessarily.  It works like a stack.
>
> How is that different - LIFO, no?
> In any case, how it works needs to be made clear
> somewhere.

Yes, of course.  But in that case I think the help-echo should say
explicitly: "the last-deleted one not yet reinserted", or something like
that.

>>> If you use INS in one customize buffer after using
>>> DEL in another, or after using DEL in another part
>>> of the same buffer (but not part of the same sequence),
>>> is that last-deleted item inserted?  Or is it only the
>>> last-deleted (if any) from the same sequence?
>>
>> The latter.  This only implements moving items within the same
> sequence.
>
> So maybe make that clear somehow.

Will try.

[-- Attachment #2: Type: text/html, Size: 1094 bytes --]

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Will try.

OK, so you're sending a new version of the patch?  Then I won't apply
the one you've already posted (which seemed clear enough to me, really).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 347 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Will try.
>
> OK, so you're sending a new version of the patch?  Then I won't apply
> the one you've already posted (which seemed clear enough to me, really).

Yes, here it is.  I fixed the typo, and tried to improve
the help-echo of the INS button.

[-- Attachment #1.2: Type: text/html, Size: 526 bytes --]

[-- Attachment #2: 0001-Allow-moving-members-of-editable-list-widget-via-del.patch --]
[-- Type: text/x-patch, Size: 6261 bytes --]

From ffbfcdc10f916ef37187ed2a6a8f3b2f0380e643 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Wed, 21 Oct 2020 11:07:49 -0300
Subject: [PATCH] Allow moving members of editable-list widget, via
 delete+insert

Suggested by Lennart Borgman in Bug#6419.

* lisp/wid-edit.el (widget-editable-list-delete-at): Save into a new
widget property, :last-deleted, the WIDGET to be deleted.  Add
docstring.
(widget-editable-list-insert-before): If there is a recently deleted
child for the editable list, insert that one, instead of a new default
widget.  Add docstring.
(insert-button widget): Make :help-echo a function to avoid the
help-echo string become too long.
(delete-button widget): Tweak the :help-echo string, to document this
behavior.

* test/lisp/wid-edit-tests.el (widget-test-moving-editable-list-item):
Test the feature.

* etc/NEWS (Widget): Announce the feature.
---
 etc/NEWS                    |  7 +++++++
 lisp/wid-edit.el            | 32 ++++++++++++++++++++++++++++----
 test/lisp/wid-edit-tests.el | 19 +++++++++++++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f3e3d9a1b6..6e20530f6d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1166,6 +1166,13 @@ window after starting).  This variable defaults to nil.
 +++
 *** 'widget-choose' now supports menus in extended format.
 
+---
+*** The 'editable-list' widget now supports moving items up and down.
+You can now move items up and down by deleting and then reinserting
+them, using the DEL and INS buttons respectively.  This is useful in
+Custom buffers, for example, to change the order of the elements in a
+list.
+
 ** Miscellaneous
 
 ---
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 6568cd2c8f..33c7de24bc 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -2721,7 +2721,10 @@ widget-radio-action
 (define-widget 'insert-button 'push-button
   "An insert button for the `editable-list' widget."
   :tag "INS"
-  :help-echo "Insert a new item into the list at this position."
+  :help-echo (lambda (widget)
+               (if (widget-get (widget-get widget :parent) :last-deleted)
+                   "Insert back the last deleted item from this list, at this position."
+                 "Insert a new item into the list at this position."))
   :action 'widget-insert-button-action)
 
 (defun widget-insert-button-action (widget &optional _event)
@@ -2734,7 +2737,7 @@ widget-insert-button-action
 (define-widget 'delete-button 'push-button
   "A delete button for the `editable-list' widget."
   :tag "DEL"
-  :help-echo "Delete this item from the list."
+  :help-echo "Delete this item from the list, saving it for later reinsertion."
   :action 'widget-delete-button-action)
 
 (defun widget-delete-button-action (widget &optional _event)
@@ -2824,9 +2827,17 @@ widget-editable-list-match-inline
     (cons found value)))
 
 (defun widget-editable-list-insert-before (widget before)
-  ;; Insert a new child in the list of children.
+  "Insert a new widget as a child of WIDGET.
+
+If there is a recently deleted child, the new widget is that deleted child.
+Otherwise, the new widget is the default child of WIDGET.
+
+The new widget gets inserted at the position of the BEFORE child."
   (save-excursion
     (let ((children (widget-get widget :children))
+          (last-deleted (let ((lst (widget-get widget :last-deleted)))
+                          (and lst (prog1 (pop lst)
+                                     (widget-put widget :last-deleted lst)))))
 	  (inhibit-read-only t)
 	  (inhibit-modification-hooks t))
       (cond (before
@@ -2834,7 +2845,11 @@ widget-editable-list-insert-before
 	    (t
 	     (goto-char (widget-get widget :value-pos))))
       (let ((child (widget-editable-list-entry-create
-		    widget nil nil)))
+                    widget (and last-deleted
+                                (widget-apply last-deleted
+                                              :value-to-external
+                                              (widget-get last-deleted :value)))
+                    last-deleted)))
 	(when (< (widget-get child :entry-from) (widget-get widget :from))
 	  (set-marker (widget-get widget :from)
 		      (widget-get child :entry-from)))
@@ -2847,6 +2862,15 @@ widget-editable-list-insert-before
   (widget-apply widget :notify widget))
 
 (defun widget-editable-list-delete-at (widget child)
+  "Delete the widget CHILD from the known children of widget WIDGET.
+
+Save CHILD into the :last-deleted list, so it can be inserted later."
+  ;; Save the current value of CHILD, to use if the user later inserts the
+  ;; widget.
+  (widget-put child :value (widget-apply child :value-get))
+  (let ((lst (widget-get widget :last-deleted)))
+    (push child lst)
+    (widget-put widget :last-deleted lst))
   ;; Delete child from list of children.
   (save-excursion
     (let ((buttons (copy-sequence (widget-get widget :buttons)))
diff --git a/test/lisp/wid-edit-tests.el b/test/lisp/wid-edit-tests.el
index df49ffc822..4508b68023 100644
--- a/test/lisp/wid-edit-tests.el
+++ b/test/lisp/wid-edit-tests.el
@@ -129,4 +129,23 @@ widget-test-editable-field-widget-value
       (widget-insert "And some non-widget text.")
       (should (string= (widget-apply wid :value-get) "")))))
 
+(ert-deftest widget-test-moving-editable-list-item ()
+  "Check that we can move an editable list item up or down, via delete+insert."
+  (with-temp-buffer
+    (widget-insert "Testing editable-list.\n\n")
+    (let ((lst (widget-create 'editable-list
+                              :value '("beg" "end" "middle")
+                              '(editable-field :value "unknown"))))
+      (use-local-map widget-keymap)
+      (widget-setup)
+      ;; Go to the DEL button for the 2nd element and action it.
+      (goto-char (widget-get (nth 2 (widget-get lst :buttons)) :from))
+      (widget-apply-action (widget-at))
+      ;; Go to the INS button and action it.
+      (goto-char (widget-get lst :to))
+      (widget-backward 1)
+      (widget-apply-action (widget-at))
+      ;; Check that we effectively moved the item to the last position.
+      (should (equal (widget-value lst) '("beg" "middle" "end"))))))
+
 ;;; wid-edit-tests.el ends here
-- 
2.28.0

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Yes, here it is.  I fixed the typo, and tried to improve
> the help-echo of the INS button.

Thanks, applied to Emacs 28 with one super-minor tweak:

> +          (last-deleted (let ((lst (widget-get widget :last-deleted)))
> +                          (and lst (prog1 (pop lst)
> +                                     (widget-put widget :last-deleted lst))))

I made this into a `when-let' instead of the let+and.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#6419: A suggestion for easier editing of lists in Custom buffers

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Yes, here it is.  I fixed the typo, and tried to improve
>> the help-echo of the INS button.
>
> Thanks, applied to Emacs 28 with one super-minor tweak:
>
>> +          (last-deleted (let ((lst (widget-get widget :last-deleted)))
>> +                          (and lst (prog1 (pop lst)
>> +                                     (widget-put widget :last-deleted
lst))))
>
> I made this into a `when-let' instead of the let+and.

Noticed.  Thanks!

[-- Attachment #2: Type: text/html, Size: 794 bytes --]