bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Ihor Radchenko]

Following up the discussion in bug#57003.

Consider the following file:

------------
# -*- mode:my/test -*-
This is test with keyword to be fontified.

# Local Variables:
# eval: (setq unsafe-variable t)
# End:
-------------

Then, consider the following major mode:

(define-derived-mode my/test-mode text-mode "Test"
  ""
  (add-hook 'hack-local-variables-hook
	    (lambda ()
	      (setq-local my/test-mode-keywords '(("keyword" . font-lock-keyword-face)))
	      (setq font-lock-defaults '(my/test-mode-keywords)))
            nil 'local))

1. emacs -Q
2. Evaluate the major mode definition
3. Open the file
4. Answer "y" in the unsafe variable prompt
5. Observe "keyword" not being fontified.
6. Expected: "keyword" fontified using font-lock-keyword-face.

I can also reproduce using
(define-derived-mode my/test-mode text-mode "Test"
""
(hack-local-variables)
(setq-local my/test-mode-keywords '(("keyword" . font-lock-keyword-face)))
(setq font-lock-defaults '(my/test-mode-keywords)))

The fontification works as expected with
(define-derived-mode my/test-mode text-mode "Test"
""
(setq-local my/test-mode-keywords '(("keyword" . font-lock-keyword-face)))
(setq font-lock-defaults '(my/test-mode-keywords)))

Also reproduced on Emacs master.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

> ------------
> # -*- mode:my/test -*-
> This is test with keyword to be fontified.
>
> # Local Variables:
> # eval: (setq unsafe-variable t)
> # End:
> -------------
>
> Then, consider the following major mode:
>
> (define-derived-mode my/test-mode text-mode "Test"
>   ""
>   (add-hook 'hack-local-variables-hook
> 	    (lambda ()
> 	      (setq-local my/test-mode-keywords '(("keyword" . font-lock-keyword-face)))
> 	      (setq font-lock-defaults '(my/test-mode-keywords)))
>             nil 'local))
>
> 1. emacs -Q
> 2. Evaluate the major mode definition
> 3. Open the file
> 4. Answer "y" in the unsafe variable prompt
> 5. Observe "keyword" not being fontified.
> 6. Expected: "keyword" fontified using font-lock-keyword-face.

Hmm... AFAICT the problem here is that the implementation of
`global-font-lock-mode` ends up trying to enable `font-lock-mode` in
that file's buffer during execution of the
`after-major-mode-change-hook` of *another* buffer while querying
whether to obey those file-local settings, and then fails to try again
when the hook is run in the desired buffer.

I suspect the patch below might help (requires recompiling
`font-core.el` and re-dumping Emacs), but as the comment in there
explains it might not always be sufficient either.

Hmm...


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.patch --]
[-- Type: text/x-diff, Size: 3698 bytes --]

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 7d54a84687b..4d2174fbe6a 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -485,6 +485,8 @@ define-globalized-minor-mode
 	 (extra-keywords nil)
          (MODE-variable mode)
 	 (MODE-buffers (intern (concat global-mode-name "-buffers")))
+	 (MODE-enable-in-buffer
+	  (intern (concat global-mode-name "-enable-in-buffer")))
 	 (MODE-enable-in-buffers
 	  (intern (concat global-mode-name "-enable-in-buffers")))
 	 (MODE-check-buffers
@@ -551,10 +553,10 @@ define-globalized-minor-mode
 	 (if ,global-mode
 	     (progn
 	       (add-hook 'after-change-major-mode-hook
-			 #',MODE-enable-in-buffers)
+			 #',MODE-enable-in-buffer)
 	       (add-hook 'find-file-hook #',MODE-check-buffers)
 	       (add-hook 'change-major-mode-hook #',MODE-cmhh))
-	   (remove-hook 'after-change-major-mode-hook #',MODE-enable-in-buffers)
+	   (remove-hook 'after-change-major-mode-hook #',MODE-enable-in-buffer)
 	   (remove-hook 'find-file-hook #',MODE-check-buffers)
 	   (remove-hook 'change-major-mode-hook #',MODE-cmhh))
 
@@ -601,7 +603,32 @@ define-globalized-minor-mode
        ;; List of buffers left to process.
        (defvar ,MODE-buffers nil)
 
-       ;; The function that calls TURN-ON in each buffer.
+       ;; The function that calls TURN-ON in the current buffer.
+       (defun ,MODE-enable-in-buffer ()
+         ;; Remove ourselves from the list of pending buffers.
+         (setq ,MODE-buffers (delq (current-buffer) ,MODE-buffers))
+         (unless ,MODE-set-explicitly
+           (unless (eq ,MODE-major-mode major-mode)
+             (if ,MODE-variable
+                 (progn
+                   (,mode -1)
+                   (funcall ,turn-on-function))
+               (funcall ,turn-on-function))))
+         (setq ,MODE-major-mode major-mode))
+       (put ',MODE-enable-in-buffer 'definition-name ',global-mode)
+
+       ;; In the normal case, major modes run `after-change-major-mode-hook'
+       ;; which will have called `MODE-enable-in-buffer' for us.  But some
+       ;; major modes don't use `define-derived-mode' and thus fail to run
+       ;; `after-change-major-mode-hook', so have a combination of ugly hacks
+       ;; to try and catch those corner cases by listening to
+       ;; `change-major-mode-hook' to discover potential candidates and then
+       ;; checking in `post-command-hook' and `find-file-hook' if some of those
+       ;; still haven't run `after-change-major-mode-hook'.
+       ;; FIXME: We should try a get rid of this ugly hack and rely purely
+       ;; on `after-change-major-mode-hook' because they can (and do) end up
+       ;; running `MODE-enable-in-buffer' too early (when the major isn't yet
+       ;; fully setup) in some cases (see bug#58888).
        (defun ,MODE-enable-in-buffers ()
          (let ((buffers ,MODE-buffers))
            ;; Clear MODE-buffers to avoid scanning the same list of
@@ -611,14 +638,7 @@ define-globalized-minor-mode
            (dolist (buf buffers)
              (when (buffer-live-p buf)
                (with-current-buffer buf
-                 (unless ,MODE-set-explicitly
-                   (unless (eq ,MODE-major-mode major-mode)
-                     (if ,MODE-variable
-                         (progn
-                           (,mode -1)
-                           (funcall ,turn-on-function))
-                       (funcall ,turn-on-function))))
-                 (setq ,MODE-major-mode major-mode))))))
+                 (,MODE-enable-in-buffer))))))
        (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
 
        (defun ,MODE-check-buffers ()

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hmm... AFAICT the problem here is that the implementation of
> `global-font-lock-mode` ends up trying to enable `font-lock-mode` in
> that file's buffer during execution of the
> `after-major-mode-change-hook` of *another* buffer while querying
> whether to obey those file-local settings, and then fails to try again
> when the hook is run in the desired buffer.
>
> I suspect the patch below might help (requires recompiling
> `font-core.el` and re-dumping Emacs), but as the comment in there
> explains it might not always be sufficient either.

I tried the patch via make extraclean; make bootstrap
I can still reproduce the original recipe.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

>> Hmm... AFAICT the problem here is that the implementation of
>> `global-font-lock-mode` ends up trying to enable `font-lock-mode` in
>> that file's buffer during execution of the
>> `after-major-mode-change-hook` of *another* buffer while querying
>> whether to obey those file-local settings, and then fails to try again
>> when the hook is run in the desired buffer.
>>
>> I suspect the patch below might help (requires recompiling
>> `font-core.el` and re-dumping Emacs), but as the comment in there
>> explains it might not always be sufficient either.
>
> I tried the patch via make extraclean; make bootstrap
> I can still reproduce the original recipe.

I pushed that patch to m`aster` because it fixed other cases
(e.g. bug#69431) but I think to fix bug#58888 we need the next step,
which is the patch below.

Can you confirm that it fixes it for you as well?

It will remove support for globalized minor modes in those few remaining
major modes which don't use `run-mode-hooks`.  `run-mode-hooks` was
introduced in Emacs-22 and for many years it was really important to
support modes which don't use it, but nowadays those modes are
vanishingly rare and really should get fixed, so I think it's time to
get rid of those ugly hacks.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.patch --]
[-- Type: text/x-diff, Size: 0 bytes --]

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Eli Zaretskii]

> Cc: 58888@debbugs.gnu.org
> Date: Mon, 08 Apr 2024 08:48:36 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I pushed that patch to m`aster` because it fixed other cases
> (e.g. bug#69431) but I think to fix bug#58888 we need the next step,
> which is the patch below.

ENOPATCH

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

>> I pushed that patch to m`aster` because it fixed other cases
>> (e.g. bug#69431) but I think to fix bug#58888 we need the next step,
>> which is the patch below.
>
> ENOPATCH

[ Hmm... not sure how I managed to make that empty file.
  I'll put this in the "creativity" column.  ]

I think this one isn't empty,


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.patch --]
[-- Type: text/x-diff, Size: 4043 bytes --]

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 095bd5faa03..eaad9646985 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -495,11 +495,6 @@ define-globalized-minor-mode
 	 (MODE-buffers (intern (concat global-mode-name "-buffers")))
 	 (MODE-enable-in-buffer
 	  (intern (concat global-mode-name "-enable-in-buffer")))
-	 (MODE-enable-in-buffers
-	  (intern (concat global-mode-name "-enable-in-buffers")))
-	 (MODE-check-buffers
-	  (intern (concat global-mode-name "-check-buffers")))
-	 (MODE-cmhh (intern (concat global-mode-name "-cmhh")))
 	 (minor-MODE-hook (intern (concat mode-name "-hook")))
 	 (MODE-set-explicitly (intern (concat mode-name "-set-explicitly")))
 	 (MODE-major-mode (intern (concat (symbol-name mode) "-major-mode")))
@@ -559,14 +554,9 @@ define-globalized-minor-mode
 
 	 ;; Setup hook to handle future mode changes and new buffers.
 	 (if ,global-mode
-	     (progn
-	       (add-hook 'after-change-major-mode-hook
-			 #',MODE-enable-in-buffer)
-	       (add-hook 'find-file-hook #',MODE-check-buffers)
-	       (add-hook 'change-major-mode-hook #',MODE-cmhh))
-	   (remove-hook 'after-change-major-mode-hook #',MODE-enable-in-buffer)
-	   (remove-hook 'find-file-hook #',MODE-check-buffers)
-	   (remove-hook 'change-major-mode-hook #',MODE-cmhh))
+	     (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))
@@ -623,47 +613,7 @@ define-globalized-minor-mode
                    (funcall ,turn-on-function))
                (funcall ,turn-on-function))))
          (setq ,MODE-major-mode major-mode))
-       (put ',MODE-enable-in-buffer 'definition-name ',global-mode)
-
-       ;; In the normal case, major modes run `after-change-major-mode-hook'
-       ;; which will have called `MODE-enable-in-buffer' for us.  But some
-       ;; major modes don't use `run-mode-hooks' (customarily used via
-       ;; `define-derived-mode') and thus fail to run
-       ;; `after-change-major-mode-hook'.
-       ;; The functions below try to handle those major modes, with
-       ;; a combination of ugly hacks to try and catch those corner
-       ;; cases by listening to `change-major-mode-hook' to discover
-       ;; potential candidates and then checking in `post-command-hook'
-       ;; and `find-file-hook' if some of those still haven't run
-       ;; `after-change-major-mode-hook'.  FIXME: We should try and get
-       ;; rid of this ugly hack and rely purely on
-       ;; `after-change-major-mode-hook' because they can (and do) end
-       ;; up running `MODE-enable-in-buffer' too early (when the major
-       ;; isn't yet fully setup) in some cases (see bug#58888).
-
-       ;; The function that calls TURN-ON in each buffer.
-       (defun ,MODE-enable-in-buffers ()
-         (let ((buffers ,MODE-buffers))
-           ;; Clear MODE-buffers to avoid scanning the same list of
-           ;; buffers in recursive calls to MODE-enable-in-buffers.
-           ;; Otherwise it could lead to infinite recursion.
-           (setq ,MODE-buffers nil)
-           (dolist (buf buffers)
-             (when (buffer-live-p buf)
-              (with-current-buffer buf
-                (,MODE-enable-in-buffer))))))
-       (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
-
-       (defun ,MODE-check-buffers ()
-	 (,MODE-enable-in-buffers)
-	 (remove-hook 'post-command-hook #',MODE-check-buffers))
-       (put ',MODE-check-buffers 'definition-name ',global-mode)
-
-       ;; The function that catches kill-all-local-variables.
-       (defun ,MODE-cmhh ()
-	 (add-to-list ',MODE-buffers (current-buffer))
-	 (add-hook 'post-command-hook #',MODE-check-buffers))
-       (put ',MODE-cmhh 'definition-name ',global-mode))))
+       (put ',MODE-enable-in-buffer 'definition-name ',global-mode))))
 
 (defun easy-mmode--globalized-predicate-p (predicate)
   (cond

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I tried the patch via make extraclean; make bootstrap
>> I can still reproduce the original recipe.
>
> I pushed that patch to m`aster` because it fixed other cases
> (e.g. bug#69431) but I think to fix bug#58888 we need the next step,
> which is the patch below.
>
> Can you confirm that it fixes it for you as well?

Yes, after make bootstrap, I am no longer able to reproduce the recipe.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]

>> I pushed that patch to m`aster` because it fixed other cases
>> (e.g. bug#69431) but I think to fix bug#58888 we need the next step,
>> which is the patch below.
>>
>> Can you confirm that it fixes it for you as well?
>
> Yes, after make bootstrap, I am no longer able to reproduce the recipe.

Thanks for confirming.
Eli, any objection to the patch below?


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-define-globalized-minor-mode-Require-the-use-of-run-.patch --]
[-- Type: text/x-diff, Size: 5622 bytes --]

From 4fd9a51b4d9f2e2e0c04341eeabb656884059301 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Wed, 10 Apr 2024 16:31:58 -0400
Subject: [PATCH] (define-globalized-minor-mode): Require the use of
 `run-mode-hooks`

When `define-globalized-minor-mode` was introduced (Emacs-22),
`run-mode-hooks` was brand new, so we could not expect all major
modes to use it and we had to rely on brittle workarounds to try
and approximate `after-change-major-mode-hook`.

These workarounds have undesirable side effects, and they're not
needed any more now that virtually all major modes have been
changed to use `run-mode-hooks`.

* lisp/emacs-lisp/easy-mmode.el (define-globalized-minor-mode):
Rely only on `after-change-major-mode-hook`.  Fixes bug#58888.
---
 etc/NEWS                      |  6 ++++
 lisp/emacs-lisp/easy-mmode.el | 58 +++--------------------------------
 2 files changed, 10 insertions(+), 54 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index a2a3fe494cb..0da59201e55 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1658,6 +1658,12 @@ documentation and examples.
 \f
 * Incompatible Lisp Changes in Emacs 30.1
 
+** 'define-globalized-minor-mode' requires that modes use 'run-mode-hooks'.
+Minor modes defined with 'define-globalized-minor-mode', such as
+'global-font-lock-mode', don't work any more with major modes which
+don't use 'run-mode-hooks'.  Major modes defined with
+'define-derived-mode' are not affected.
+
 ---
 ** Old derived.el functions removed.
 The following functions have been deleted because they were only used
diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 095bd5faa03..eaad9646985 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -495,11 +495,6 @@ define-globalized-minor-mode
 	 (MODE-buffers (intern (concat global-mode-name "-buffers")))
 	 (MODE-enable-in-buffer
 	  (intern (concat global-mode-name "-enable-in-buffer")))
-	 (MODE-enable-in-buffers
-	  (intern (concat global-mode-name "-enable-in-buffers")))
-	 (MODE-check-buffers
-	  (intern (concat global-mode-name "-check-buffers")))
-	 (MODE-cmhh (intern (concat global-mode-name "-cmhh")))
 	 (minor-MODE-hook (intern (concat mode-name "-hook")))
 	 (MODE-set-explicitly (intern (concat mode-name "-set-explicitly")))
 	 (MODE-major-mode (intern (concat (symbol-name mode) "-major-mode")))
@@ -559,14 +554,9 @@ define-globalized-minor-mode
 
 	 ;; Setup hook to handle future mode changes and new buffers.
 	 (if ,global-mode
-	     (progn
-	       (add-hook 'after-change-major-mode-hook
-			 #',MODE-enable-in-buffer)
-	       (add-hook 'find-file-hook #',MODE-check-buffers)
-	       (add-hook 'change-major-mode-hook #',MODE-cmhh))
-	   (remove-hook 'after-change-major-mode-hook #',MODE-enable-in-buffer)
-	   (remove-hook 'find-file-hook #',MODE-check-buffers)
-	   (remove-hook 'change-major-mode-hook #',MODE-cmhh))
+	     (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))
@@ -623,47 +613,7 @@ define-globalized-minor-mode
                    (funcall ,turn-on-function))
                (funcall ,turn-on-function))))
          (setq ,MODE-major-mode major-mode))
-       (put ',MODE-enable-in-buffer 'definition-name ',global-mode)
-
-       ;; In the normal case, major modes run `after-change-major-mode-hook'
-       ;; which will have called `MODE-enable-in-buffer' for us.  But some
-       ;; major modes don't use `run-mode-hooks' (customarily used via
-       ;; `define-derived-mode') and thus fail to run
-       ;; `after-change-major-mode-hook'.
-       ;; The functions below try to handle those major modes, with
-       ;; a combination of ugly hacks to try and catch those corner
-       ;; cases by listening to `change-major-mode-hook' to discover
-       ;; potential candidates and then checking in `post-command-hook'
-       ;; and `find-file-hook' if some of those still haven't run
-       ;; `after-change-major-mode-hook'.  FIXME: We should try and get
-       ;; rid of this ugly hack and rely purely on
-       ;; `after-change-major-mode-hook' because they can (and do) end
-       ;; up running `MODE-enable-in-buffer' too early (when the major
-       ;; isn't yet fully setup) in some cases (see bug#58888).
-
-       ;; The function that calls TURN-ON in each buffer.
-       (defun ,MODE-enable-in-buffers ()
-         (let ((buffers ,MODE-buffers))
-           ;; Clear MODE-buffers to avoid scanning the same list of
-           ;; buffers in recursive calls to MODE-enable-in-buffers.
-           ;; Otherwise it could lead to infinite recursion.
-           (setq ,MODE-buffers nil)
-           (dolist (buf buffers)
-             (when (buffer-live-p buf)
-              (with-current-buffer buf
-                (,MODE-enable-in-buffer))))))
-       (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
-
-       (defun ,MODE-check-buffers ()
-	 (,MODE-enable-in-buffers)
-	 (remove-hook 'post-command-hook #',MODE-check-buffers))
-       (put ',MODE-check-buffers 'definition-name ',global-mode)
-
-       ;; The function that catches kill-all-local-variables.
-       (defun ,MODE-cmhh ()
-	 (add-to-list ',MODE-buffers (current-buffer))
-	 (add-hook 'post-command-hook #',MODE-check-buffers))
-       (put ',MODE-cmhh 'definition-name ',global-mode))))
+       (put ',MODE-enable-in-buffer 'definition-name ',global-mode))))
 
 (defun easy-mmode--globalized-predicate-p (predicate)
   (cond
-- 
2.43.0

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Eli Zaretskii]

> Cc: 58888@debbugs.gnu.org
> Date: Wed, 10 Apr 2024 16:34:48 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Eli, any objection to the patch below?

Any idea how many major modes out there don't use run-mode-hooks?  We
are basically breaking those with this change, right?

> Subject: [PATCH] (define-globalized-minor-mode): Require the use of
>  `run-mode-hooks`
> 
> When `define-globalized-minor-mode` was introduced (Emacs-22),
> `run-mode-hooks` was brand new, so we could not expect all major
> modes to use it and we had to rely on brittle workarounds to try
> and approximate `after-change-major-mode-hook`.
> 
> These workarounds have undesirable side effects, and they're not
> needed any more now that virtually all major modes have been
> changed to use `run-mode-hooks`.
> 
> * lisp/emacs-lisp/easy-mmode.el (define-globalized-minor-mode):
> Rely only on `after-change-major-mode-hook`.  Fixes bug#58888.

Please don't quote `like this`.  (I think you should by now have a
commit hook in your init files that replaces the quoting with our
style, because this seems to be ubiquitous in all your writings.)

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1658,6 +1658,12 @@ documentation and examples.
>  \f
>  * Incompatible Lisp Changes in Emacs 30.1
>  
> +** 'define-globalized-minor-mode' requires that modes use 'run-mode-hooks'.
> +Minor modes defined with 'define-globalized-minor-mode', such as
> +'global-font-lock-mode', don't work any more with major modes which
> +don't use 'run-mode-hooks'.  Major modes defined with
> +'define-derived-mode' are not affected.

IMO, this NEWS entry is not detailed enough.  First, it should mention
that run-mode-hooks is the modern way of running the mode's hook, a
replacement for run-hooks.  And second, it should at least hint on
what will happen with modes which don't use run-mode-hooks, so that
affected users could identify the problem when they see its signs.

Also, I think we should tell in the ELisp manual that run-mode-hooks
is now a must, not just a preference, and we should mention in the
Minor Modes chapter that the globalized minor modes will work
correctly only if the major mode uses run-mode-hooks to run its hooks.

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Any idea how many major modes out there don't use run-mode-hooks?

Hard to tell.  I did a quick

    grep '^[^;]*kill-all-local-variables' $(grep -L run-mode-hooks **/*.el)

in our own code, and it shows a fairly large number of occurrences, but
once you look at them it's not clear what to make of them.  Many of them
don't bother to define an actual mode for the buffer, they just call
`kill-all-local-variables` then fill the buffer somehow, occasionally do
some major-mode-like setup such as `use-local-map`, and finally display
the result to the user.

> We are basically breaking those with this change, right?

Yes.  More specifically the minor modes defined with
`define-globalized-minor-mode` will stay inactive in them.

> Please don't quote `like this`.  (I think you should by now have a
> commit hook in your init files that replaces the quoting with our
> style, because this seems to be ubiquitous in all your writings.)

I do my best to follow the ugly `...' convention in ELisp file and the
'...' convention in our other files, but not for anything else.
I strongly prefer the Markdown convention over '...' because it's
a convention enshrined in many tools.  I could live with the Org
convention if you prefer, but I strongly feel that we should
use notational conventions that tools are able to use.

> IMO, this NEWS entry is not detailed enough.  First, it should mention
> that run-mode-hooks is the modern way of running the mode's hook, a
> replacement for run-hooks.  And second, it should at least hint on
> what will happen with modes which don't use run-mode-hooks, so that
> affected users could identify the problem when they see its signs.
>
> Also, I think we should tell in the ELisp manual that run-mode-hooks
> is now a must, not just a preference, and we should mention in the
> Minor Modes chapter that the globalized minor modes will work
> correctly only if the major mode uses run-mode-hooks to run its hooks.

I'll get back to this once we decide we do want the change.


        Stefan

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: yantar92@posteo.net,  58888@debbugs.gnu.org
> Date: Thu, 11 Apr 2024 09:27:44 -0400
> 
> I'll get back to this once we decide we do want the change.

How shall we proceed with this decision?

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> How shall we proceed with this decision?

I just pushed it to `master`.
Like a wise man once said, let's "brace for impact".


        Stefan

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

>>> I tried the patch via make extraclean; make bootstrap
>>> I can still reproduce the original recipe.
>> I pushed that patch to m`aster` because it fixed other cases
>> (e.g. bug#69431) but I think to fix bug#58888 we need the next step,
>> which is the patch below.
>> Can you confirm that it fixes it for you as well?
> Yes, after make bootstrap, I am no longer able to reproduce the recipe.

A more tepid solution might be the patch below: the problem will still
bite those modes defined by hand (as opposed to using
`define-derived-mode`), but it should be sufficient for the most
important cases.

If we don't go for the complete removal of the hacks to support modes
that don't use `run-mode-hooks`, maybe we should try and arrange to
missing calls to `run-mode-hooks` and emit warnings accordingly.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.patch --]
[-- Type: text/x-diff, Size: 884 bytes --]

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 095bd5faa03..580cc0115cf 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -651,7 +651,12 @@ define-globalized-minor-mode
            (dolist (buf buffers)
              (when (buffer-live-p buf)
               (with-current-buffer buf
-                (,MODE-enable-in-buffer))))))
+                ;; If `delay-mode-hooks' is set, it indicates that
+                ;; the current buffer's mode is not fully setup yet,
+                ;; and also that `run-mode-hooks' will be run afterwards
+                ;; anyway, so we don't need to keep BUF in MODE-buffers.
+                (unless delay-mode-hooks
+                  (,MODE-enable-in-buffer)))))))
        (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
 
        (defun ,MODE-check-buffers ()

bug#58888: 28.1.90; font-lock-defaults not respected when hack-local-variables unsafe variable dialogue is displayed before setting the defaults

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

> A more tepid solution might be the patch below: the problem will still
> bite those modes defined by hand (as opposed to using
> `define-derived-mode`), but it should be sufficient for the most
> important cases.

The version below is a bit better.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.patch --]
[-- Type: text/x-diff, Size: 893 bytes --]

diff --git a/lisp/emacs-lisp/easy-mmode.el b/lisp/emacs-lisp/easy-mmode.el
index 095bd5faa03..015ab1d3f84 100644
--- a/lisp/emacs-lisp/easy-mmode.el
+++ b/lisp/emacs-lisp/easy-mmode.el
@@ -661,8 +661,12 @@ define-globalized-minor-mode
 
        ;; The function that catches kill-all-local-variables.
        (defun ,MODE-cmhh ()
-	 (add-to-list ',MODE-buffers (current-buffer))
-	 (add-hook 'post-command-hook #',MODE-check-buffers))
+         ;; If `delay-mode-hooks' is set, it indicates that the current
+         ;; buffer's mode will run `run-mode-hooks' afterwards anyway,
+         ;; so we don't need to keep BUF in MODE-buffers.
+	 (unless delay-mode-hooks
+	   (add-to-list ',MODE-buffers (current-buffer))
+	   (add-hook 'post-command-hook #',MODE-check-buffers)))
        (put ',MODE-cmhh 'definition-name ',global-mode))))
 
 (defun easy-mmode--globalized-predicate-p (predicate)