Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command

From: Eli Zaretskii <[email-masked]>
Date: Mon, 12 Feb 2024 21:32:51 +0200

> From: Philip Kaludercic <[email-masked]>
> Cc: [email-masked]
> Date: Mon, 12 Feb 2024 18:56:39 +0000
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> From: Philip Kaludercic <[email-masked]>
> >> Date: Mon, 12 Feb 2024 17:32:37 +0000
> >> 
> >> +;;;###autoload
> >> +(defun customize-toggle-option (opt)
> >> +  "Toggle the value of boolean option OPT for this session."
> >> +  (interactive (let (opts)
> >> +		 (mapatoms
> >> +		  (lambda (sym)
> >> +		    (when (eq (get sym 'custom-type) 'boolean)
> >> +		      (push sym opts))))
> >> +		 (list (intern (completing-read "Option: " opts)))))
> >> +  (message "%s user options '%s'."
> >> +	   (if (funcall (or (get opt 'custom-set) #'set-default)
> >> +			opt (not (funcall (or (get opt 'custom-get)
> >> +					      #'symbol-value)
> >> +					  opt)))
> >> +	       "Enabled" "Disabled")
> >> +	   opt))
> >
> > Shouldn't this have some validation? what if the argument OPT is not a
> > boolean?
> 
> My assumption was that the command would only be invoked interactivly,
> so I can either make that explicit with an `interactive-only' or repeat
> the check.  What do you think would be better?

I think an explicit test is better, since then we get to display a
user-friendly error message, instead of relying on Lisp errors to
explain themselves.

Btw, are you sure that the users can never succeed in inputting a
non-boolean option with the way you prompt them?