emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

master 6bec60a: (define-minor-mode): Warn about use of pre-Emacs-21 styl


From: Stefan Monnier
Subject: master 6bec60a: (define-minor-mode): Warn about use of pre-Emacs-21 style args
Date: Mon, 12 Apr 2021 11:08:25 -0400 (EDT)

branch: master
commit 6bec60ad3151825c2ee5f775848ea3d4c70c72a5
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    (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 6cf4dd2..88f2f14 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 7483a6e..88583d9 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 addb58c..e23ff5a 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 [&not keywordp] sexp
                            &optional [&not 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]