bug#12864: 24.3.50; bad & confusing Customize `State' Menu behavior

[Mauro Aranda <maurooaranda@gmail.com>]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I'm not sure how to proceed here: should I send different patches for
>> the bugs, or put the fixes altogether in one patch?
>
> It's really up to what you feel makes more sense: if they are closely
> related, you might prefer sending them in one patch; otherwise, it might
> be better to send the changes in separate patches.
>

Hello Stefan,

Thanks for your answer.

I went with a single patch.  If after seeing the patch someone feels it
should be split, just let me know.

Please review, and thanks.

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

[-- Attachment #2: 0001-Make-State-button-interaction-less-confusing-Bug-128.patch --]
[-- Type: text/x-patch, Size: 6439 bytes --]

From f2e900d06005541a6084482d07bb014f9251133c Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 1 Sep 2020 17:08:32 -0300
Subject: [PATCH] Make State button interaction less confusing (Bug#12864)

* lisp/cus-edit.el (custom-variable-current-value): New function.
(custom-variable-backup-value): Use it.
(custom-variable-set, custom-variable-mark-to-reset-standard): Check
that old value is different than the new one.  If it is, make a
backup.  This way, we avoid offering the Set to Backup Value
unnecesarily.
(custom-variable-reset-saved): Reset the variable-comment property for
the variable, to help custom-variable-state be more correct.  Also
check if we should backup old value.
(custom-variable-state): If a variable was set to the standard value,
say its state is standard rather than set, which is more correct.
Getting the right variable state is important for menu options to be
enabled/disabled, and for displaying the right message to the user.
---
 lisp/cus-edit.el | 59 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 23ceb3a857..164e865143 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -2789,7 +2789,9 @@ custom-variable-state
 		   (and (equal value (eval (car tmp)))
 			(equal comment temp))
 		 (error nil))
-	       'set
+               (if (equal value (eval (car (get symbol 'standard-value))))
+                   'standard
+	         'set)
 	     'changed))
 	  ((progn (setq tmp (get symbol 'theme-value))
 		  (setq temp (get symbol 'saved-variable-comment))
@@ -2859,6 +2861,18 @@ custom-variable-state-set
 (defun custom-variable-standard-value (widget)
   (get (widget-value widget) 'standard-value))
 
+(defun custom-variable-current-value (widget)
+  "Return the current value of the variable edited by WIDGET.
+
+WIDGET should be a custom-variable widget."
+  (let* ((symbol (widget-value widget))
+         (get (or (get symbol 'custom-get) 'default-value))
+         (type (custom-variable-type symbol))
+         (conv (widget-convert type)))
+    (if (default-boundp symbol)
+        (funcall get symbol)
+      (widget-get conv :value))))
+
 (defvar custom-variable-menu
   `(("Set for Current Session" custom-variable-set
      (lambda (widget)
@@ -2956,10 +2970,12 @@ custom-variable-set
 	     (setq comment nil)
 	     ;; Make the comment invisible by hand if it's empty
 	     (custom-comment-hide comment-widget))
-	   (custom-variable-backup-value widget)
+           (setq val (widget-value child))
+           (unless (equal (eval val) (custom-variable-current-value widget))
+	     (custom-variable-backup-value widget))
 	   (custom-push-theme 'theme-value symbol 'user
-			      'set (custom-quote (widget-value child)))
-	   (funcall set symbol (eval (setq val (widget-value child))))
+                              'set (custom-quote val))
+	   (funcall set symbol (eval val))
 	   (put symbol 'customized-value (list val))
 	   (put symbol 'variable-comment comment)
 	   (put symbol 'customized-variable-comment comment))
@@ -2968,10 +2984,12 @@ custom-variable-set
 	     (setq comment nil)
 	     ;; Make the comment invisible by hand if it's empty
 	     (custom-comment-hide comment-widget))
-	   (custom-variable-backup-value widget)
+           (setq val (widget-value child))
+           (unless (equal val (custom-variable-current-value widget))
+	     (custom-variable-backup-value widget))
 	   (custom-push-theme 'theme-value symbol 'user
-			      'set (custom-quote (widget-value child)))
-	   (funcall set symbol (setq val (widget-value child)))
+			      'set (custom-quote val))
+	   (funcall set symbol val)
 	   (put symbol 'customized-value (list (custom-quote val)))
 	   (put symbol 'variable-comment comment)
 	   (put symbol 'customized-variable-comment comment)))
@@ -3040,17 +3058,23 @@ custom-variable-reset-saved
   (let* ((symbol (widget-value widget))
 	 (saved-value (get symbol 'saved-value))
 	 (comment (get symbol 'saved-variable-comment))
+         (old-value (custom-variable-current-value widget))
          value)
-    (custom-variable-backup-value widget)
     (if (not (or saved-value comment))
-        ;; If there is no saved value, remove the setting.
-        (custom-push-theme 'theme-value symbol 'user 'reset)
+        (progn
+          (setq value (car (get symbol 'standard-value)))
+          ;; If there is no saved value, remove the setting.
+          (custom-push-theme 'theme-value symbol 'user 'reset)
+          ;; And reset this property too.
+          (put symbol 'variable-comment nil))
       (setq value (car-safe saved-value))
       (custom-push-theme 'theme-value symbol 'user 'set value)
       (put symbol 'variable-comment comment))
+    (unless (equal (eval value) old-value)
+      (custom-variable-backup-value widget))
     (ignore-errors
       (funcall (or (get symbol 'custom-set) #'set-default) symbol
-               (eval (or value (car (get symbol 'standard-value))))))
+               (eval value)))
     (put symbol 'customized-value nil)
     (put symbol 'customized-variable-comment nil)
     (widget-put widget :custom-state 'unknown)
@@ -3063,7 +3087,9 @@ custom-variable-mark-to-reset-standard
 redraw the widget immediately."
   (let* ((symbol (widget-value widget)))
     (if (get symbol 'standard-value)
-	(custom-variable-backup-value widget)
+	(unless (equal (custom-variable-current-value widget)
+                       (eval (car (get symbol 'standard-value))))
+          (custom-variable-backup-value widget))
       (user-error "No standard setting known for %S" symbol))
     (put symbol 'variable-comment nil)
     (put symbol 'customized-value nil)
@@ -3100,13 +3126,8 @@ custom-variable-reset-standard
 (defun custom-variable-backup-value (widget)
   "Back up the current value for WIDGET's variable.
 The backup value is kept in the car of the `backup-value' property."
-  (let* ((symbol (widget-value widget))
-	 (get (or (get symbol 'custom-get) 'default-value))
-	 (type (custom-variable-type symbol))
-	 (conv (widget-convert type))
-	 (value (if (default-boundp symbol)
-		    (funcall get symbol)
-		  (widget-get conv :value))))
+  (let ((symbol (widget-value widget))
+	(value (custom-variable-current-value widget)))
     (put symbol 'backup-value (list value))))
 
 (defun custom-variable-reset-backup (widget)
-- 
2.28.0