bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Drew Adams]

1. The `Revert...' menu items should be controled by either :enable or
:visible.  It makes no sense to show a user an active menu item `Undo
Edits in Customization Buffer' if there have been no edits.  After a
user clicks this, undoing any edits, this menu item should disappear or
be dimmed.  Similarly for other menu items.
 
2. `Apply' is not helpful as is.  The name does not sufficiently
indicate what it does (apply what? to what?).  The help echo is also not
helpful enough: `Apply settings (for current session only)'.  What
settings?  And again, if no settings have been changed (edited) then
this button should not even be available.  Its presence when it is a
no-op just confuses.
 
In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2013-05-10 on ODIEONE
Bzr revision: 112542 rgm@gnu.org-20130510102119-fklj7xlajezey0tr
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -IC:/Devel/emacs/build/include --ldflags -LC:/Devel/emacs/build/lib'
 

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Mauro Aranda]

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

tags 14398 patch
quit

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

> 1. The `Revert...' menu items should be controled by either :enable or
> :visible.  It makes no sense to show a user an active menu item `Undo
> Edits in Customization Buffer' if there have been no edits.  After a
> user clicks this, undoing any edits, this menu item should disappear or
> be dimmed.  Similarly for other menu items.

Now that widget-choose supports extended menus, this can be done like in
Bug#4787.

> 2. `Apply' is not helpful as is.  The name does not sufficiently
> indicate what it does (apply what? to what?).  The help echo is also not
> helpful enough: `Apply settings (for current session only)'.  What
> settings?

While "Apply" might not be helpful, I think that the help-echo
answers the questions.  And I'd say it is evident that it talks about
the settings the user is seeing in the Custom buffer at that moment.

> And again, if no settings have been changed (edited) then
> this button should not even be available.  Its presence when it is a
> no-op just confuses.

Makes sense to me.  And this can be easily controlled by activating and
deactivating the widgets.

I think that toolbar icons and menu bar items should also be
enabled/disabled according to the state of the options in the Custom
buffer, so I propose a patch that adds logic to do that as well.

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

[-- Attachment #2: 0001-Enable-disable-buttons-tool-bar-and-menu-items-in-Cu.patch --]
[-- Type: text/x-patch, Size: 11123 bytes --]

From 863abe4fcad5cb560ed51b6f0a82dc9e0dd58443 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Tue, 18 Aug 2020 12:50:17 -0300
Subject: [PATCH] Enable/disable buttons, tool bar and menu items in Custom
 buffer (Bug#14398)

* lisp/cus-edit.el (custom-reset-extended-menu): Keymap menu for the
Revert... menu button.
(custom-reset-menu): Keep for backward compatibility, but default to
nil, so we prefer the keymap menu instead.
(custom-reset): Pass the new keymap menu to widget-choose.

(custom-commands): Add an element to each list item, to manage its
enable/disable state.  Add docstring.
(custom-command-buttons): New variable, to hold the buttons that act
on all options in a Custom buffer.
(custom-buffer-create-internal): When creating the command buttons,
add the optional :notify function to enable/disable them.  Add the
buttons to the new variable custom-command-buttons.
(customize-menu-create): Add an :active form to the vector passed to
tool-bar-local-item-from-menu, so buttons can be enabled/disabled.
(custom-buffer-create): Notify the command buttons after creating the
Custom buffer, so they are correctly enabled/disabled.
(custom-redraw-magic, custom-notify): Notify the command buttons and
update the tool bar when changing a widget to the modified state.
---
 lisp/cus-edit.el | 148 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 110 insertions(+), 38 deletions(-)

diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index 34d76bf096..d1077d367d 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -730,48 +730,86 @@ custom-sort-items
 ;; `custom-buffer-create-internal' if `custom-buffer-verbose-help' is non-nil.
 
 (defvar custom-commands
-  '((" Apply " Custom-set t
-     "Apply settings (for the current session only)."
-     "index"
-     "Apply")
-    (" Apply and Save " Custom-save
-     (or custom-file user-init-file)
-     "Apply settings and save for future sessions."
-     "save"
-     "Save")
+  '((" Apply " Custom-set t "Apply settings (for the current session only)."
+     "index" "Apply" (modified))
+    (" Apply and Save " Custom-save (or custom-file user-init-file)
+     "Apply settings and save for future sessions." "save" "Save"
+     (modified set changed rogue))
     (" Undo Edits " Custom-reset-current t
      "Restore customization buffer to reflect existing settings."
-     "refresh"
-     "Undo")
+     "refresh" "Undo" (modified))
     (" Reset Customizations " Custom-reset-saved t
-     "Undo any settings applied only for the current session."
-     "undo"
-     "Reset")
+     "Undo any settings applied only for the current session." "undo" "Reset"
+     (modified set changed rogue))
     (" Erase Customizations " Custom-reset-standard
      (or custom-file user-init-file)
-     "Un-customize settings in this and future sessions."
-     "delete"
-     "Uncustomize")
-    (" Help for Customize " Custom-help t
-     "Get help for using Customize."
-     "help"
-     "Help")
-    (" Exit " Custom-buffer-done t "Exit Customize." "exit" "Exit")))
+     "Un-customize settings in this and future sessions." "delete" "Uncustomize"
+     (modified set changed rogue saved))
+    (" Help for Customize " Custom-help t "Get help for using Customize."
+     "help" "Help" t)
+    (" Exit " Custom-buffer-done t "Exit Customize." "exit" "Exit" t))
+  "Alist of specifications for Customize menu items, tool bar icons and buttons.
+Each member has the format (TAG COMMAND VISIBLE HELP ICON LABEL ENABLE).
+TAG is a string, used as the :tag property of a widget.
+COMMAND is the command that the item or button runs.
+VISIBLE should be a form, suitable to pass as the :visible property for menu
+or tool bar items.
+HELP should be a string that can be used as the help echo property for tooltips
+and the like.
+ICON is a string that names the image to use for the tool bar item, like in the
+first argument of `tool-bar-local-item'.
+LABEL should be a string, used as the name of the menu items.
+ENABLE should be a list of custom states or t.  When ENABLE is t, the item is
+always enabled.  Otherwise, it is enabled only if at least one option displayed
+in the Custom buffer is in a state present in ENABLE.")
+
+(defvar-local custom-command-buttons nil
+  "A list that holds the buttons that act on all settings in a Custom buffer.
+`custom-buffer-create-internal' adds the buttons to this list.
+Changes in the state of the custom options should notify the buttons via the
+:notify property, so buttons can be enabled/disabled correctly at all times.")
 
 (defun Custom-help ()
   "Read the node on Easy Customization in the Emacs manual."
   (interactive)
   (info "(emacs)Easy Customization"))
 
-(defvar custom-reset-menu
-  '(("Undo Edits in Customization Buffer" . Custom-reset-current)
-    ("Revert This Session's Customizations" . Custom-reset-saved)
-    ("Erase Customizations" . Custom-reset-standard))
-  "Alist of actions for the `Reset' button.
+(defvar custom-reset-menu nil
+  "If non-nil, an alist of actions for the `Reset' button.
+
+This variable is kept for backward compatibility reasons, please use
+`custom-reset-extended-menu' instead.
+
 The key is a string containing the name of the action, the value is a
 Lisp function taking the widget as an element which will be called
 when the action is chosen.")
 
+(defvar custom-reset-extended-menu
+  (let ((map (make-sparse-keymap)))
+    (define-key-after map [Custom-reset-current]
+      '(menu-item "Undo Edits in Customization Buffer" Custom-reset-current
+                  :enable (seq-some (lambda (option)
+                                      (eq (widget-get option :custom-state)
+                                          'modified))
+                                    custom-options)))
+    (define-key-after map [Custom-reset-saved]
+      '(menu-item "Revert This Session's Customizations" Custom-reset-saved
+                  :enable (seq-some (lambda (option)
+                                      (memq (widget-get option :custom-state)
+                                          '(modified set changed rogue)))
+                                    custom-options)))
+    (when (or custom-file user-init-file)
+      (define-key-after map [Custom-reset-standard]
+        '(menu-item "Erase Customizations" Custom-reset-standard
+                    :enable (seq-some
+                             (lambda (option)
+                               (memq (widget-get option :custom-state)
+                                     '(modified set changed rogue saved)))
+                             custom-options))))
+    map)
+  "A menu for the \"Revert...\" button.
+Used in `custom-reset' to show a menu to the user.")
+
 (defvar custom-options nil
   "Customization widgets in the current buffer.")
 
@@ -821,7 +859,8 @@ custom-reset
   "Select item from reset menu."
   (let* ((completion-ignore-case t)
 	 (answer (widget-choose "Reset settings"
-				custom-reset-menu
+                                (or custom-reset-menu
+                                    custom-reset-extended-menu)
 				event)))
     (if answer
 	(funcall answer))))
@@ -1555,7 +1594,10 @@ custom-buffer-create
 DESCRIPTION is unused."
   (pop-to-buffer-same-window
    (custom-get-fresh-buffer (or name "*Customization*")))
-  (custom-buffer-create-internal options))
+  (custom-buffer-create-internal options)
+  ;; Notify the command buttons, to correctly enable/disable them.
+  (dolist (btn custom-command-buttons)
+    (widget-apply btn :notify)))
 
 ;;;###autoload
 (defun custom-buffer-create-other-window (options &optional name _description)
@@ -1672,11 +1714,24 @@ custom-buffer-create-internal
     (if custom-buffer-verbose-help
 	(widget-insert "
 Operate on all settings in this buffer:\n"))
-    (let ((button (lambda (tag action active help _icon _label)
+    (let ((button (lambda (tag action visible help _icon _label active)
 		    (widget-insert " ")
-		    (if (eval active)
-			(widget-create 'push-button :tag tag
-				       :help-echo help :action action))))
+                    (if (eval visible)
+                        (push (widget-create
+                               'push-button :tag tag
+                               :help-echo help :action action
+                               :notify
+                               (lambda (widget)
+                                 (when (listp active)
+                                   (if (seq-some
+                                        (lambda (widget)
+                                          (memq
+                                           (widget-get widget :custom-state)
+                                           active))
+                                        custom-options)
+                                       (widget-apply widget :activate)
+                                     (widget-apply widget :deactivate)))))
+                              custom-command-buttons))))
 	  (commands custom-commands))
       (if custom-reset-button-menu
 	  (progn
@@ -2215,7 +2270,11 @@ custom-notify
   (let ((state (widget-get widget :custom-state)))
     (unless (eq state 'modified)
       (unless (memq state '(nil unknown hidden))
-	(widget-put widget :custom-state 'modified))
+	(widget-put widget :custom-state 'modified)
+        ;; Tell our buttons and the tool bar that we changed the widget's state.
+        (force-mode-line-update)
+        (dolist (btn custom-command-buttons)
+          (widget-apply btn :notify)))
       ;; Update the status text (usually from "STANDARD" to "EDITED
       ;; bla bla" in the buffer after the command has run.  Otherwise
       ;; commands like `M-u' (that work on a region in the buffer)
@@ -2254,7 +2313,10 @@ custom-redraw-magic
 	       (custom-group-state-update widget)))
 	    (t
 	     (setq widget nil)))))
-  (widget-setup))
+  (widget-setup)
+  (force-mode-line-update)
+  (dolist (btn custom-command-buttons)
+    (widget-apply btn :notify)))
 
 (defun custom-show (widget value)
   "Non-nil if WIDGET should be shown with VALUE by default."
@@ -4945,9 +5007,19 @@ customize-menu-create
 	 (mapcar (lambda (arg)
 		   (let ((tag     (nth 0 arg))
 			 (command (nth 1 arg))
-			 (active  (nth 2 arg))
-			 (help    (nth 3 arg)))
-		     (vector tag command :active (eval active) :help help)))
+                         (visible (nth 2 arg))
+                         (help    (nth 3 arg))
+                         (active  (nth 6 arg)))
+                     (vector tag command :visible (eval visible)
+                             :active
+                             `(or (eq t ',active)
+                                  (seq-some ,(lambda (widget)
+                                               (memq
+                                                (widget-get widget
+                                                            :custom-state)
+                                                active))
+                                            custom-options))
+                             :help help)))
 		 custom-commands)))
 
 (defvar tool-bar-map)
-- 
2.29.2

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I think that toolbar icons and menu bar items should also be
> enabled/disabled according to the state of the options in the Custom
> buffer, so I propose a patch that adds logic to do that as well.

I haven't tried the patch, but this makes sense to me.

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

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I think that toolbar icons and menu bar items should also be
>> enabled/disabled according to the state of the options in the Custom
>> buffer, so I propose a patch that adds logic to do that as well.
>
> I haven't tried the patch, but this makes sense to me.

Thanks.  Should I wait for further comments, or is it OK to push and see
if anything comes up? I mean, I have tested the code quite a bit, but
perhaps I should wait for comments about the code itself?

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

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Drew Adams]

Haven't tried it, but the patch looks great.

I especially appreciate the fact that you adding
a missing doc string.  Thx.
___

FWIW -

I'll incorporate those changes into my library
`cus-edit+.el', which adds these three items to
`custom-commands': `Consider Unchanged', `Ignore
Unsaved Changes', and `Set from External Changes'.

The help strings for those 3 are:

"Treat changed preferences as if they were
unchanged, without saving them"

"Add to the `customize-unsaved-ignore' preferences,
whose changes are ignored by `customize-unsaved'"

"Tell Customize that all preferences changed
outside it are now set"

(Tried to interest Emacs in these, to no avail.)

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Thanks.  Should I wait for further comments, or is it OK to push and see
> if anything comes up? I mean, I have tested the code quite a bit, but
> perhaps I should wait for comments about the code itself?

The code looked fine to me, so just go ahead and push.

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

bug#14398: 24.3.50; Customize: `Revert...' and `Apply'

[Mauro Aranda]

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

tags 14398 fixed
close 14398 28.1
quit

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Thanks.  Should I wait for further comments, or is it OK to push and see
>> if anything comes up? I mean, I have tested the code quite a bit, but
>> perhaps I should wait for comments about the code itself?
>
> The code looked fine to me, so just go ahead and push.

Great, thanks.  I've now pushed, and I'm closing this bug report.

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