GNU bug report logs - #6542
24.0.50; menu-bar-make-toggle

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 6542 in the body.
You can then email your comments to 6542 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: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.0.50; menu-bar-make-toggle
Date: Wed, 30 Jun 2010 10:46:04 -0700

1. Please a doc string.  In particular, the use of BODY is not obvious.
 
2. Please add the ability to use additional keywords, such as :keys,
:enable, and :visible.
 
Consider, for example, being able to list an option that only makes
sense to toggle when some minor mode is enabled.  You would want to be
able to specify `:enable foo-mode' or `:visible foo-mode', where
`foo-mode' is the minor-mode variable.

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-06-21 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags -Ic:/xpm/include'
 

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 17954 <at> debbugs.gnu.org, 6542 <at> debbugs.gnu.org
Subject: Re: bug#17954: 24.4.50;
 [patch] `menu-bar-make-toggle' should allow for keywords
Date: Thu, 28 Apr 2016 00:05:35 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> It cannot accept keywords in the normal way because it defines BODY as
> an &rest parameter.  This mistake should be corrected, making for an
> incompatible change unfortunately, in order to allow a KEYWORDS &rest
> parameter.
>
> BODY is seldom used.  In the Emacs Lisp sources, there are only these
> two occurrences of `menu-bar-make-toggle' that use BODY:
> `toggle-save-place-globally' and `toggle-uniquify-buffer-names'.  They
> would just need to wrap the current BODY code in an explicit `progn'.

I don't think such an incompatible change would be appropriate.

> Of course, there may be 3rd-party code that uses parameter BODY.  The
> only way I can think of to be sensitive to that breakage is to use a new
> macro name and code, and deprecate the old name `menu-bar-make-toggle'.
> Perhaps this incompatible change could be tolerated, since BODY is rare
> and the adaptation for any existing code is trivial (wrap with `progn').

And having two macros that do the same wouldn't be very appropriate,
either...

I think it would make more sense to extend the HELP parameter to be a
list, and then it could be the keyword list.

Your code as a patch included below.

diff --git a/lisp/menu-bar.el b/lisp/menu-bar.el
index cc7233e..ca40e94 100644
--- a/lisp/menu-bar.el
+++ b/lisp/menu-bar.el
@@ -629,31 +629,44 @@ menu-bar-make-mm-toggle
 	       :button (:toggle . (and (default-boundp ',fname)
 				       (default-value ',fname)))))
 
-(defmacro menu-bar-make-toggle (name variable doc message help &rest body)
+(defmacro menu-bar-make-toggle (command variable item-name message help
+                                        &optional setting-sexp &rest keywords)
+  "Define a menu-bar toggle command.
+COMMAND (a symbol) is the toggle command to define.
+VARIABLE (a symbol) is the variable to set.
+ITEM-NAME (a string) is the menu-item name.
+MESSAGE is a format string for the toggle message, with %s for the new
+ status.
+HELP (a string) is the `:help' tooltip text and the doc string first
+ line (minus final period) for the command.
+SETTING-SEXP is a Lisp sexp that sets VARIABLE, or it is nil meaning
+ set it according to its `defcustom' or using `set-default'.
+KEYWORDS is a plist for `menu-item' for keywords other than `:help'."
   `(progn
-     (defun ,name (&optional interactively)
+     (defun ,command (&optional interactively)
        ,(concat "Toggle whether to " (downcase (substring help 0 1))
-		(substring help 1) ".
+                (substring help 1) ".
 In an interactive call, record this option as a candidate for saving
 by \"Save Options\" in Custom buffers.")
        (interactive "p")
-       (if ,(if body `(progn . ,body)
-	      `(progn
+       (if ,(if setting-sexp
+                `,setting-sexp
+              `(progn
 		 (custom-load-symbol ',variable)
 		 (let ((set (or (get ',variable 'custom-set) 'set-default))
 		       (get (or (get ',variable 'custom-get) 'default-value)))
 		   (funcall set ',variable (not (funcall get ',variable))))))
-	   (message ,message "enabled globally")
-  	 (message ,message "disabled globally"))
-       ;; The function `customize-mark-as-set' must only be called when
-       ;; a variable is set interactively, as the purpose is to mark it as
-       ;; a candidate for "Save Options", and we do not want to save options
-       ;; the user have already set explicitly in his init file.
-       (if interactively (customize-mark-as-set ',variable)))
-     '(menu-item ,doc ,name
-		 :help ,help
-		 :button (:toggle . (and (default-boundp ',variable)
-					 (default-value ',variable))))))
+           (message ,message "enabled globally")
+         (message ,message "disabled globally"))
+       ;; `customize-mark-as-set' must only be called when a variable is set
+       ;; interactively, because the purpose is to mark the variable as a
+       ;; candidate for `Save Options', and we do not want to save options that
+       ;; the user has already set explicitly in the init file.
+       (when interactively (customize-mark-as-set ',variable)))
+     '(menu-item ,item-name ,command :help ,help
+                 :button (:toggle . (and (default-boundp ',variable)
+                                         (default-value ',variable)))
+                 ,@keywords)))
 
 ;; Function for setting/saving default font.
 

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

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 17954 <at> debbugs.gnu.org, 6542 <at> debbugs.gnu.org
Subject: Re: bug#17954: 24.4.50; [patch] `menu-bar-make-toggle' should allow
 for keywords
Date: Sat, 19 Sep 2020 18:24:37 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> `menu-bar-make-toggle' handles no keywords.  It takes the value of
> :help as an explicit argument (without keyword), but it does not pass
> along :keys, :active, :enable, etc. to `menu-item'.  It should.
>
> It cannot accept keywords in the normal way because it defines BODY as
> an &rest parameter.  This mistake should be corrected, making for an
> incompatible change unfortunately, in order to allow a KEYWORDS &rest
> parameter.
>
> BODY is seldom used.  In the Emacs Lisp sources, there are only these
> two occurrences of `menu-bar-make-toggle' that use BODY:
> `toggle-save-place-globally' and `toggle-uniquify-buffer-names'.  They
> would just need to wrap the current BODY code in an explicit `progn'.

I've now applied your patch, but renamed the resulting macro to
menu-bar-make-toggle-command, while the former name is now an obsolete
wrapper macro.  I've also adjusted all in-tree callers.

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

Previous Next

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