GNU bug report logs - #69079
[PATCH] Add 'customize-toggle-option' command

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69079 in the body.
You can then email your comments to 69079 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add 'customize-toggle-option' command
Date: Mon, 12 Feb 2024 17:32:37 +0000

[Message part 1 (text/plain, inline)]

I have had this option in my own init.el for a while, and think it would
be nice to upstream it.  Any comments:

[0001-Add-'custom-variable'-command.patch (text/x-patch, attachment)]

Message #8 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Mon, 12 Feb 2024 19:47:57 +0200

> From: Philip Kaludercic <philipk <at> posteo.net>
> 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?

And the prompt should IMO say "Toggle boolean option: ".

Message #11 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Philip Kaludercic <philipk <at> posteo.net>
Cc: "69079 <at> debbugs.gnu.org" <69079 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#69079: [PATCH] Add 'customize-toggle-option'
 command
Date: Mon, 12 Feb 2024 18:41:39 +0000

FWIW, (since 2006) Icicles has this, which is bound
to `M-i M-i` during completion.

With a prefix arg you can toggle options and other
variables whose values are generalized Booleans:
`nil' or non-`nil' (not just `t').

This is for toggling an option's current value; it
does only this: (set SYMBOL (not (eval SYMBOL))).
But the completion predicate determines the proper
candidates (depending on prefix arg).

There are (rightfully) many options whose values
are `nil' for false and non-`nil' for true.  If the
command didn't let you toggle such options (with a
prefix arg) then it would be _far_ less useful.
___

Doc string:

Toggle option's value.  This makes sense for binary (toggle) options.
By default, completion candidates are limited to user options that
have `boolean' custom types.  However, there are many "binary" options
that allow other non-nil values than t.

You can use a prefix argument to change the set of completion
candidates, as follows:

 - With a non-negative prefix arg, all user options are candidates.
 - With a negative prefix arg, all variables are candidates.

Message #14 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Mon, 12 Feb 2024 18:56:39 +0000

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> 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?

> And the prompt should IMO say "Toggle boolean option: ".

Good point.

Message #17 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Mon, 12 Feb 2024 21:32:51 +0200

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: 69079 <at> debbugs.gnu.org
> Date: Mon, 12 Feb 2024 18:56:39 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk <at> posteo.net>
> >> 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?

Message #20 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Tue, 13 Feb 2024 00:14:38 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 69079 <at> debbugs.gnu.org
>> Date: Mon, 12 Feb 2024 18:56:39 +0000
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Philip Kaludercic <philipk <at> posteo.net>
>> >> 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?

No, that was not ensured, and I think it is better not to.  I have
adjusted the patch to check and prompt the user if the user option is
non-boolean, in case they know what they are doing.  WDYT?

[0001-Add-custom-variable-command.patch (text/x-patch, attachment)]

Message #23 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Tue, 13 Feb 2024 14:40:49 +0200

> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: 69079 <at> debbugs.gnu.org
> Date: Tue, 13 Feb 2024 00:14:38 +0000
> 
> >> 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?
> 
> No, that was not ensured, and I think it is better not to.  I have
> adjusted the patch to check and prompt the user if the user option is
> non-boolean, in case they know what they are doing.  WDYT?

LGTM, although I haven't tried to actually use the code.

Thanks.

Message #26 received at 69079 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, 69079 <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Tue, 13 Feb 2024 13:46:28 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: 69079 <at> debbugs.gnu.org
>> Date: Tue, 13 Feb 2024 00:14:38 +0000
>>
>> >> 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?
>>
>> No, that was not ensured, and I think it is better not to.  I have
>> adjusted the patch to check and prompt the user if the user option is
>> non-boolean, in case they know what they are doing.  WDYT?
>
> LGTM, although I haven't tried to actually use the code.
>
> Thanks.

FWIW, I think it'd be nice to use the as the default minibuffer argument
symbol at point, if applicable.

Message #31 received at 69079-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 69079-done <at> debbugs.gnu.org
Subject: Re: bug#69079: [PATCH] Add 'customize-toggle-option' command
Date: Tue, 13 Feb 2024 20:09:46 +0000

Eshel Yaron <me <at> eshelyaron.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>> Cc: 69079 <at> debbugs.gnu.org
>>> Date: Tue, 13 Feb 2024 00:14:38 +0000
>>>
>>> >> 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?
>>>
>>> No, that was not ensured, and I think it is better not to.  I have
>>> adjusted the patch to check and prompt the user if the user option is
>>> non-boolean, in case they know what they are doing.  WDYT?
>>
>> LGTM, although I haven't tried to actually use the code.
>>
>> Thanks.
>
> FWIW, I think it'd be nice to use the as the default minibuffer argument
> symbol at point, if applicable.

Done and pushed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.