bug#7851: `customize's presentation of :type (repeat plist) declarations

bug#7851: `customize's presentation of :type (repeat plist) declarations

[MON KEY]

`customize's presentation of variables defined with a type speciifer:

 :type '(repeat plist :value-type (repeat symbol))

do not present a sensibly in customize buffers.

For example:

(defcustom *tt--ugly-plist-custom*
  '((:KEY-PL0-0 (sym0-0a sym0-0b sym0-0c)
     :KEY-PL0-1 (sym0-1a sym0-1b sym0-1c))
    (:KEY-PL1-0 (sym1-0a sym1-0b sym1-0c)
     :KEY-PL1-1 (sym1-1a sym1-1b sym1-1c)))
  "This variable will have an ugly customize presentation.
Evaluating the following form:\n
 \(customize-variable '*tt--ugly-plist-custom*\)\n
presents a buffer named \"*Customize *Tt  Ugly Plist Custom*\".\n
Notice that for each plist the first pair of nested buttons [INS]
\[DEL] is indented deeper than subsequent pairs within the same plist
element. IOW, Instead of prsenting:\n
\[INS] [DEL] Plist:
             [INS] [DEL] Key: :KEY-PL0-0
                            Value: \(sym0-0a sym0-0b sym0-0c\)
             [INS] [DEL] Key: :KEY-PL0-1
                            Value: \(sym0-1a sym0-1b sym0-1c\)
             [INS]\n
\[INS] [DEL] Plist:
             [INS] [DEL] Key: :KEY-PL1-0
                            Value: \(sym1-0a sym1-0b sym1-0c\)
             [INS] [DEL] Key: :KEY-PL1-1
                            Value: \(sym1-1a sym1-1b sym1-1c\)
             [INS]\n\n
We instead get a customize presentation like this:\n
\[INS] [DEL] Plist:
                            [INS] [DEL] Key: :KEY-PL0-0
                            Value: \(sym0-0a sym0-0b sym0-0c\)
             [INS] [DEL] Key: :KEY-PL0-1
                            Value: \(sym0-1a sym0-1b sym0-1c\)
             [INS]\n
\[INS] [DEL] Plist:
                            [INS] [DEL] Key: :KEY-PL1-0
                            Value: \(sym1-0a sym1-0b sym1-0c\)
             [INS] [DEL] Key: :KEY-PL1-1
                            Value: \(sym1-1a sym1-1b sym1-1c\)
             [INS]\n\n
This indentation style \"plist\" custom types is confusing.\n"
  :type '(repeat plist :value-type (repeat symbol))
  :group 'emacs)

(customize-variable '*tt--ugly-plist-custom*)

emacs-version => 23.2.1

--
/s_P\

bug#7851: `customize's presentation of :type (repeat plist) declarations

[Mauro Aranda]

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

tags 7851 patch
quit

MON KEY <monkey@sandpframing.com> writes:

> `customize's presentation of variables defined with a type speciifer:
>
>  :type '(repeat plist :value-type (repeat symbol))
>
> do not present a sensibly in customize buffers.
>

I can reproduce this.  The problem is that in some cases, the
code that does the indenting (by inserting space characters) doesn't
really check if it should indent or not.

Sometimes it checks for a preceding newline character, but since
`widget-specify-insert' narrows the buffer, it can easily fail to detect
that "\n".  Sometimes, it doesn't check if it should indent at all, like
in `widget-editable-list-format-handler'.

I propose the attached patch, that introduces a function to perform the
necessary checks before indenting, and then uses it in all places I've
found is necessary.

Best regards,
Mauro.

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

[-- Attachment #2: 0001-Fix-indentation-of-widgets-Bug-7851.patch --]
[-- Type: text/x-patch, Size: 4156 bytes --]

From 3ec43f6de185cde341b80579ecf3a051239e5fef Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 24 Sep 2019 18:01:58 -0300
Subject: [PATCH] Fix indentation of widgets (Bug#7851)

* lisp/wid-edit.el (widget--should-indent-p): New function, to decide
whether to indent or not.
(widget-checklist-value-add-item, widget-radio-add-item)
(widget-editable-list-format-handler)
(widget-editable-list-entry-create)
(widget-group-value-create): Use it.
---
 lisp/wid-edit.el | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index 2978dc8..52b7532 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -412,6 +412,17 @@ widget-specify-doc
     (overlay-put overlay 'evaporate t)
     (widget-put widget :doc-overlay overlay)))
 
+(defun widget--should-indent-p (&optional check-after)
+  "Non-nil if we should indent at the current position.
+With CHECK-AFTER non-nil, considers also the content after point, if needed."
+  (save-restriction
+    (widen)
+    (and (eq (preceding-char) ?\n)
+         (or (not check-after)
+             ;; If there is a space character, then we probably already
+             ;; indented it.
+             (not (eq (following-char) ?\s))))))
+
 (defmacro widget-specify-insert (&rest form)
   "Execute FORM without inheriting any text properties."
    (declare (debug body))
@@ -2254,7 +2265,7 @@ widget-checklist-value-create
 (defun widget-checklist-add-item (widget type chosen)
   "Create checklist item in WIDGET of type TYPE.
 If the item is checked, CHOSEN is a cons whose cdr is the value."
-  (and (eq (preceding-char) ?\n)
+  (and (widget--should-indent-p)
        (widget-get widget :indent)
        (insert-char ?\s (widget-get widget :indent)))
   (widget-specify-insert
@@ -2435,7 +2446,7 @@ widget-radio-value-create
 (defun widget-radio-add-item (widget type)
   "Add to radio widget WIDGET a new radio button item of type TYPE."
   ;; (setq type (widget-convert type))
-  (and (eq (preceding-char) ?\n)
+  (and (widget--should-indent-p)
        (widget-get widget :indent)
        (insert-char ?\s (widget-get widget :indent)))
   (widget-specify-insert
@@ -2614,7 +2625,8 @@ widget-editable-list-format-handler
   ;; We recognize the insert button.
     ;; (let ((widget-push-button-gui widget-editable-list-gui))
     (cond ((eq escape ?i)
-	   (and (widget-get widget :indent)
+	   (and (widget--should-indent-p)
+                (widget-get widget :indent)
 		(insert-char ?\s (widget-get widget :indent)))
 	   (apply 'widget-create-child-and-convert
 		  widget 'insert-button
@@ -2723,8 +2735,9 @@ widget-editable-list-entry-create
 	child delete insert)
     (widget-specify-insert
      (save-excursion
-       (and (widget-get widget :indent)
-	    (insert-char ?\s (widget-get widget :indent)))
+       (and (widget--should-indent-p)
+            (widget-get widget :indent)
+            (insert-char ?\s (widget-get widget :indent)))
        (insert (widget-get widget :entry-format)))
      ;; Parse % escapes in format.
      (while (re-search-forward "%\\(.\\)" nil t)
@@ -2752,6 +2765,12 @@ widget-editable-list-entry-create
        (if insert (push insert buttons))
        (if delete (push delete buttons))
        (widget-put widget :buttons buttons))
+     ;; After creating the entry, we must check if we should indent the
+     ;; following entry.  This is necessary, for example, to keep the correct
+     ;; indentation of editable lists inside group widgets.
+     (and (widget--should-indent-p t)
+          (widget-get widget :indent)
+          (insert-char ?\s (widget-get widget :indent)))
      (let ((entry-from (point-min-marker))
 	   (entry-to (point-max-marker)))
        (set-marker-insertion-type entry-from t)
@@ -2786,7 +2805,7 @@ widget-group-value-create
 	    args (cdr args)
 	    answer (widget-match-inline arg value)
 	    value (cdr answer))
-      (and (eq (preceding-char) ?\n)
+      (and (widget--should-indent-p)
 	   (widget-get widget :indent)
 	   (insert-char ?\s (widget-get widget :indent)))
       (push (cond ((null answer)
-- 
2.7.4

bug#7851: `customize's presentation of :type (repeat plist) declarations

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I propose the attached patch, that introduces a function to perform the
> necessary checks before indenting, and then uses it in all places I've
> found is necessary.

Looks good to me, so I've now applied it to the trunk.

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