GNU bug report logs - #71814
define-globalized-minor-mode Should predicate variable be defined before?

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 71814 in the body.
You can then email your comments to 71814 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: "Elijah G." <eg642616 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: define-globalized-minor-mode Should predicate variable be defined
 before?
Date: Thu, 27 Jun 2024 19:20:53 -0600

Hello, i've noticed when defining a globalized minor mode using the
define-globalized-minor-mode macro, gives a byte-compile warning about
the auto-generated :predicate variable not being defined.

I found that it's because the macro defines the predicate user option
after defining the minor mode:

         (put ',global-mode 'globalized-minor-mode t)
         :autoload-end
         (defvar-local ,MODE-major-mode nil))
       ;; The actual global minor-mode
       (define-minor-mode ,global-mode
         ,(concat (format "Toggle %s in all buffers.\n" pretty-name)
                  (internal--format-docstring-line
                   ...)

	 ;; Setup hook to handle future mode changes and new buffers.
	 (if ,global-mode
	     (add-hook 'after-change-major-mode-hook
		       #',MODE-enable-in-buffer)
	   (remove-hook 'after-change-major-mode-hook #',MODE-enable-in-buffer))

	 ;; Go through existing buffers.
	 (dolist (buf (buffer-list))
	   (with-current-buffer buf
             (if ,global-mode (funcall ,turn-on-function)
               (when ,mode (,mode -1)))))
         ,@body)

       ,(when predicate
          `(defcustom ,MODE-predicate ,(car predicate)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

My question is, should it be defined before defining the minor mode?
if not, then why it's defined like that?

I made a little patch for solve this, since i think it would be it would
be beneficial for packages that use that macro (also i barely remember
that there are some built-in packages that defines the user-option
before using the macro for solve this issue).

Thanks.
--8<---------------cut here---------------start------------->8---
diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index ba0f8bad393..69cc1332282 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -525,6 +525,36 @@ define-globalized-minor-mode
          (put ',global-mode 'globalized-minor-mode t)
          :autoload-end
          (defvar-local ,MODE-major-mode nil))
+       ,(when predicate
+          `(defcustom ,MODE-predicate ,(car predicate)
+             ,(format "Which major modes `%s' is switched on in.
+This variable can be either t (all major modes), nil (no major modes),
+or a list of modes and (not modes) to switch use this minor mode or
+not.  For instance
+
+  (c-mode (not message-mode mail-mode) text-mode)
+
+means \"use this mode in all modes derived from `c-mode', don't use in
+modes derived from `message-mode' or `mail-mode', but do use in other
+modes derived from `text-mode'\".  An element with value t means \"use\"
+and nil means \"don't use\".  There's an implicit nil at the end of the
+list."
+                      mode)
+             :type '(choice
+                     (const :tag "Enable in all major modes" t)
+                     (const :tag "Don't enable in any major mode" nil)
+                     (repeat
+                      :tag "Rules (earlier takes precedence)..."
+                      (choice
+                       (const :tag "Enable in all (other) modes" t)
+                       (const :tag "Don't enable in any (other) mode" nil)
+                       (symbol :value fundamental-mode
+                               :tag "Enable in major mode")
+                       (cons :tag "Don't enable in major modes"
+                             (const :tag "Don't enable in..." not)
+                             (repeat (symbol :value fundamental-mode
+                                             :tag "Major mode"))))))
+             ,@group))
        ;; The actual global minor-mode
        (define-minor-mode ,global-mode
          ,(concat (format "Toggle %s in all buffers.\n" pretty-name)
@@ -565,37 +595,6 @@ define-globalized-minor-mode
                (when ,mode (,mode -1)))))
          ,@body)
 
-       ,(when predicate
-          `(defcustom ,MODE-predicate ,(car predicate)
-             ,(format "Which major modes `%s' is switched on in.
-This variable can be either t (all major modes), nil (no major modes),
-or a list of modes and (not modes) to switch use this minor mode or
-not.  For instance
-
-  (c-mode (not message-mode mail-mode) text-mode)
-
-means \"use this mode in all modes derived from `c-mode', don't use in
-modes derived from `message-mode' or `mail-mode', but do use in other
-modes derived from `text-mode'\".  An element with value t means \"use\"
-and nil means \"don't use\".  There's an implicit nil at the end of the
-list."
-                      mode)
-             :type '(choice
-                     (const :tag "Enable in all major modes" t)
-                     (const :tag "Don't enable in any major mode" nil)
-                     (repeat
-                      :tag "Rules (earlier takes precedence)..."
-                      (choice
-                       (const :tag "Enable in all (other) modes" t)
-                       (const :tag "Don't enable in any (other) mode" nil)
-                       (symbol :value fundamental-mode
-                               :tag "Enable in major mode")
-                       (cons :tag "Don't enable in major modes"
-                             (const :tag "Don't enable in..." not)
-                             (repeat (symbol :value fundamental-mode
-                                             :tag "Major mode"))))))
-             ,@group))
-
        ;; Autoloading define-globalized-minor-mode autoloads everything
        ;; up-to-here.
        :autoload-end
--8<---------------cut here---------------end--------------->8---

--
E.G. from Gnus The Emacs Newsreader and E-mail client

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: "Elijah G." <eg642616 <at> gmail.com>, 71814 <at> debbugs.gnu.org
Subject: Re: bug#71814: define-globalized-minor-mode Should predicate variable
 be defined before?
Date: Fri, 28 Jun 2024 20:27:17 -0700

"Elijah G." <eg642616 <at> gmail.com> writes:

> Hello, i've noticed when defining a globalized minor mode using the
> define-globalized-minor-mode macro, gives a byte-compile warning about
> the auto-generated :predicate variable not being defined.

Thanks for the bug report and proposed fix.

Could you please provide a recipe to reproduce the problem?

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

From: Elijah G <eg642616 <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 71814 <at> debbugs.gnu.org
Subject: Re: bug#71814: define-globalized-minor-mode Should predicate variable
 be defined before?
Date: Fri, 28 Jun 2024 22:33:25 -0600

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

El vie., 28 de junio de 2024 9:27 p. m., Stefan Kangas <
stefankangas <at> gmail.com> escribió:

> "Elijah G." <eg642616 <at> gmail.com> writes:
>
> > Hello, i've noticed when defining a globalized minor mode using the
> > define-globalized-minor-mode macro, gives a byte-compile warning about
> > the auto-generated :predicate variable not being defined.
>
> Thanks for the bug report and proposed fix.
>
> Could you please provide a recipe to reproduce the problem?
>

Sure.

1. create a .el file
2. insert this snippet into to file:
```
(define-minor-mode test-mode "")
(define-globalized-minor-mode global-test-mode
  test-mode #'ignore
  :group 'test
  :predicate '(prog-mode text-mode))
```
3. byte-compile the file
4. in the compile log buffer should appear this warning:
`Warning: reference to free variable ‘global-test-modes’`
which is the variable generated by the macro.

>

[Message part 2 (text/html, inline)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Elijah G <eg642616 <at> gmail.com>
Cc: 71814-done <at> debbugs.gnu.org
Subject: Re: bug#71814: define-globalized-minor-mode Should predicate variable
 be defined before?
Date: Sat, 29 Jun 2024 05:43:56 -0700

Elijah G <eg642616 <at> gmail.com> writes:

> 1. create a .el file
> 2. insert this snippet into to file:
> ```
> (define-minor-mode test-mode "")
> (define-globalized-minor-mode global-test-mode
>   test-mode #'ignore
>   :group 'test
>   :predicate '(prog-mode text-mode))
> ```
> 3. byte-compile the file
> 4. in the compile log buffer should appear this warning:
> `Warning: reference to free variable ‘global-test-modes’`
> which is the variable generated by the macro.

Thanks, I've now fixed this on emacs-30 (commit a65b6aac6b5).

Previous Next

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