GNU bug report logs - #17954
24.4.50; [patch] `menu-bar-make-toggle' should allow for keywords

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 17954 in the body.
You can then email your comments to 17954 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.4.50; [patch] `menu-bar-make-toggle' should allow for keywords
Date: Sat, 5 Jul 2014 22:28:37 -0700 (PDT)

`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'.

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').

Here is one possible definition (with a doc string and better parameter
names):

(defmacro bmkp-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 ,command (&optional interactively)
      ,(concat "Toggle whether to " (downcase (substring help 0 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 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"))
      ;; `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)))


In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-06-28 on ODIEONE
Bzr revision: 117431 rgm <at> gnu.org-20140628015517-eku6hj8mpgcvfnso
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs 'CFLAGS=-O0 -g3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

Message #12 received at 17954 <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 #15 received at 17954 <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.