Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]

[Alan Mackenzie <acm@muc.de>]

Hi, Stefan.

On Thu, Jan 17, 2013 at 01:31:32PM -0500, Stefan Monnier wrote:
[Note: this discussion is relevant to bug#11152.]

> The issue is how to locally turn off the globalized minor mode from
> a major-mode hook.  If the globalized mode is enabled after the
> major-mode hook, you basically can't do it without resorting to
> ugly gymnastics.

> I'm not completely sure what happens (and what should happen instead) in
> your scenario, but for the case described by João, where you don't just
> want to disable it in some modes, but you want to prevent it from ever
> being enabled in such modes, obviously the current approach
> fails similarly.

> A simple solution is to activate the minor mode late, and provide
> a `disable-foo-mode' variable, that can be set in the major mode hook.
> But it's kind of ugly because it requires another var and the user needs
> to know about that magic var.

> Maybe we could do it this way:

> for a globalized minor mode `foo', before running the major-mode hook, a magic
> `disable-foo-mode' is set to nil.  A function is added to `foo-mode-hook'
> which sets this var to t when foo-mode is disabled.  And after running
> the major-mode hook, the globalized minor mode code checks the magic var
> to decide whether to enable the minor mode.

OK, I've bitten the bullet.  Please see my patch below, which implements
this precisely.  It seems to work, in that:
(i) It fixes #11152 - the new global-font-lock-mode calls font-lock-mode
  _after_ the major mode hook.
(ii) Should the major mode hook do (font-lock-mode -1), this isn't
  overridden by global mode.

> This way, the user doesn't need to know about the extra magic var; she
> can just call (foo-mode -1) in her major-mode hook as she can now, but
> the minor-mode is really only activated late.

> It's also kind of ugly because of all the magic, but it's the best I can
> come up with so far.

Here's the patch, based on easy-mmode.el in the emacs-24 branch:



=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el	2013-01-01 09:11:05 +0000
--- lisp/emacs-lisp/easy-mmode.el	2013-01-31 10:13:51 +0000
***************
*** 340,348 ****
  enabled, then disabling and reenabling MODE should make MODE work
  correctly with the current major mode.  This is important to
  prevent problems with derived modes, that is, major modes that
! call another major mode in their body."
    (declare (doc-string 2))
    (let* ((global-mode-name (symbol-name global-mode))
  	 (pretty-name (easy-mmode-pretty-mode-name mode))
  	 (pretty-global-name (easy-mmode-pretty-mode-name global-mode))
  	 (group nil)
--- 340,353 ----
  enabled, then disabling and reenabling MODE should make MODE work
  correctly with the current major mode.  This is important to
  prevent problems with derived modes, that is, major modes that
! call another major mode in their body.
! 
! MODE is actually turned on when a major mode is initialized, just
! after running the major mode's hook.  However, MODE is not turned
! on, should the hook explicitly disable it."
    (declare (doc-string 2))
    (let* ((global-mode-name (symbol-name global-mode))
+ 	 (mode-name (symbol-name mode))
  	 (pretty-name (easy-mmode-pretty-mode-name mode))
  	 (pretty-global-name (easy-mmode-pretty-mode-name global-mode))
  	 (group nil)
***************
*** 353,358 ****
--- 358,369 ----
  	 (MODE-check-buffers
  	  (intern (concat global-mode-name "-check-buffers")))
  	 (MODE-cmhh (intern (concat global-mode-name "-cmhh")))
+ 	 (MODE-cancel-disable
+ 	  (intern (concat global-mode-name "-cancel-disable")))
+ 	 (MODE-disable-in-buffer
+ 	  (intern (concat global-mode-name "-disable-in-buffer")))
+ 	 (minor-MODE-hook (intern (concat mode-name "-hook")))
+ 	 (disable-MODE (intern (concat "disable-" mode-name)))
  	 (MODE-major-mode (intern (concat (symbol-name mode) "-major-mode")))
  	 keyw)
  
***************
*** 397,403 ****
  	       (add-hook 'after-change-major-mode-hook
  			 ',MODE-enable-in-buffers)
  	       (add-hook 'change-major-mode-after-body-hook
! 			 ',MODE-enable-in-buffers)
  	       (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)
--- 408,414 ----
  	       (add-hook 'after-change-major-mode-hook
  			 ',MODE-enable-in-buffers)
  	       (add-hook 'change-major-mode-after-body-hook
! 			 ',MODE-cancel-disable)
  	       (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)
***************
*** 415,420 ****
--- 426,435 ----
         ;; up-to-here.
         :autoload-end
  
+        ;; A function which checks whether MODE has been disabled in the major
+        ;; mode hook which has just been run.
+        (add-hook ',minor-MODE-hook ',MODE-disable-in-buffer)
+ 
         ;; List of buffers left to process.
         (defvar ,MODE-buffers nil)
  
***************
*** 423,436 ****
  	 (dolist (buf ,MODE-buffers)
  	   (when (buffer-live-p buf)
  	     (with-current-buffer buf
!                (unless (eq ,MODE-major-mode major-mode)
!                  (if ,mode
!                      (progn
!                        (,mode -1)
!                        (,turn-on)
!                        (setq ,MODE-major-mode major-mode))
!                    (,turn-on)
!                    (setq ,MODE-major-mode major-mode)))))))
         (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
  
         (defun ,MODE-check-buffers ()
--- 438,452 ----
  	 (dolist (buf ,MODE-buffers)
  	   (when (buffer-live-p buf)
  	     (with-current-buffer buf
!                (if ,disable-MODE
! 		   (if ,mode (,mode -1))
! 		 (unless (eq ,MODE-major-mode major-mode)
! 		   (if ,mode
! 		       (progn
! 			 (,mode -1)
! 			 (,turn-on))
! 		     (,turn-on))))
! 	       (setq ,MODE-major-mode major-mode)))))
         (put ',MODE-enable-in-buffers 'definition-name ',global-mode)
  
         (defun ,MODE-check-buffers ()
***************
*** 443,449 ****
         (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))))
  
  ;;;
  ;;; easy-mmode-defmap
--- 459,473 ----
         (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)
!        (defvar ,disable-MODE nil)
!        (defun ,MODE-cancel-disable ()
! 	 (setq ,disable-MODE nil))
!        (put ',MODE-cancel-disable 'definition-name ',global-mode)
!        (defun ,MODE-disable-in-buffer ()
! 	 (unless ,mode
! 	   (setq ,disable-MODE t)))
!        (put ',MODE-disable-in-buffer 'definition-name ',global-mode))))
  
  ;;;
  ;;; easy-mmode-defmap


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).