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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: "Elijah G." <eg642616 <at> gmail.com>
Subject: bug#71814: closed (Re: bug#71814: define-globalized-minor-mode
 Should predicate variable be defined before?)
Date: Sat, 29 Jun 2024 12:45:02 +0000

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

Your bug report

#71814: define-globalized-minor-mode Should predicate variable be defined before?

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 71814 <at> debbugs.gnu.org.

-- 
71814: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=71814
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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

[Message part 3 (message/rfc822, inline)]

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

Previous Next

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