From 6bec60ad3151825c2ee5f775848ea3d4c70c72a5 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Mon, 12 Apr 2021 11:08:19 -0400 Subject: [PATCH] (define-minor-mode): Warn about use of pre-Emacs-21 style args * lisp/emacs-lisp/easy-mmode.el (define-minor-mode): Use `advertised-calling-convention` to avoid promoting the old style arguments. Emit a wanring when old-style arguments are used. Massage the docstring accordingly. * doc/lispref/modes.texi (Defining Minor Modes): Document the keyword arguments rather than the old-style positional arguments. --- doc/lispref/modes.texi | 64 +++++------ etc/NEWS | 5 + lisp/emacs-lisp/easy-mmode.el | 197 +++++++++++++++++----------------- 3 files changed, 128 insertions(+), 138 deletions(-) diff --git a/doc/lispref/modes.texi b/doc/lispref/modes.texi index 6cf4dd21c19..88f2f14c092 100644 --- a/doc/lispref/modes.texi +++ b/doc/lispref/modes.texi @@ -1660,7 +1660,7 @@ reserved for users. @xref{Key Binding Conventions}. The macro @code{define-minor-mode} offers a convenient way of implementing a mode in one self-contained definition. -@defmac define-minor-mode mode doc [init-value [lighter [keymap]]] keyword-args@dots{} body@dots{} +@defmac define-minor-mode mode doc keyword-args@dots{} body@dots{} This macro defines a new minor mode whose name is @var{mode} (a symbol). It defines a command named @var{mode} to toggle the minor mode, with @var{doc} as its documentation string. @@ -1675,41 +1675,12 @@ If @var{doc} is @code{nil}, the macro supplies a default documentation string explaining the above. By default, it also defines a variable named @var{mode}, which is set to -@code{t} or @code{nil} by enabling or disabling the mode. The variable -is initialized to @var{init-value}. Except in unusual circumstances -(see below), this value must be @code{nil}. +@code{t} or @code{nil} by enabling or disabling the mode. -The string @var{lighter} says what to display in the mode line -when the mode is enabled; if it is @code{nil}, the mode is not displayed -in the mode line. - -The optional argument @var{keymap} specifies the keymap for the minor -mode. If non-@code{nil}, it should be a variable name (whose value is -a keymap), a keymap, or an alist of the form - -@example -(@var{key-sequence} . @var{definition}) -@end example - -@noindent -where each @var{key-sequence} and @var{definition} are arguments -suitable for passing to @code{define-key} (@pxref{Changing Key -Bindings}). If @var{keymap} is a keymap or an alist, this also -defines the variable @code{@var{mode}-map}. - -The above three arguments @var{init-value}, @var{lighter}, and -@var{keymap} can be (partially) omitted when @var{keyword-args} are -used. The @var{keyword-args} consist of keywords followed by +The @var{keyword-args} consist of keywords followed by corresponding values. A few keywords have special meanings: @table @code -@item :group @var{group} -Custom group name to use in all generated @code{defcustom} forms. -Defaults to @var{mode} without the possible trailing @samp{-mode}. -@strong{Warning:} don't use this default group name unless you have -written a @code{defgroup} to define that group properly. @xref{Group -Definitions}. - @item :global @var{global} If non-@code{nil}, this specifies that the minor mode should be global rather than buffer-local. It defaults to @code{nil}. @@ -1719,19 +1690,34 @@ One of the effects of making a minor mode global is that the through the Customize interface turns the mode on and off, and its value can be saved for future Emacs sessions (@pxref{Saving Customizations,,, emacs, The GNU Emacs Manual}. For the saved -variable to work, you should ensure that the @code{define-minor-mode} -form is evaluated each time Emacs starts; for packages that are not -part of Emacs, the easiest way to do this is to specify a -@code{:require} keyword. +variable to work, you should ensure that the minor mode function +is available each time Emacs starts; usually this is done by +marking the @code{define-minor-mode} form as autoloaded. @item :init-value @var{init-value} -This is equivalent to specifying @var{init-value} positionally. +This is the value to which the @var{mode} variable is initialized. +Except in unusual circumstances (see below), this value must be +@code{nil}. @item :lighter @var{lighter} -This is equivalent to specifying @var{lighter} positionally. +The string @var{lighter} says what to display in the mode line +when the mode is enabled; if it is @code{nil}, the mode is not displayed +in the mode line. @item :keymap @var{keymap} -This is equivalent to specifying @var{keymap} positionally. +The optional argument @var{keymap} specifies the keymap for the minor +mode. If non-@code{nil}, it should be a variable name (whose value is +a keymap), a keymap, or an alist of the form + +@example +(@var{key-sequence} . @var{definition}) +@end example + +@noindent +where each @var{key-sequence} and @var{definition} are arguments +suitable for passing to @code{define-key} (@pxref{Changing Key +Bindings}). If @var{keymap} is a keymap or an alist, this also +defines the variable @code{@var{mode}-map}. @item :variable @var{place} This replaces the default variable @var{mode}, used to store the state diff --git a/etc/NEWS b/etc/NEWS index 7483a6e5b75..88583d952ff 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2363,6 +2363,11 @@ This is to keep the same behavior as Eshell. * Incompatible Lisp Changes in Emacs 28.1 ++++ +** The use of positional arguments in 'define-minor-mode' is obsolete. +These were actually rendered obsolete in Emacs-21 but were never +marked as such. + ** 'facemenu-color-alist' is now obsolete, and is not used. ** 'facemenu.el' is no longer preloaded. diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el index addb58cdbbe..e23ff5ae513 100644 --- a/lisp/emacs-lisp/easy-mmode.el +++ b/lisp/emacs-lisp/easy-mmode.el @@ -139,39 +139,31 @@ documenting what its argument does. If the word \"ARG\" does not appear in DOC, a paragraph is added to DOC explaining usage of the mode argument. -Optional INIT-VALUE is the initial value of the mode's variable. - Note that the minor mode function won't be called by setting - this option, so the value *reflects* the minor mode's natural - initial state, rather than *setting* it. - In the vast majority of cases it should be nil. -Optional LIGHTER is displayed in the mode line when the mode is on. -Optional KEYMAP is the default keymap bound to the mode keymap. - If non-nil, it should be a variable name (whose value is a keymap), - or an expression that returns either a keymap or a list of - (KEY . BINDING) pairs where KEY and BINDING are suitable for - `define-key'. If you supply a KEYMAP argument that is not a - symbol, this macro defines the variable MODE-map and gives it - the value that KEYMAP specifies. - BODY contains code to execute each time the mode is enabled or disabled. It is executed after toggling the mode, and before running MODE-hook. Before the actual body code, you can write keyword arguments, i.e. alternating keywords and values. If you provide BODY, then you must - provide (even if just nil) INIT-VALUE, LIGHTER, and KEYMAP, or provide - at least one keyword argument, or both; otherwise, BODY would be - misinterpreted as the first omitted argument. The following special + provide at least one keyword argument. The following special keywords are supported (other keywords are passed to `defcustom' if the minor mode is global): -:group GROUP Custom group name to use in all generated `defcustom' forms. :global GLOBAL If non-nil specifies that the minor mode is not meant to be buffer-local, so don't make the variable MODE buffer-local. By default, the mode is buffer-local. -:init-value VAL Same as the INIT-VALUE argument. +:init-value VAL the initial value of the mode's variable. + Note that the minor mode function won't be called by setting + this option, so the value *reflects* the minor mode's natural + initial state, rather than *setting* it. + In the vast majority of cases it should be nil. Not used if you also specify :variable. -:lighter SPEC Same as the LIGHTER argument. -:keymap MAP Same as the KEYMAP argument. -:require SYM Same as in `defcustom'. +:lighter SPEC Text displayed in the mode line when the mode is on. +:keymap MAP Keymap bound to the mode keymap. Defaults to `MODE-map'. + If non-nil, it should be a variable name (whose value is + a keymap), or an expression that returns either a keymap or + a list of (KEY . BINDING) pairs where KEY and BINDING are + suitable for `define-key'. If you supply a KEYMAP argument + that is not a symbol, this macro defines the variable MODE-map + and gives it the value that KEYMAP specifies. :interactive VAL Whether this mode should be a command or not. The default is to make it one; use nil to avoid that. If VAL is a list, it's interpreted as a list of major modes this minor mode @@ -185,15 +177,18 @@ BODY contains code to execute each time the mode is enabled or disabled. sets it. If you specify a :variable, this function does not define a MODE variable (nor any of the terms used in :variable). - :after-hook A single lisp form which is evaluated after the mode hooks have been run. It should not be quoted. For example, you could write (define-minor-mode foo-mode \"If enabled, foo on you!\" :lighter \" Foo\" :require \\='foo :global t :group \\='hassle :version \"27.5\" - ...BODY CODE...)" + ...BODY CODE...) + +For backward compatibility with the Emacs<21 calling convention, +BODY can also start with the triplet INIT-VALUE LIGHTER KEYMAP." (declare (doc-string 2) + (advertised-calling-convention (mode doc &rest body) "28.1") (debug (&define name string-or-null-p [&optional [¬ keywordp] sexp &optional [¬ keywordp] sexp @@ -201,23 +196,12 @@ For example, you could write [&rest [keywordp sexp]] def-body))) - ;; Allow skipping the first three args. - (cond - ((keywordp init-value) - (setq body (if keymap `(,init-value ,lighter ,keymap ,@body) - `(,init-value ,lighter)) - init-value nil lighter nil keymap nil)) - ((keywordp lighter) - (setq body `(,lighter ,keymap ,@body) lighter nil keymap nil)) - ((keywordp keymap) (push keymap body) (setq keymap nil))) - (let* ((last-message (make-symbol "last-message")) (mode-name (symbol-name mode)) - (pretty-name (easy-mmode-pretty-mode-name mode lighter)) + (pretty-name nil) (globalp nil) (set nil) (initialize nil) - (group nil) (type nil) (extra-args nil) (extra-keywords nil) @@ -225,14 +209,28 @@ For example, you could write (setter `(setq ,mode)) ;The beginning of the exp to set the mode var. (getter mode) ;The exp to get the mode value. (modefun mode) ;The minor mode function name we're defining. - (require t) (after-hook nil) (hook (intern (concat mode-name "-hook"))) (hook-on (intern (concat mode-name "-on-hook"))) (hook-off (intern (concat mode-name "-off-hook"))) (interactive t) + (warnwrap (if (keywordp init-value) #'identity + (lambda (exp) + (macroexp-warn-and-return + "Use keywords rather than deprecated positional arguments to `define-minor-mode'" + exp)))) keyw keymap-sym tmp) + ;; Allow skipping the first three args. + (cond + ((keywordp init-value) + (setq body (if keymap `(,init-value ,lighter ,keymap ,@body) + `(,init-value ,lighter)) + init-value nil lighter nil keymap nil)) + ((keywordp lighter) + (setq body `(,lighter ,keymap ,@body) lighter nil keymap nil)) + ((keywordp keymap) (push keymap body) (setq keymap nil))) + ;; Check keys. (while (keywordp (setq keyw (car body))) (setq body (cdr body)) @@ -246,9 +244,7 @@ For example, you could write (:extra-args (setq extra-args (pop body))) (:set (setq set (list :set (pop body)))) (:initialize (setq initialize (list :initialize (pop body)))) - (:group (setq group (nconc group (list :group (pop body))))) (:type (setq type (list :type (pop body)))) - (:require (setq require (pop body))) (:keymap (setq keymap (pop body))) (:interactive (setq interactive (pop body))) (:variable (setq variable (pop body)) @@ -264,6 +260,7 @@ For example, you could write (:after-hook (setq after-hook (pop body))) (_ (push keyw extra-keywords) (push (pop body) extra-keywords)))) + (setq pretty-name (easy-mmode-pretty-mode-name mode lighter)) (setq keymap-sym (if (and keymap (symbolp keymap)) keymap (intern (concat mode-name "-map")))) @@ -301,70 +298,72 @@ or call the function `%s'.")))) ,(format base-doc-string pretty-name mode mode) ,@set ,@initialize - ,@group ,@type - ,@(unless (eq require t) `(:require ,require)) ,@(nreverse extra-keywords))))) ;; The actual function. - (defun ,modefun (&optional arg ,@extra-args) - ,(easy-mmode--mode-docstring doc pretty-name keymap-sym) - ,(when interactive - ;; Use `toggle' rather than (if ,mode 0 1) so that using - ;; repeat-command still does the toggling correctly. - (if (consp interactive) - `(interactive - (list (if current-prefix-arg - (prefix-numeric-value current-prefix-arg) - 'toggle)) - ,@interactive) - '(interactive (list (if current-prefix-arg - (prefix-numeric-value current-prefix-arg) - 'toggle))))) - (let ((,last-message (current-message))) - (,@setter - (cond ((eq arg 'toggle) - (not ,getter)) - ((and (numberp arg) - (< arg 1)) - nil) - (t - t))) - ;; Keep minor modes list up to date. - ,@(if globalp - ;; When running this byte-compiled code in earlier - ;; Emacs versions, these variables may not be defined - ;; there. So check defensively, even if they're - ;; always defined in Emacs 28 and up. - `((when (boundp 'global-minor-modes) - (setq global-minor-modes - (delq ',modefun global-minor-modes)) - (when ,getter - (push ',modefun global-minor-modes)))) - ;; Ditto check. - `((when (boundp 'local-minor-modes) - (setq local-minor-modes (delq ',modefun local-minor-modes)) - (when ,getter - (push ',modefun local-minor-modes))))) - ,@body - ;; The on/off hooks are here for backward compatibility only. - (run-hooks ',hook (if ,getter ',hook-on ',hook-off)) - (if (called-interactively-p 'any) - (progn - ,(if (and globalp (not variable)) - `(customize-mark-as-set ',mode)) - ;; Avoid overwriting a message shown by the body, - ;; but do overwrite previous messages. - (unless (and (current-message) - (not (equal ,last-message - (current-message)))) - (let ((local ,(if globalp "" " in current buffer"))) - (message ,(format "%s %%sabled%%s" pretty-name) - (if ,getter "en" "dis") local))))) - ,@(when after-hook `(,after-hook))) - (force-mode-line-update) - ;; Return the new setting. - ,getter) + ,(funcall + warnwrap + `(defun ,modefun (&optional arg ,@extra-args) + ,(easy-mmode--mode-docstring doc pretty-name keymap-sym) + ,(when interactive + ;; Use `toggle' rather than (if ,mode 0 1) so that using + ;; repeat-command still does the toggling correctly. + (if (consp interactive) + `(interactive + (list (if current-prefix-arg + (prefix-numeric-value current-prefix-arg) + 'toggle)) + ,@interactive) + '(interactive + (list (if current-prefix-arg + (prefix-numeric-value current-prefix-arg) + 'toggle))))) + (let ((,last-message (current-message))) + (,@setter + (cond ((eq arg 'toggle) + (not ,getter)) + ((and (numberp arg) + (< arg 1)) + nil) + (t + t))) + ;; Keep minor modes list up to date. + ,@(if globalp + ;; When running this byte-compiled code in earlier + ;; Emacs versions, these variables may not be defined + ;; there. So check defensively, even if they're + ;; always defined in Emacs 28 and up. + `((when (boundp 'global-minor-modes) + (setq global-minor-modes + (delq ',modefun global-minor-modes)) + (when ,getter + (push ',modefun global-minor-modes)))) + ;; Ditto check. + `((when (boundp 'local-minor-modes) + (setq local-minor-modes + (delq ',modefun local-minor-modes)) + (when ,getter + (push ',modefun local-minor-modes))))) + ,@body + ;; The on/off hooks are here for backward compatibility only. + (run-hooks ',hook (if ,getter ',hook-on ',hook-off)) + (if (called-interactively-p 'any) + (progn + ,(if (and globalp (not variable)) + `(customize-mark-as-set ',mode)) + ;; Avoid overwriting a message shown by the body, + ;; but do overwrite previous messages. + (unless (and (current-message) + (not (equal ,last-message + (current-message)))) + (let ((local ,(if globalp "" " in current buffer"))) + (message ,(format "%s %%sabled%%s" pretty-name) + (if ,getter "en" "dis") local))))) + ,@(when after-hook `(,after-hook))) + (force-mode-line-update) + ;; Return the new setting. + ,getter)) ;; Autoloading a define-minor-mode autoloads everything ;; up-to-here. -- 2.39.2