Subject: Re: bug#66635: 30.0.50; customize-icon State button doesn't work (never did)

From: Mauro Aranda <[email-masked]>
Date: Sat, 21 Oct 2023 09:28:33 -0300

On 21/10/23 08:51, Stefan Kangas wrote:
 > Eli Zaretskii <[email-masked]> writes:
 >
 >> I think this is OK for emacs-29, thanks.
 >
 > Thanks.
 >
 > Mauro, could you send the latest version of the patch?  I believe the
 > one you sent before has a typo.

Hmm, I didn't find the typo.  But it's hard for me to catch typos of my
own.  Sending the patch again anyway, but let me know if you spot it.

From ab4fabf48665ddc142ad95a26898eb6207cd2bdc Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Thu, 19 Oct 2023 08:46:35 -0300
Subject: [PATCH] Fix State button for customize-icon (Bug#66635)

* lisp/cus-edit.el (custom-icon-action): New function.
(custom-icon): Use it as the :action.  Otherwise, clicking the State
button is a noop.  Remove irrelevant stuff from the docstring and
comment out some copy-pasta.
(custom-icon-extended-menu): New variable, the menu to show upon
:action.
(custom-icon-set): Really redraw the widget with the new settings.
Comment out strange call to custom-variable-backup-value.
(custom-icon-save): New function.

* lisp/emacs-lisp/icons.el (icons--merge-spec): Fix call to plist-get
and avoid infloop.
---
 lisp/cus-edit.el         | 71 +++++++++++++++++++++++++++++++++-------
 lisp/emacs-lisp/icons.el |  6 ++--
 2 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 706e08d5657..953b8b8b80f 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -5366,11 +5366,6 @@ 'custom-icon
 :hidden-states should be a list of widget states for which the
   widget's initial contents are to be hidden.
 
-:custom-form should be a symbol describing how to display and
-  edit the variable---either `edit' (using edit widgets),
-  `lisp' (as a Lisp sexp), or `mismatch' (should not happen);
-  if nil, use the return value of `custom-variable-default-form'.
-
 :shown-value, if non-nil, should be a list whose `car' is the
   variable value to display in place of the current value.
 
@@ -5383,11 +5378,34 @@ 'custom-icon
   :custom-category 'option
   :custom-state nil
   :custom-form nil
-  :value-create 'custom-icon-value-create
+  :value-create #'custom-icon-value-create
   :hidden-states '(standard)
-  :custom-set 'custom-icon-set
-  :custom-reset-current 'custom-redraw
-  :custom-reset-saved 'custom-variable-reset-saved)
+  :action #'custom-icon-action
+  :custom-set #'custom-icon-set
+  :custom-reset-current #'custom-redraw)
+  ;; Not implemented yet.
+  ;; :custom-reset-saved 'custom-icon-reset-saved)
+
+(defvar custom-icon-extended-menu
+  (let ((map (make-sparse-keymap)))
+    (define-key-after map [custom-icon-set]
+      '(menu-item "Set for Current Session" custom-icon-set
+                  :enable (eq (widget-get custom-actioned-widget :custom-state)
+                              'modified)))
+    (when (or custom-file init-file-user)
+      (define-key-after map [custom-icon-save]
+        '(menu-item "Save for Future Sessions" custom-icon-save
+                    :enable (memq
+                             (widget-get custom-actioned-widget :custom-state)
+                             '(modified set changed)))))
+    (define-key-after map [custom-redraw]
+      '(menu-item "Undo Edits" custom-redraw
+                  :enable (memq
+                           (widget-get custom-actioned-widget :custom-state)
+                           '(modified changed))))
+    map)
+  "A menu for `custom-icon' widgets.
+Used in `custom-icon-action' to show a menu to the user.")
 
 (defun custom-icon-value-create (widget)
   "Here is where you edit the icon's specification."
@@ -5517,6 +5535,24 @@ custom-icon-value-create
 	  (custom-add-parent-links widget))
 	(custom-add-see-also widget)))))
 
+(defun custom-icon-action (widget &optional event)
+  "Show the menu for `custom-icon' WIDGET.
+Optional EVENT is the location for the menu."
+  (if (eq (widget-get widget :custom-state) 'hidden)
+      (custom-toggle-hide widget)
+    (unless (eq (widget-get widget :custom-state) 'modified)
+      (custom-icon-state-set widget))
+    (custom-redraw-magic widget)
+    (let* ((completion-ignore-case t)
+           (custom-actioned-widget widget)
+           (answer (widget-choose (concat "Operation on "
+                                          (custom-unlispify-tag-name
+                                           (widget-get widget :value)))
+                                  custom-icon-extended-menu
+                                  event)))
+      (when answer
+        (funcall answer widget)))))
+
 (defun custom-toggle-hide-icon (visibility-widget &rest _ignore)
   "Toggle the visibility of a `custom-icon' parent widget.
 By default, this signals an error if the parent has unsaved
@@ -5553,10 +5589,21 @@ custom-icon-set
       (user-error "Cannot update hidden icon"))
 
     (setq val (custom--icons-widget-value child))
-    (unless (equal val (icon-complete-spec symbol))
-      (custom-variable-backup-value widget))
+    ;; FIXME: What was the intention here?
+    ;; (unless (equal val (icon-complete-spec symbol))
+    ;;   (custom-variable-backup-value widget))
     (custom-push-theme 'theme-icon symbol 'user 'set val)
-    (custom-redraw-magic widget)))
+    (custom-redraw widget)))
+
+(defun custom-icon-save (widget)
+  "Save value of icon edited by widget WIDGET."
+  (custom-set-icons (cons (widget-value widget)
+                          (list
+                           (custom--icons-widget-value
+                            (car (widget-get widget :children))))))
+  (custom-save-all)
+  (custom-icon-state-set widget)
+  (custom-redraw-magic widget))
 
 ;;;###autoload
 (defun customize-icon (icon)
diff --git a/lisp/emacs-lisp/icons.el b/lisp/emacs-lisp/icons.el
index cb08c1a6b81..9a6d26243c7 100644
--- a/lisp/emacs-lisp/icons.el
+++ b/lisp/emacs-lisp/icons.el
@@ -181,9 +181,9 @@ icons--merge-spec
         (let ((parent-keywords (icon-spec-keywords elem))
               (current-keywords (icon-spec-keywords current)))
           (while parent-keywords
-            (unless (plist-get (car parent-keywords) current-keywords)
-              (nconc current (take 2 parent-keywords))
-              (setq parent-keywords (cddr parent-keywords))))))))
+            (unless (plist-get current-keywords (car parent-keywords))
+              (nconc current (take 2 parent-keywords)))
+            (setq parent-keywords (cddr parent-keywords)))))))
   merged)
 
 (cl-defmethod icons--create ((_type (eql 'image)) icon keywords)
-- 
2.34.1