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: Stefan Kangas <stefankangas <at> gmail.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#71814: closed (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 message dated Sat, 29 Jun 2024 05:43:56 -0700
with message-id <CADwFkmka4OpMfEN1r_4RqS_s3b9pwepHA-+rzTb-BPdtAZ+cpA <at> mail.gmail.com>
and subject line Re: bug#71814: define-globalized-minor-mode Should predicate variable be defined before?
has caused the debbugs.gnu.org bug report #71814,
regarding define-globalized-minor-mode Should predicate variable be defined before?
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> 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: "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 part 3 (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).

Previous Next

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