Subject: Re: bug#65708: 30.0.50; Wrong default value for functions in filesets.el

From: Mauro Aranda <[email-masked]>
Date: Sat, 2 Sep 2023 20:19:01 -0300

On 2/9/23 19:46, Mauro Aranda wrote:
 > On 2/9/23 19:42, Stefan Kangas wrote:
 >  > Mauro Aranda <[email-masked]> writes:
 >  >
 >  >> The following user options:
 >  >> filesets-browse-dir-function
 >  >> filesets-open-file-function
 >  >> filesets-save-buffer-function
 >  >> filesets-commands
 >  >> filesets-external-viewers
 >  >>
 >  >> have overridden the default value of the function widget to the value
 >  >> nil.  That's an invalid default value.  It'd be better to just remove
 >  >> that.
 >  >
 >  > Makes sense, but you forgot to attach the patch.  ;-)
 >
 > I was writing it but then I found some other problems with the
 > defcustoms in filesets.el :-(
 >
 > I'll attach the patch trying to fix all of them (with a description of
 > the problems, of course).

OK, I found a couple of bugs.  To reproduce, with emacs -Q
(require 'filesets)
M-x customize-option RET filesets-commands
Move to the last INS button, the one that inserts a brand new command.
Hit RET:

INS DEL Definition:
             String:
             Command: Value Menu invalid (nil)
             Argument List:
             INS

invalid (nil) indicates a bad default value for the choice widget.  I
fixed that.

M-x customize-option RET filesets-external-viewers
Move to the INS button that inserts a Property for the last Definition.
Hit RET
INS DEL Definition:
             Pattern: .\.\(tiff\|xpm\|gif\|pgn\)\'
             Viewer: Value Menu Program: gqview
             Properties:
             INS DEL Choice: Value Menu :ignore-on-open-all:
                         Boolean: Toggle  on (non-nil)
             INS DEL Choice: Value Menu :ignore-on-read-text:
                         Boolean: Toggle  on (non-nil)
             INS DEL Choice: Value Menu invalid ((:constraintp))
             INS

invalid ((:constraintp)) indicates another bad default value.  I fixed
that, and other default values for the list widgets in this option.

One thing I observed is that there are lot of redundant tags in this
file, like (function :tag "Function").  But anyway, those aren't bugs,
so I didn't bothered.  I attach a patch for the emacs-29 branch.
From f2a2d4f17c1e7e93c8fc39004cb4b0246dae5e74 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Sat, 2 Sep 2023 20:16:30 -0300
Subject: [PATCH] Fix defcustoms in filesets.el (Bug#65708)

* lisp/filesets.el (filesets-browse-dir-function)
(filesets-open-file-function filesets-save-buffer-function)
(filesets-commands filesets-external-viewers): Don't override default
value for the function widget with an invalid default value.
Remove redundant ":" in tag.
Fix default values for choice and list widgets.
---
 lisp/filesets.el | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/lisp/filesets.el b/lisp/filesets.el
index 1b7e6ffa81f..c0ff1a3b6a9 100644
--- a/lisp/filesets.el
+++ b/lisp/filesets.el
@@ -413,15 +413,14 @@ filesets-browse-dir-function
 
 Note: You have to manually rebuild the menu if you change this value."
   :set #'filesets-set-default
-  :type '(choice :tag "Function:"
+  :type '(choice :tag "Function"
 		 (const :tag "dired"
 			:value dired)
 		 (list :tag "Command"
 		       :value ("" "%s")
 		       (string :tag "Name")
 		       (string :tag "Arguments"))
-		 (function :tag "Function"
-			   :value nil)))
+                 (function :tag "Function")))
 
 (defcustom filesets-open-file-function #'filesets-find-or-display-file
   "The function used for opening files.
@@ -437,23 +436,21 @@ filesets-open-file-function
 
 Caveat: Changes will take effect only after rebuilding the menu."
   :set #'filesets-set-default
-  :type '(choice :tag "Function:"
+  :type '(choice :tag "Function"
 		 (const :tag "filesets-find-or-display-file"
 			:value filesets-find-or-display-file)
 		 (const :tag "filesets-find-file"
 			:value filesets-find-file)
-		 (function :tag "Function"
-			   :value nil)))
+                 (function :tag "Function")))
 
 (defcustom filesets-save-buffer-function #'save-buffer
   "The function used to save a buffer.
 Caveat: Changes will take effect after rebuilding the menu."
   :set #'filesets-set-default
-  :type '(choice :tag "Function:"
+  :type '(choice :tag "Function"
 		 (const :tag "save-buffer"
 			:value save-buffer)
-		 (function :tag "Function"
-			   :value nil)))
+                 (function :tag "Function")))
 
 (defcustom filesets-find-file-delay
   (if (and (featurep 'xemacs) gutter-buffers-tab-visible-p)
@@ -535,7 +532,7 @@ filesets-commands
   :type '(repeat :tag "Commands"
 		 (list :tag "Definition" :value ("")
 		       (string "Name")
-		       (choice :tag "Command"
+                       (choice :tag "Command" :value ""
 			       (string :tag "String")
 			       (function :tag "Function"))
 		       (repeat :tag "Argument List"
@@ -546,8 +543,7 @@ filesets-commands
 					       :value "<file-name>")
 				       (string :tag "Quoted File Name"
 					       :value "<<file-name>>")
-				       (function :tag "Function"
-                                                 :value nil))))))
+                                       (function :tag "Function"))))))
 
 (defcustom filesets-external-viewers
   (let
@@ -647,12 +643,12 @@ filesets-external-viewers
 		       (repeat :tag "Properties"
 			       (choice
 				(list :tag ":constraintp"
-				      :value (:constraintp)
+                                      :value (:constraintp ignore)
 				      (const :format ""
 					     :value :constraintp)
 				      (function :tag "Function"))
 				(list :tag ":constraint-flag (obsolete)"
-				      :value (:constraint-flag)
+                                      :value (:constraint-flag nil)
 				      (const :format ""
 					     :value :constraint-flag)
 				      (sexp :tag "Symbol"))
@@ -667,7 +663,7 @@ filesets-external-viewers
 					      :value :ignore-on-read-text)
 				      (boolean :tag "Boolean"))
 				(list :tag ":args"
-				      :value (:args)
+                                      :value (:args nil)
 				      (const :format ""
 					     :value :args)
 				      (repeat :tag "List"
@@ -676,10 +672,9 @@ filesets-external-viewers
 							      :value "")
 						      (symbol :tag "Symbol"
 							      :value nil)
-						      (function :tag "Function"
-								:value nil))))
+                                                      (function :tag "Function"))))
 				(list :tag ":open-hook"
-				      :value (:open-hook)
+                                      :value (:open-hook nil)
 				      (const :format ""
 					     :value :open-hook)
 				      (hook :tag "Hook"))
-- 
2.34.1