Re: org-mode and mode hooks.

[Luc Teirlinck]

The new patch below to easy-mmode implements the suggested changes
with a few proposed differences.  Instead of thismode-... I believe it
is better to use MODE-..., since thismode could refer to major modes
or to the global minor mode.  I believe that MODE-.. more clearly
indicates that it refers to the mode in the MODE argument.  MODE is
also at least somewhat shorter than thismode.  MODE-enable-in-buffers
might be an appropriate name for the function priorly named buffers,
but not for the variable, so I renamed that one MODE-buffers.  Also, I
believe that MODE-recorded-major-mode can be shortened to
MODE-major-mode, since the latter already obviously refers to the
major mode associated with MODE.

The patches to font-{lock,core} contain no further changes from the
latest previous versions other than being adapted to the namechanges
in easy-mmode.

In as far as Font Lock is concerned, the situation before the patches
was that there was a non-trivial probability that Font Lock could be
enabled for a wrong mode.  After the patches, everything works
completely OK for modes that follow the conventions.  For modes that
do not follow the conventions, Font Lock will still be enabled for the
correct mode.  But there is a small probability that keywords added to
mode-hooks of non-standard major modes might not be enabled in derived
modes.  The docstrings of font-lock-{add,remove}-keywords mention this
possibility and suggest to file a bug report in such cases, so that
the major mode can be fixed.

I have been very much confused in all of this by some confusing and
inaccurate docs of Font Lock.  The patches below already correct the
worst problems I encountered in docstrings.  I will propose patches to
the Elisp and maybe the Emacs manuals, once we agree on the final
versions of my three patches.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el       22 May 2005 16:50:33 -0500      1.63
--- easy-mmode.el       01 Jun 2005 21:16:52 -0500      
***************
*** 271,284 ****
  TURN-ON is a function that will be called with no args in every buffer
    and that should try to turn MODE on if applicable for that buffer.
  KEYS is a list of CL-style keyword arguments:
! :group to specify the custom group."
    (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)
         (extra-args nil)
!        (buffers (intern (concat global-mode-name "-buffers")))
!        (cmmh (intern (concat global-mode-name "-cmmh"))))
  
      ;; Check keys.
      (while (keywordp (car keys))
--- 271,296 ----
  TURN-ON is a function that will be called with no args in every buffer
    and that should try to turn MODE on if applicable for that buffer.
  KEYS is a list of CL-style keyword arguments:
! :group to specify the custom group.
! 
! If MODE's set-up depends on the major mode in effect when it was
! 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."
! 
    (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)
         (extra-args nil)
!        (MODE-buffers (intern (concat global-mode-name "-buffers")))
!        (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")))
!        (MODE-major-mode (intern (concat (symbol-name mode) "-major-mode"))))
  
      ;; Check keys.
      (while (keywordp (car keys))
***************
*** 294,299 ****
--- 306,313 ----
                                "-mode\\'" "" (symbol-name mode))))))
  
      `(progn
+        (defvar ,MODE-major-mode nil)
+        (make-variable-buffer-local ',MODE-major-mode)
         ;; The actual global minor-mode
         (define-minor-mode ,global-mode
         ,(format "Toggle %s in every buffer.
***************
*** 306,315 ****
         ;; Setup hook to handle future mode changes and new buffers.
         (if ,global-mode
             (progn
!              (add-hook 'after-change-major-mode-hook ',buffers)
!              (add-hook 'change-major-mode-hook ',cmmh))
!          (remove-hook 'after-change-major-mode-hook ',buffers)
!          (remove-hook 'change-major-mode-hook ',cmmh))
  
         ;; Go through existing buffers.
         (dolist (buf (buffer-list))
--- 320,332 ----
         ;; 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 '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 'find-file-hook ',MODE-check-buffers)
!          (remove-hook 'change-major-mode-hook ',MODE-cmhh))
  
         ;; Go through existing buffers.
         (dolist (buf (buffer-list))
***************
*** 321,342 ****
         :autoload-end
  
         ;; List of buffers left to process.
!        (defvar ,buffers nil)
  
         ;; The function that calls TURN-ON in each buffer.
!        (defun ,buffers ()
!        (remove-hook 'post-command-hook ',buffers)
!        (while ,buffers
!          (let ((buf (pop ,buffers)))
!            (when (buffer-live-p buf)
!              (with-current-buffer buf (,turn-on))))))
!        (put ',buffers 'definition-name ',global-mode)
  
         ;; The function that catches kill-all-local-variables.
!        (defun ,cmmh ()
!        (add-to-list ',buffers (current-buffer))
!        (add-hook 'post-command-hook ',buffers))
!        (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
  ;;; easy-mmode-defmap
--- 338,370 ----
         :autoload-end
  
         ;; List of buffers left to process.
!        (defvar ,MODE-buffers nil)
  
         ;; The function that calls TURN-ON in each buffer.
!        (defun ,MODE-enable-in-buffers ()
!        (dolist (buf ,MODE-buffers)
!          (when (buffer-live-p buf)
!            (with-current-buffer buf
!              (if ,mode
!                  (unless (eq ,MODE-major-mode major-mode)
!                    (,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 ()
!        (,MODE-enable-in-buffers)
!        (setq ,MODE-buffers nil)
!        (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))))
  
  ;;;
  ;;; easy-mmode-defmap
============================================================

===File ~/font-core-diff====================================
*** font-core.el        22 May 2005 16:46:07 -0500      1.28
--- font-core.el        01 Jun 2005 21:06:08 -0500      
***************
*** 88,93 ****
--- 88,95 ----
  It will be passed one argument, which is the current value of
  `font-lock-mode'.")
  
+ ;; The mode for which font-lock was initialized, or nil if none.
+ (defvar font-lock-mode-major-mode)
  (define-minor-mode font-lock-mode
    "Toggle Font Lock mode.
  With arg, turn Font Lock mode off if and only if arg is a non-positive
***************
*** 156,162 ****
    ;; Arrange to unfontify this buffer if we change major mode later.
    (if font-lock-mode
        (add-hook 'change-major-mode-hook 'font-lock-change-mode nil t)
!     (remove-hook 'change-major-mode-hook 'font-lock-change-mode t)))
  
  ;; Get rid of fontification for the old major mode.
  ;; We do this when changing major modes.
--- 158,166 ----
    ;; Arrange to unfontify this buffer if we change major mode later.
    (if font-lock-mode
        (add-hook 'change-major-mode-hook 'font-lock-change-mode nil t)
!     (remove-hook 'change-major-mode-hook 'font-lock-change-mode t))
!   (when font-lock-mode
!     (setq font-lock-mode-major-mode major-mode)))
  
  ;; Get rid of fontification for the old major mode.
  ;; We do this when changing major modes.
***************
*** 175,180 ****
--- 179,185 ----
                                      '(font-lock-face)))
      (restore-buffer-modified-p modp)))
  
+ (defvar font-lock-set-defaults)
  (defun font-lock-default-function (mode)
    ;; Turn on Font Lock mode.
    (when mode
***************
*** 201,209 ****
    ;; Only do hard work if the mode has specified stuff in
    ;; `font-lock-defaults'.
    (when (or font-lock-defaults
!           (and (boundp 'font-lock-keywords) font-lock-keywords)
            (with-no-warnings
!            (cdr (assq major-mode font-lock-defaults-alist))))
      (font-lock-mode-internal mode)))
  
  (defun turn-on-font-lock ()
--- 206,219 ----
    ;; Only do hard work if the mode has specified stuff in
    ;; `font-lock-defaults'.
    (when (or font-lock-defaults
!           (if (boundp 'font-lock-keywords) font-lock-keywords)
            (with-no-warnings
!             (cdr (assq major-mode font-lock-defaults-alist)))
!           (and mode
!                (boundp 'font-lock-set-defaults)
!                font-lock-set-defaults
!                font-lock-mode-major-mode
!                (not (eq font-lock-mode-major-mode major-mode))))
      (font-lock-mode-internal mode)))
  
  (defun turn-on-font-lock ()
============================================================

===File ~/font-lock-diff====================================
*** font-lock.el        29 May 2005 09:15:38 -0500      1.259
--- font-lock.el        01 Jun 2005 21:06:21 -0500      
***************
*** 683,691 ****
  adds two fontification patterns for C mode, to fontify `FIXME:' words, even in
  comments, and to fontify `and', `or' and `not' words as keywords.
  
! When used from a Lisp program (such as a minor mode), it is recommended to
! use nil for MODE (and place the call on a hook) to avoid subtle problems
! due to details of the implementation.
  
  Note that some modes have specialized support for additional patterns, e.g.,
  see the variables `c-font-lock-extra-types', `c++-font-lock-extra-types',
--- 683,704 ----
  adds two fontification patterns for C mode, to fontify `FIXME:' words, even in
  comments, and to fontify `and', `or' and `not' words as keywords.
  
! The above procedure will only add the keywords for C mode, not
! for modes derived from C mode.  To add them for derived modes too,
! pass nil for MODE and add the call to c-mode-hook.
! 
! For example:
! 
!  (add-hook 'c-mode-hook
!   (lambda ()
!    (font-lock-add-keywords 'c-mode
!     '((\"\\\\\\=<\\\\(FIXME\\\\):\" 1 font-lock-warning-face prepend)
!       (\"\\\\\\=<\\\\(and\\\\|or\\\\|not\\\\)\\\\\\=>\" .
!        font-lock-keyword-face)))))
! 
! The above procedure may fail to add keywords to derived modes if
! some involved major mode does not follow the standard conventions.
! File a bug report if this happens, so the major mode can be corrected.
  
  Note that some modes have specialized support for additional patterns, e.g.,
  see the variables `c-font-lock-extra-types', `c++-font-lock-extra-types',
***************
*** 704,710 ****
         (font-lock-update-removed-keyword-alist mode keywords append))
        (t
         ;; Otherwise set or add the keywords now.
!        ;; This is a no-op if it has been done already in this buffer.
         (font-lock-set-defaults)
         (let ((was-compiled (eq (car font-lock-keywords) t)))
           ;; Bring back the user-level (uncompiled) keywords.
--- 717,724 ----
         (font-lock-update-removed-keyword-alist mode keywords append))
        (t
         ;; Otherwise set or add the keywords now.
!        ;; This is a no-op if it has been done already in this buffer
!        ;; for the correct major mode.
         (font-lock-set-defaults)
         (let ((was-compiled (eq (car font-lock-keywords) t)))
           ;; Bring back the user-level (uncompiled) keywords.
***************
*** 774,782 ****
  MODE should be a symbol, the major mode command name, such as `c-mode'
  or nil.  If nil, highlighting keywords are removed for the current buffer.
  
! When used from a Lisp program (such as a minor mode), it is recommended to
! use nil for MODE (and place the call on a hook) to avoid subtle problems
! due to details of the implementation."
    (cond (mode
         ;; Remove one keyword at the time.
         (dolist (keyword keywords)
--- 788,798 ----
  MODE should be a symbol, the major mode command name, such as `c-mode'
  or nil.  If nil, highlighting keywords are removed for the current buffer.
  
! To make the removal apply to modes derived from MODE as well,
! pass nil for MODE and add the call to MODE-hook.  This may fail
! for some derived modes if some involved major mode does not
! follow the standard conventions.  File a bug report if this
! happens, so the major mode can be corrected."
    (cond (mode
         ;; Remove one keyword at the time.
         (dolist (keyword keywords)
***************
*** 1571,1582 ****
  
  (defvar font-lock-set-defaults nil)   ; Whether we have set up defaults.
  
  (defun font-lock-set-defaults ()
    "Set fontification defaults appropriately for this mode.
  Sets various variables using `font-lock-defaults' (or, if nil, using
  `font-lock-defaults-alist') and `font-lock-maximum-decoration'."
!   ;; Set fontification defaults iff not previously set.
!   (unless font-lock-set-defaults
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
--- 1587,1600 ----
  
  (defvar font-lock-set-defaults nil)   ; Whether we have set up defaults.
  
+ (defvar font-lock-mode-major-mode)
  (defun font-lock-set-defaults ()
    "Set fontification defaults appropriately for this mode.
  Sets various variables using `font-lock-defaults' (or, if nil, using
  `font-lock-defaults-alist') and `font-lock-maximum-decoration'."
!   ;; Set fontification defaults iff not previously set for correct major mode.
!   (unless (and font-lock-set-defaults
!              (eq font-lock-mode-major-mode major-mode))
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
============================================================