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

[Alan Mackenzie]

Hi, Emacs.

On Sun, Jan 13, 2013 at 07:28:54PM +0000, Alan Mackenzie wrote:
> The situation here is the direct cause of bug #11152, where in CC Mode,
> doc comments whose fontification is specified in a mode hook don't get
> fontified properly, or at all.  It would be good to fix this bug for
> Emacs 24.3.

> In define-globalized-minor-mode L72-75, the newly defined -enable-
> function is added to both the following hooks:
>     change-major-mode-after-body-hook
>     after-change-major-mode-hook
> .  (These hooks are run before and after the major mode hook.)

> It seems the hacker who formulated this macro was undecided whether to
> run the -enable- function before or after the mode hooks, so decided
> upon both as a compromise.  This isn't harmless.

> In particular, running `global-font-lock-mode-enable-in-buffers' before
> the objc-mode-hook causes a one-time font-lock-keywords initialisation
> to happen before a critical initialisation has been performed by that
> hook.

> Can some means be found so that `font-lock-mode' isn't called twice?
> (For that matter, `global-hi-lock-mode-enable-in-buffers' doesn't need
> calling twice, either.)  Such a means might be the addition of another
> parameter to define-globalized-minor-mode specifying when the -enable-
> call should take place.

I propose to amend define-globalized-minor-mode such that the generated
-enable- function gets added to precisely one of the following hooks, not
both:
    change-major-mode-after-body-hook
    after-change-major-mode-hook
, the latter one by default.  To change this default, the new
keyword-parameter :call-before-hook is introduced.  I think this
amendment should go into Emacs 24.3 (it "solves" bug #11152).

There are 6 invocations of define-globalized-minor-mode in Emacs,
1 global-visual-line-mode
2 global-hi-lock-mode
3 global-font-lock-mode
4 global-cwarn-mode
5 global-linum-mode
6 global-highlight-changes-mode
.  2 and 3 appear to work fine with the amended macro.  I haven't tested
the others.

Here is the patch to the .el file (another will be needed for NEWS):


=== 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-14 15:50:50 +0000
***************
*** 329,335 ****
    and that should try to turn MODE on if applicable for that buffer.
  KEYS is a list of CL-style keyword arguments.  As the minor mode
    defined by this function is always global, any :global keyword is
!   ignored.  Other keywords have the same meaning as in `define-minor-mode',
    which see.  In particular, :group specifies the custom group.
    The most useful keywords are those that are passed on to the
    `defcustom'.  It normally makes no sense to pass the :lighter
--- 329,338 ----
    and that should try to turn MODE on if applicable for that buffer.
  KEYS is a list of CL-style keyword arguments.  As the minor mode
    defined by this function is always global, any :global keyword is
!   ignored.  If :call-before-hook is present with a non-nil argument, TURN-ON
!   will be called before running the major mode's hook, otherwise it will be
!   called afterwards.
!  Other keywords have the same meaning as in `define-minor-mode',
    which see.  In particular, :group specifies the custom group.
    The most useful keywords are those that are passed on to the
    `defcustom'.  It normally makes no sense to pass the :lighter
***************
*** 346,351 ****
--- 349,355 ----
         (pretty-name (easy-mmode-pretty-mode-name mode))
         (pretty-global-name (easy-mmode-pretty-mode-name global-mode))
         (group nil)
+        (call-before-hook nil)
         (extra-keywords nil)
         (MODE-buffers (intern (concat global-mode-name "-buffers")))
         (MODE-enable-in-buffers
***************
*** 362,367 ****
--- 366,372 ----
        (pcase keyw
        (`:group (setq group (nconc group (list :group (pop keys)))))
        (`:global (setq keys (cdr keys)))
+       (`:call-before-hook (setq call-before-hook (pop keys)))
        (_ (push keyw extra-keywords) (push (pop keys) extra-keywords))))
  
      (unless group
***************
*** 394,402 ****
         ;; Setup hook to handle future mode changes and new buffers.
         (if ,global-mode
             (progn
!              (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))
--- 399,407 ----
         ;; Setup hook to handle future mode changes and new buffers.
         (if ,global-mode
             (progn
!              (add-hook ',(if call-before-hook
!                              'change-major-mode-after-body-hook
!                            'after-change-major-mode-hook)
                         ',MODE-enable-in-buffers)
               (add-hook 'find-file-hook ',MODE-check-buffers)
               (add-hook 'change-major-mode-hook ',MODE-cmhh))



-- 
Alan Mackenzie (Nuremberg, Germany).