org-mode and mode hooks.

org-mode and mode hooks.

[Lute Kamstra]

org-mode in lisp/textmodes/org.el is derived from outline-mode, but it
doesn't use define-derived-mode to accomplish this.  Instead, it just
calls outline-mode as the first thing it does.  (I guess this is
because org-mode accepts an argument, which define-derived-mode
doesn't support.)  The problem with this approach is that
outline-mode's mode hook and after-change-major-mode-hook are run at
the wrong time.  What about the following patch to fix this?

Lute.


Index: lisp/textmodes/org.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/textmodes/org.el,v
retrieving revision 1.18
diff -c -r1.18 org.el
*** lisp/textmodes/org.el	20 May 2005 11:24:48 -0000	1.18
--- lisp/textmodes/org.el	25 May 2005 13:27:59 -0000
***************
*** 1544,1550 ****
  
  \\{org-mode-map}"
    (interactive "P")
!   (outline-mode)
    (setq major-mode 'org-mode)
    (setq mode-name "Org")
    (use-local-map org-mode-map)
--- 1544,1551 ----
  
  \\{org-mode-map}"
    (interactive "P")
!   (delay-mode-hooks  
!     (outline-mode))
    (setq major-mode 'org-mode)
    (setq mode-name "Org")
    (use-local-map org-mode-map)
***************
*** 1577,1583 ****
        (save-excursion
          (goto-char (point-min))
          (insert "    -*- mode: org -*-\n\n")))
!   (run-hooks 'org-mode-hook)
    (unless org-inhibit-startup
      (if org-startup-with-deadline-check
          (call-interactively 'org-check-deadlines)
--- 1578,1584 ----
        (save-excursion
          (goto-char (point-min))
          (insert "    -*- mode: org -*-\n\n")))
!   (run-mode-hooks 'org-mode-hook)
    (unless org-inhibit-startup
      (if org-startup-with-deadline-check
          (call-interactively 'org-check-deadlines)
***************
*** 3120,3126 ****
       "--")
     (mapcar 'org-file-menu-entry org-agenda-files)))
    (org-agenda-set-mode-name)
!   (run-hooks 'org-agenda-mode-hook))
  
  (define-key org-agenda-mode-map [(tab)] 'org-agenda-goto)
  (define-key org-agenda-mode-map [(return)] 'org-agenda-switch-to)
--- 3121,3127 ----
       "--")
     (mapcar 'org-file-menu-entry org-agenda-files)))
    (org-agenda-set-mode-name)
!   (run-mode-hooks 'org-agenda-mode-hook))
  
  (define-key org-agenda-mode-map [(tab)] 'org-agenda-goto)
  (define-key org-agenda-mode-map [(return)] 'org-agenda-switch-to)

Re: org-mode and mode hooks.

[Stefan Monnier]

> org-mode in lisp/textmodes/org.el is derived from outline-mode, but it
> doesn't use define-derived-mode to accomplish this.  Instead, it just
> calls outline-mode as the first thing it does.  (I guess this is
> because org-mode accepts an argument, which define-derived-mode
> doesn't support.)

define-derived-mode doesn't support it because the elisp manual says:

   23.1.1 Major Mode Conventions
   -----------------------------
   
   [...]
   
      * Define a command whose name ends in `-mode', with no arguments,
        that switches to the new mode in the current buffer.  This command
        should set up the keymap, syntax table, and buffer-local variables
        in an existing buffer, without changing the buffer's contents.

Why doesn't org-mode use auto-insert-mode to handle the insertion of
a template?  I.e. just add the relevant template to auto-insert-alist and
let the users decide whether they want it or not by turning on
auto-insert-mode.

> The problem with this approach is that outline-mode's mode hook and
> after-change-major-mode-hook are run at the wrong time.  What about the
> following patch to fix this?

While I think that org-mode should use define-derived-mode, I'm wondering
why it's a problem that after-change-major-mode-hook is run at the
wrong time.  After all, such manual mode derivation (without using
define-derived-mode) is pretty common, so if there's a problem with it, we
should fix it.


        Stefan

Re: org-mode and mode hooks.

[Carsten Dominik]

I will change org-mode to use define-derived-mode.  When I started
writing this mode, some versions of define-derived-mode (maybe the one
in XEmacs, I don't remember) did not allow a BODY argument.  It seems
they now all do, so I will modify org-mode.  I don't really need the
argument to org-mode.



- Carsten

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

>> org-mode in lisp/textmodes/org.el is derived from outline-mode, but it
>> doesn't use define-derived-mode to accomplish this.  Instead, it just
>> calls outline-mode as the first thing it does.  (I guess this is
>> because org-mode accepts an argument, which define-derived-mode
>> doesn't support.)

SM> define-derived-mode doesn't support it because the elisp manual says:

SM>    23.1.1 Major Mode Conventions
SM>    -----------------------------
   
SM>    [...]
   
SM>       * Define a command whose name ends in `-mode', with no arguments,
SM>         that switches to the new mode in the current buffer.  This command
SM>         should set up the keymap, syntax table, and buffer-local variables
SM>         in an existing buffer, without changing the buffer's contents.

SM> Why doesn't org-mode use auto-insert-mode to handle the insertion of
SM> a template?  I.e. just add the relevant template to auto-insert-alist and
SM> let the users decide whether they want it or not by turning on
SM> auto-insert-mode.

>> The problem with this approach is that outline-mode's mode hook and
>> after-change-major-mode-hook are run at the wrong time.  What about the
>> following patch to fix this?

SM> While I think that org-mode should use define-derived-mode, I'm wondering
SM> why it's a problem that after-change-major-mode-hook is run at the
SM> wrong time.  After all, such manual mode derivation (without using
SM> define-derived-mode) is pretty common, so if there's a problem with it, we
SM> should fix it.


SM>         Stefan

-- 
Carsten Dominik <dominik@science.uva.nl>        \ _ /
Sterrenkundig Instituut "Anton Pannekoek"        |X|               _
Kruislaan 403; NL-1098 SJ Amsterdam             /| |\   _  _     _/ \
phone +31 (20) 525-7477; FAX +31 (20) 525-7484 __|o|___/ ~~ \___/    ~~~

Re: org-mode and mode hooks.

[Lute Kamstra]

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

[...]

> While I think that org-mode should use define-derived-mode, I'm wondering
> why it's a problem that after-change-major-mode-hook is run at the
> wrong time.  After all, such manual mode derivation (without using
> define-derived-mode) is pretty common, so if there's a problem with it, we
> should fix it.

It is a problem in combination with a global minor mode defined with
define-global-minor-mode since this change:

2005-05-22  Luc Teirlinck  <teirllm@auburn.edu>

	* emacs-lisp/easy-mmode.el (define-global-minor-mode): Use
	`after-change-major-mode-hook' instead of `find-file-hook'.

See another message of mine in this thread.

Lute.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Lute Kamstra wrote:

   It is a problem in combination with a global minor mode defined with
   define-global-minor-mode since this change:

   2005-05-22  Luc Teirlinck  <teirllm@auburn.edu>

	   * emacs-lisp/easy-mmode.el (define-global-minor-mode): Use
	   `after-change-major-mode-hook' instead of `find-file-hook'.

   See another message of mine in this thread.

I guess `define-global-minor-mode' could use both
`after-change-major-mode-hook' and `find-file-hook', although unless
special care is taken, that could mean that the hook function is run
twice.

But that would not eliminate the need for modes to run
`after-change-major-mode-hook' at their end.  The code before I
changed it failed if the mode was run from a timer or by a process.

The fallback for modes that do not use `after-change-major-mode-hook'
yet is post-command-hook.  Even without `after-change-major-mode-hook'
things should work out OK if you do `M-x org-mode' or visit a file
with `C-x C-f'.  I believe that the only problems should be modes run
from timers or processes.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Lute Kamstra]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Lute Kamstra wrote:
>
>    It is a problem in combination with a global minor mode defined with
>    define-global-minor-mode since this change:
>
>    2005-05-22  Luc Teirlinck  <teirllm@auburn.edu>
>
> 	   * emacs-lisp/easy-mmode.el (define-global-minor-mode): Use
> 	   `after-change-major-mode-hook' instead of `find-file-hook'.
>
>    See another message of mine in this thread.
>
> I guess `define-global-minor-mode' could use both
> `after-change-major-mode-hook' and `find-file-hook', although unless
> special care is taken, that could mean that the hook function is run
> twice.

As I understand it, only the first call of GLOBAL-MODE-buffers will
have effect, because it clears the list variable GLOBAL-MODE-buffers.
after-change-major-mode-hook is run before find-file-hook.  If
after-change-major-mode-hook is run too early, that can have negative
effects.  In the case of global-font-lock-mode, calling
global-font-lock-mode-buffers before setting font-lock-defaults is a
problem.

> But that would not eliminate the need for modes to run
> `after-change-major-mode-hook' at their end.  The code before I
> changed it failed if the mode was run from a timer or by a process.
>
> The fallback for modes that do not use `after-change-major-mode-hook'
> yet is post-command-hook.  Even without `after-change-major-mode-hook'
> things should work out OK if you do `M-x org-mode' or visit a file
> with `C-x C-f'.  I believe that the only problems should be modes run
> from timers or processes.

You seem only to consider the problem that GLOBAL-MODE-buffers is not
called.  It can also be a problem when it is called too early.

Lute.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Lute Kamstra wrote:

   You seem only to consider the problem that GLOBAL-MODE-buffers is not
   called.  It can also be a problem when it is called too early.

I agree that if run-mode-hooks is used, it should be run at the very
end.  That was the convention even before
`after-change-major-mode-hook' was introduced.

Actually, confusion arose, because I was looking at the current code
of org-mode, which does not run `after-change-major-mode-hook' at all.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Lute Kamstra]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Lute Kamstra wrote:
>
>    You seem only to consider the problem that GLOBAL-MODE-buffers is not
>    called.  It can also be a problem when it is called too early.
>
> I agree that if run-mode-hooks is used, it should be run at the very
> end.  That was the convention even before
> `after-change-major-mode-hook' was introduced.
>
> Actually, confusion arose, because I was looking at the current code
> of org-mode, which does not run `after-change-major-mode-hook' at all.

It runs after-change-major-mode-hook indirectly: org-mode calls
outline-mode, which calls run-mode-hooks, which runs
after-change-major-mode-hook.  That way, global-font-lock-mode-buffers
is first called before org-mode sets font-lock-defaults.

Lute.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Lute Kamstra wrote:

   It runs after-change-major-mode-hook indirectly: org-mode calls
   outline-mode, which calls run-mode-hooks, which runs
   after-change-major-mode-hook.  That way, global-font-lock-mode-buffers
   is first called before org-mode sets font-lock-defaults.

Sorry, I should have thought of this and I should have read your
original message more carefully.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   While I think that org-mode should use define-derived-mode, I'm wondering
   why it's a problem that after-change-major-mode-hook is run at the
   wrong time.

Because users of after-change-major-mode-hook should be able to rely
on the fact that it is run at the very end of the mode.  Hooks should
be run as documented.

>From `(elisp)Mode Hooks':

 -- Variable: after-change-major-mode-hook
     Every major mode function should run this normal hook at its very
     end.  It normally does not need to do so explicitly.  Indeed, a
     major mode function should normally run its mode hook with
     `run-mode-hooks' as the very last thing it does and
     `run-mode-hooks' runs `after-change-major-mode-hook' at its very
     end.


   After all, such manual mode derivation (without using
   define-derived-mode) is pretty common, so if there's a problem with it, we
   should fix it.

I believe that we plan to eventually make all modes run
`after-change-major-mode-hook', usually as a byproduct of making them
run their mode hook with `run-mode-hooks'.  If we see one that does
not, we fix it.  In the meantime, if we use `after-change-mode-hook'
in a very general way, we need some fallback for modes that do not yet
use it.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    I believe that we plan to eventually make all modes run
    `after-change-major-mode-hook', usually as a byproduct of making them
    run their mode hook with `run-mode-hooks'.

If there are any major modes that fail to use run-mode-hooks, we want
to fix them now.  It would be good for someone to look for them.

However, users also have major modes, and we probably cannot expect
all of them to be changed for many years.  So it will not be possible
to absolutely rely on all major modes to run after-change-major-mode-hook
at the end.  Features that depend on this will fail to work in those
user-defined major modes that have not been updated.

In some cases that will not be a real problem.  When it is ok to say,
"Your major mode has a bug and needs to be corrected," we can say it.
For instance, if a global minor mode doesn't turn on in the user's
major mode because of this, we can respond that way.

However, when a feature really needs to be reliable, it should not use
after-change-major-mode-hook now or for the next few years.  I will
document that in the manual.

    I do not know whether there are a lot of derived modes that do not
    follow the convention.  The most satisfactory solution would obviously
    be to make them all follow the conventions.  If this would be
    unrealistic, possibilities include to come up with a better fallback
    alternative or to simply revert my change.

If the change provides advantages, we have no reason to revert it.
What is the worst thing that can happen when a derived mode fails to
run after-change-major-mode-hook?  Only that it won't enable certain
global minor modes.  If that bothers the user, he can fix his derived
mode to follow the latest conventions.

However, if we can easily make global minor modes work more reliably,
we may as well do so.  Perhaps by using find-file-hook *as well as*
after-change-major-mode-hook.  Why not?

Re: org-mode and mode hooks.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     I believe that we plan to eventually make all modes run
>     `after-change-major-mode-hook', usually as a byproduct of making them
>     run their mode hook with `run-mode-hooks'.
>
> If there are any major modes that fail to use run-mode-hooks, we want
> to fix them now.  It would be good for someone to look for them.

I'll do that.

> However, users also have major modes, and we probably cannot expect
> all of them to be changed for many years.  So it will not be possible
> to absolutely rely on all major modes to run after-change-major-mode-hook
> at the end.  Features that depend on this will fail to work in those
> user-defined major modes that have not been updated.
>
> In some cases that will not be a real problem.  When it is ok to say,
> "Your major mode has a bug and needs to be corrected," we can say it.
> For instance, if a global minor mode doesn't turn on in the user's
> major mode because of this, we can respond that way.
>
> However, when a feature really needs to be reliable, it should not use
> after-change-major-mode-hook now or for the next few years.  I will
> document that in the manual.
>
>     I do not know whether there are a lot of derived modes that do not
>     follow the convention.  The most satisfactory solution would obviously
>     be to make them all follow the conventions.  If this would be
>     unrealistic, possibilities include to come up with a better fallback
>     alternative or to simply revert my change.
>
> If the change provides advantages, we have no reason to revert it.
> What is the worst thing that can happen when a derived mode fails to
> run after-change-major-mode-hook?  Only that it won't enable certain
> global minor modes.

Font Lock mode is an example of such a global minor mode.  My guess is
that over 90% of Emacs' users enable global-font-lock-mode.  It's
pretty bad if enabling that breaks for some modes.

> If that bothers the user, he can fix his derived mode to follow the
> latest conventions.
>
> However, if we can easily make global minor modes work more reliably,
> we may as well do so.  Perhaps by using find-file-hook *as well as*
> after-change-major-mode-hook.  Why not?

That solves the problem that global minor modes are not enabled for
major modes that do not use run-mode-hooks.  However, it doesn't solve
the problem that a global minor mode is enabled *too soon* because a
derived major mode doesn't delay the running of hooks of its parent
mode.  (Note: derived modes defined with define-derived-mode always
delay the running of hooks.)

Lute.

Re: org-mode and mode hooks.

[Lute Kamstra]

Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>> If there are any major modes that fail to use run-mode-hooks, we want
>> to fix them now.  It would be good for someone to look for them.
>
> I'll do that.

I've fixed some 100 major modes by grepping lisp files for
"(.*run-hooks.*mode".

I didn't fix Gnus.  Maybe someone here can ask the Gnus developers to
do that?

Of course my method failed to find any major modes that don't run mode
hooks at all.  Maybe someone else could try to fix those?

Lute.

Re: org-mode and mode hooks.

[Richard Stallman]

    Of course my method failed to find any major modes that don't run mode
    hooks at all.  Maybe someone else could try to fix those?

Perhaps one could find these easily with a Lisp program
that searches for a defun whose start matches "(defun [-a-z]+-mode "
and that does not contain "(run-".

Re: org-mode and mode hooks.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     Of course my method failed to find any major modes that don't run mode
>     hooks at all.  Maybe someone else could try to fix those?
>
> Perhaps one could find these easily with a Lisp program
> that searches for a defun whose start matches "(defun [-a-z]+-mode "
> and that does not contain "(run-".

I did this.  I think I found all and fixed most modes that did not run
mode hooks.  There are a few exceptions involving special major modes
like terminal-mode in lisp/terminal.el where the mode hook is run
outside of the major mode function by the caller of the major mode
function.  I figured that fixing those complex cases is too
error-prone so I left them alone.

Lute.

Re: org-mode and mode hooks.

[Richard M. Stallman]

    > Perhaps one could find these easily with a Lisp program
    > that searches for a defun whose start matches "(defun [-a-z]+-mode "
    > and that does not contain "(run-".

    I did this.  I think I found all and fixed most modes that did not run
    mode hooks.

Thanks.

      There are a few exceptions involving special major modes
    like terminal-mode in lisp/terminal.el where the mode hook is run
    outside of the major mode function by the caller of the major mode
    function.  I figured that fixing those complex cases is too
    error-prone so I left them alone.

Ok.

Re: org-mode and mode hooks.

[Richard Stallman]

    > If there are any major modes that fail to use run-mode-hooks, we want
    > to fix them now.  It would be good for someone to look for them.

    I'll do that.

I see you have already done it.  Thank you.

We also need to find other modes that are effectively "derived" and
ought to use delay-mode-hooks.  I think one could write a Lisp program
that would search for a match for "([-a-z]+-mode " within a defun that
starts with "(defun [-a-z]+-mode ".

Re: org-mode and mode hooks.

[Juri Linkov]

> We also need to find other modes that are effectively "derived" and
> ought to use delay-mode-hooks.  I think one could write a Lisp program
> that would search for a match for "([-a-z]+-mode " within a defun that
> starts with "(defun [-a-z]+-mode ".

For searching in Lisp structures I use the following piece of code
(not a separate package yet):

(defun emacs-lisp-grep (dir expr)
  (if (file-directory-p dir)
      (mapcan
       (lambda (filename)
         (let ((fullname (concat dir filename)))
           (cond
            ((member filename '("." "..")) nil)
            ((file-directory-p fullname) (emacs-lisp-grep fullname expr))
            ((and (file-exists-p fullname) (string-match "\.el$" fullname))
             (with-temp-buffer
               (insert-file-contents fullname)
               (let (s res)
                 (condition-case nil
                     (while t
                       (setq s (read (current-buffer)))
                       (let ((ret (funcall expr s)))
                         (if ret (setq res (cons (list filename s ret) res)))))
                   (error res))
                 res))))))
       (directory-files (setq dir (file-name-as-directory dir)) nil nil t))))

Using it for searching for "[-a-z]+-mode " with the code:

(mapc (lambda (elt)
        (insert (format "%s:%s %S\n" (nth 0 elt) (cadr (nth 1 elt)) (nth 2 elt))))
      (emacs-lisp-grep
       "./emacs/lisp"
       (lambda (expr)
         (and (eq 'defun (car expr))
              (string-match "[-a-z]+-mode\\'" (format "%s" (cadr expr)))
              (delq nil
                    (mapcar (lambda (elt)
                              (if (and (not (eq elt (cadr expr)))
                                       (not (memq elt '(major-mode)))
                                       (symbolp elt)
                                       (string-match "[-a-z]+-mode\\'" (format "%s" elt))) elt))
                            (flatten-list (cddr expr))))))))

(defun flatten-list (list)
  "Return a flat list."
  (cond
   ((consp list) (apply 'append (mapcar 'flatten-list list)))
   (list (list list))))

produces the output (I guess the search condition needs more tweaking
to produce more desirable output):

wdired.el:wdired-change-to-dired-mode (dired-mode)
wdired.el:wdired-change-to-wdired-mode (wdired-mode)
iimage.el:turn-off-iimage-mode (iimage-mode)
iimage.el:turn-on-iimage-mode (iimage-mode)
url-handlers.el:url-handlers-set-buffer-mode (auto-save-mode)
org.el:org-agenda-mode (org-agenda-follow-mode)
org.el:org-mode (outline-mode)
conf-mode.el:conf-xdefaults-mode (conf-colon-mode)
conf-mode.el:conf-ppd-mode (conf-colon-mode)
conf-mode.el:conf-colon-mode (conf-mode conf-unix-mode)
conf-mode.el:conf-space-mode (conf-unix-mode)
conf-mode.el:conf-javaprop-mode (conf-mode)
conf-mode.el:conf-windows-mode (conf-mode)
conf-mode.el:conf-unix-mode (conf-mode)
conf-mode.el:conf-mode (conf-javaprop-mode conf-colon-mode conf-space-mode conf-windows-mode conf-unix-mode)
tex-mode.el:tex-mode (tex-guess-mode)
tex-mode.el:tex-guess-mode (tex-default-mode slitex-mode latex-mode plain-tex-mode)
table.el:*table--cell-describe-mode (describe-mode)
reftex-toc.el:reftex-toc-mode (transient-mark-mode)
reftex-global.el:reftex-isearch-minor-mode (old-reftex-isearch-minor-mode old-reftex-isearch-minor-mode reftex-mode reftex-mode)
picture.el:picture-mode (picture-mode-old-major-mode)
nroff-mode.el:electric-nroff-mode (nroff-mode nroff-electric-mode nroff-electric-mode nroff-electric-mode nroff-electric-mode)
flyspell.el:flyspell-mode (old-flyspell-mode old-flyspell-mode)
flyspell.el:flyspell-prog-mode (flyspell-mode)
enriched.el:enriched-after-change-major-mode (enriched-mode enriched-mode)
enriched.el:enriched-before-change-major-mode (enriched-mode)
xterm.el:xterm-rxvt-set-background-mode (frame-background-mode frame-background-mode frame-set-background-mode)
rxvt.el:rxvt-set-background-mode (frame-background-mode frame-background-mode frame-set-background-mode)
gdb-ui.el:gdb-locals-mode (gud-minor-mode)
gdb-ui.el:gdb-frames-mode (font-lock-mode gud-minor-mode)
gdb-ui.el:gdb-breakpoints-mode (gud-minor-mode)
xscheme.el:xscheme-enter-debugger-mode (scheme-debugger-mode scheme-interaction-mode scheme-interaction-mode)
xscheme.el:xscheme-enter-interaction-mode (scheme-interaction-mode scheme-debugger-mode scheme-interaction-mode)
xscheme.el:exit-scheme-interaction-mode (scheme-interaction-mode xscheme-previous-mode)
xscheme.el:scheme-interaction-mode (previous-mode xscheme-previous-mode xscheme-previous-mode previous-mode)
tcl.el:tcl-auto-fill-mode (auto-fill-mode)
sql.el:sql-interactive-mode (comint-mode)
octave-inf.el:inferior-octave-mode (comint-mode)
inf-lisp.el:inferior-lisp-mode (comint-mode)
cwarn.el:turn-on-cwarn-mode (cwarn-mode)
cpp.el:cpp-edit-mode (auto-save-mode)
cperl-mode.el:cperl-mode (cperl-use-major-mode abbrev-mode cperl-old-auto-fill-mode cperl-old-auto-fill-mode auto-fill-mode auto-fill-mode cperl-old-auto-fill-mode perl-mode font-lock-mode)
compile.el:compilation-mode (name-of-mode name-of-mode)
cc-mode.el:pike-mode (c-initialize-cc-mode abbrev-mode)
cc-mode.el:idl-mode (c-initialize-cc-mode)
cc-mode.el:java-mode (c-initialize-cc-mode abbrev-mode)
cc-mode.el:objc-mode (c-initialize-cc-mode abbrev-mode)
cc-mode.el:c-mode (c-initialize-cc-mode abbrev-mode)
cc-mode.el:c-initialize-cc-mode (c-buffer-is-cc-mode c-leave-cc-mode-mode c-mode)
cc-mode.el:c-leave-cc-mode-mode (c-buffer-is-cc-mode)
antlr-mode.el:antlr-mode (c-initialize-cc-mode c-buffer-is-cc-mode)
dunnet.el:dun-mode (text-mode)
doctor.el:doctor-mode (text-mode)
decipher.el:decipher-stats-mode (indent-tabs-mode)
decipher.el:decipher-mode (indent-tabs-mode)
lazy-lock.el:lazy-lock-mode (font-lock-mode font-lock-support-mode font-lock-mode)
fast-lock.el:fast-lock-mode (font-lock-mode font-lock-support-mode font-lock-mode)
ange-ftp.el:internal-ange-ftp-mode (comint-mode)
ange-ftp.el:ange-ftp-set-buffer-mode (auto-save-mode)
mh-e.el:mh-set-scan-mode (mh-showing-mode)
rmailedit.el:rmail-edit-mode (text-mode)
viper.el:set-viper-state-in-major-mode (viper-mode)
viper.el:viper-mode (fundamental-mode)
viper.el:toggle-viper-mode (viper-mode viper-mode viper-mode)
viper-cmd.el:viper-start-R-mode (overwrite-mode)
vip.el:vip-change-mode (new-mode new-mode vip-current-mode new-mode vi-mode vip-current-mode insert-mode new-mode insert-mode vip-current-mode emacs-mode vip-help-in-insert-mode new-mode emacs-mode vip-current-mode new-mode)
vi.el:vi-mode (vi-mode-old-major-mode vi-mode-old-major-mode)
vi.el:vi-back-to-old-mode (vi-mode-old-major-mode vi-mode-old-major-mode)
vi.el:vi-switch-mode (help-mode)
tpu-edt.el:tpu-toggle-overwrite-mode (overwrite-mode overwrite-mode overwrite-mode)
crisp.el:crisp-mode (transient-mark-mode transient-mark-mode scroll-all-mode)
re-builder.el:reb-quit-subexp-mode (reb-subexp-mode)
re-builder.el:reb-enter-subexp-mode (reb-subexp-mode)
helper.el:Helper-describe-mode (help-mode)
eldoc.el:turn-on-eldoc-mode (eldoc-mode)
edebug.el:edebug-eval-mode (lisp-interaction-mode)
edebug.el:edebug-Continue-fast-mode (edebug-set-mode)
edebug.el:edebug-continue-mode (edebug-set-mode)
edebug.el:edebug-Trace-fast-mode (edebug-set-mode)
edebug.el:edebug-trace-mode (edebug-set-mode)
edebug.el:edebug-Go-nonstop-mode (edebug-set-mode)
edebug.el:edebug-go-mode (edebug-set-mode)
edebug.el:edebug-next-mode (edebug-set-mode)
edebug.el:edebug-step-mode (edebug-set-mode)
edebug.el:edebug-set-mode (edebug-execution-mode edebug-next-execution-mode)
calendar.el:calendar-mode (indent-tabs-mode)
calendar.el:describe-calendar-mode (help-mode)
woman.el:woman-mode (Man-mode Man-mode)
whitespace.el:whitespace-check-whitespace-mode (whitespace-mode whitespace-mode)
vc.el:vc-dired-toggle-terse-mode (vc-dired-mode vc-dired-terse-mode vc-dired-terse-mode vc-dired-terse-mode)
type-break.el:type-break-mode (type-break-mode-line-message-mode)
term.el:term-line-mode (term-in-char-mode)
term.el:term-char-mode (term-in-line-mode)
term.el:term-mode (indent-tabs-mode term-insert-mode)
tar-mode.el:tar-subfile-mode (auto-save-mode)
speedbar.el:speedbar-mode (speedbar-frame-mode auto-show-mode)
speedbar.el:speedbar-frame-mode (speedbar-mode speedbar-mode)
simple.el:binary-overwrite-mode (overwrite-mode overwrite-mode)
ses.el:ses-mode (transient-mark-mode)
scroll-bar.el:scroll-bar-mode (set-scroll-bar-mode)
scroll-bar.el:set-scroll-bar-mode (scroll-bar-mode scroll-bar-mode scroll-bar-mode scroll-bar-mode)
man.el:Man-mode (auto-fill-mode)
ledit.el:ledit-from-lisp-mode (ledit-mode)
ledit.el:ledit-mode (lisp-mode ledit-from-lisp-mode)
isearch.el:isearch-mode (isearch-slow-terminal-mode isearch-slow-terminal-mode)
info-look.el:info-lookup-change-mode (info-lookup-mode)
info-look.el:info-lookup-select-mode (info-lookup-mode info-lookup-mode info-lookup-mode info-lookup-mode info-lookup-mode)
ibuffer.el:ibuffer-mode (show-paren-mode show-paren-mode ibuffer-sorting-mode ibuffer-default-sorting-mode)
ibuf-ext.el:ibuffer-toggle-sorting-mode (ibuffer-sorting-mode ibuffer-sorting-mode)
ibuf-ext.el:ibuffer-set-filter-groups-by-mode (ibuffer-mode)
ibuf-ext.el:ibuffer-mouse-filter-by-mode (ibuffer-interactive-filter-by-mode)
ibuf-ext.el:ibuffer-auto-mode (ibuffer-mode)
hilit-chg.el:highlight-changes-mode (new-highlight-changes-mode new-highlight-changes-mode new-highlight-changes-mode)
hi-lock.el:hi-lock-mode (font-lock-mode font-lock-mode font-lock-mode)
hexl.el:hexl-mode (hexl-mode-old-major-mode hexl-mode-old-major-mode hexl-mode-old-ruler-mode hexl-mode-old-ruler-mode ruler-mode ruler-mode hexl-mode-old-hl-line-mode hexl-mode-old-hl-line-mode hl-line-mode hl-line-mode)
help.el:describe-minor-mode (minor-mode minor-mode minor-mode minor-mode minor-mode minor-mode minor-mode minor-mode minor-mode)
help.el:describe-mode (pretty-minor-mode pretty-minor-mode pretty-minor-mode pretty-minor-mode pretty-minor-mode pretty-minor-mode pretty-minor-mode)
help-mode.el:help-mode (view-mode)
fringe.el:fringe-mode (set-fringe-mode)
fringe.el:set-fringe-mode (fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode fringe-mode)
font-core.el:font-lock-change-mode (font-lock-mode)
follow.el:turn-off-follow-mode (follow-mode)
follow.el:turn-on-follow-mode (follow-mode)
files.el:normal-mode (default-major-mode fundamental-mode set-auto-mode)
faces.el:frame-set-background-mode (bg-mode frame-background-mode old-bg-mode background-mode bg-mode old-bg-mode background-mode bg-mode)
emerge.el:emerge-set-merge-mode (emerge-fast-mode emerge-fast-mode emerge-edit-mode)
emerge.el:emerge-edit-mode (emerge-mode emerge-fast-mode)
emerge.el:emerge-fast-mode (emerge-mode emerge-edit-mode)
ehelp.el:electric-describe-mode (describe-mode)
ehelp.el:electric-help-mode (view-mode)
ediff-merg.el:ediff-set-merge-mode (normal-mode)
cus-edit.el:customize-mode (custom-group-of-mode custom-group-of-mode custom-group-of-mode)
completion.el:dynamic-completion-mode (completion-setup-fortran-mode)
calculator.el:calculator-radix-mode (calculator-radix-input-mode calculator-radix-output-mode)
bindings.el:mode-line-auto-fill-mode (auto-fill-mode)
bindings.el:mode-line-abbrev-mode (abbrev-mode)
autorevert.el:turn-on-auto-revert-tail-mode (auto-revert-tail-mode)
autorevert.el:turn-on-auto-revert-mode (auto-revert-mode)
array.el:array-mode (overwrite-mode)
arc-mode.el:archive-mode (default-major-mode auto-save-mode archive-subfile-mode archive-subfile-mode)
gnus-dired.el:turn-on-gnus-dired-mode (gnus-dired-mode)
gnus-dired.el:gnus-dired-mode (dired-mode gnus-add-minor-mode)
mml.el:mml-mode (gnus-add-minor-mode)
gnus-undo.el:gnus-undo-mode (gnus-add-minor-mode)
gnus-topic.el:gnus-topic-mode (gnus-group-mode gnus-add-minor-mode)
gnus-sum.el:gnus-dead-summary-mode (gnus-summary-mode gnus-add-minor-mode)
gnus-sum.el:gnus-summary-mode (turn-on-gnus-mailing-list-mode summary-mode)
gnus-start.el:gnus-slave-mode (gnus-add-minor-mode)
gnus-salt.el:gnus-tree-mode (tree-mode)
gnus-salt.el:gnus-binary-mode (gnus-summary-mode gnus-add-minor-mode)
gnus-salt.el:gnus-pick-mode (gnus-summary-mode gnus-add-minor-mode)
gnus-ml.el:gnus-mailing-list-mode (gnus-summary-mode gnus-add-minor-mode)
gnus-ml.el:turn-on-gnus-mailing-list-mode (gnus-mailing-list-mode)
gnus-group.el:gnus-group-mode (group-mode gnus-undo-mode gnus-slave-mode)
gnus-gl.el:gnus-grouplens-mode (gnus-summary-mode menu-bar-mode gnus-add-minor-mode)
gnus-draft.el:gnus-draft-mode (gnus-summary-mode gnus-add-minor-mode mml-mode)
gnus-art.el:gnus-article-mode (article-mode)
gnus-agent.el:gnus-agent-mode (gnus-group-mode)
calc.el:calc-trail-mode (fundamental-mode)
calc.el:calc-mode (calc-algebraic-mode)
calc-mode.el:calc-units-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-ext-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-alg-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-bin-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-default-simplify-mode (calc-set-simplify-mode calc-num-simplify-mode calc-no-simplify-mode calc-bin-simplify-mode calc-alg-simplify-mode calc-ext-simplify-mode calc-units-simplify-mode)
calc-mode.el:calc-num-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-no-simplify-mode (calc-set-simplify-mode)
calc-mode.el:calc-set-simplify-mode (calc-change-mode calc-simplify-mode calc-simplify-mode calc-simplify-mode)
calc-mode.el:calc-matrix-mode (calc-change-mode)
calc-mode.el:calc-infinite-mode (calc-change-mode calc-change-mode)
calc-mode.el:calc-symbolic-mode (calc-change-mode)
calc-mode.el:calc-algebraic-mode (calc-change-mode calc-incomplete-algebraic-mode calc-incomplete-algebraic-mode calc-change-mode calc-incomplete-algebraic-mode calc-incomplete-algebraic-mode)
calc-mode.el:calc-total-algebraic-mode (calc-algebraic-mode calc-algebraic-mode calc-change-mode calc-algebraic-mode calc-incomplete-algebraic-mode)
calc-mode.el:calc-mode-record-mode (calc-change-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode calc-mode-save-mode)
calc-math.el:calc-radians-mode (calc-change-mode calc-angle-mode)
calc-math.el:calc-degrees-mode (calc-change-mode calc-angle-mode calc-radians-mode calc-hms-mode)
calc-frac.el:calc-frac-mode (calc-change-mode)
calc-forms.el:calc-hms-mode (calc-change-mode calc-angle-mode)
calc-ext.el:calc-change-mode (calc-mode-save-mode calc-mode-save-mode)
calc-cplx.el:calc-polar-mode (calc-complex-mode calc-change-mode calc-complex-mode calc-change-mode calc-complex-mode)
calc-alg.el:calc-modify-simplify-mode (calc-simplify-mode calc-simplify-mode calc-simplify-mode calc-simplify-mode)

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: org-mode and mode hooks.

[Lute Kamstra]

Juri Linkov <juri@jurta.org> writes:

>> We also need to find other modes that are effectively "derived" and
>> ought to use delay-mode-hooks.  I think one could write a Lisp
>> program that would search for a match for "([-a-z]+-mode " within a
>> defun that starts with "(defun [-a-z]+-mode ".
>
> For searching in Lisp structures I use the following piece of code
> (not a separate package yet):

Thanks Juri!  I used your code to find "manually defined" derived
modes.  I think I found and fixed most of them.

Lute.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   While I think that org-mode should use define-derived-mode, I'm wondering
   why it's a problem that after-change-major-mode-hook is run at the
   wrong time.  After all, such manual mode derivation (without using
   define-derived-mode) is pretty common, so if there's a problem with it, we
   should fix it.

I my previous reply to the last sentence, I forgot that we were
talking about timing and not about the actual running or not running.
But the fact that run-mode-hooks should be run _at the very end_ of a
mode has been documented ever since run-mode-hooks itself was documented.

In my replies on this issue, I was assuming that there was a problem
with org-mode as it is right now (which could only be that
after-change-major-mode-hook was not run), whereas on closer reading,
I see that the discussed problem would only occur after a change to
org-mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Now that I have taken a closer look at things, it is indeed the case
that although post-command-hook will take care of non-derived major
modes that do not follow conventions (and are not run from timers or
processes) and after-change-major-mode-hook will take care of all
modes that follow the conventions, none of the two will take care of
derived modes that do not follow the conventions.  (Where a derived
mode is defined as a major mode that runs another major mode inside
its body).

I do not know whether there are a lot of derived modes that do not
follow the convention.  The most satisfactory solution would obviously
be to make them all follow the conventions.  If this would be
unrealistic, possibilities include to come up with a better fallback
alternative or to simply revert my change.  If we do the latter, I
could fix the original Buffer Menu problem (and potential future
similar problems with reverting non-file buffers) by making
`auto-revert-buffers' run `global-font-lock-mode-buffers'.  This does
not seem very attractive, because the underlying problem with timers
or processes running modes would remain.  I also do not immediately
understand why find-file-hook could not potentially run too early too.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

> I do not know whether there are a lot of derived modes that do not
> follow the convention.

I know there are several.  Derived modes are pretty common, whereas the use
of define-derived-mode was pretty rare until recently.

W.r.t the code bundled with Emacs, there's often no problem (tho, sometimes
there is, because it's not always trivial to fix the code to use
define-derived-mode), but for the unbundled packages, the change:

2005-05-22  Luc Teirlinck  <teirllm@auburn.edu>

	* emacs-lisp/easy-mmode.el (define-global-minor-mode): Use
	`after-change-major-mode-hook' instead of `find-file-hook'.

introduced a bug where (typically) global-font-lock chooses the wrong
keywords (the ones of the parent mode rather than ones of the actual major
mode).


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   W.r.t the code bundled with Emacs, there's often no problem (tho, sometimes
   there is, because it's not always trivial to fix the code to use
   define-derived-mode), but for the unbundled packages, the change:

   2005-05-22  Luc Teirlinck  <teirllm@auburn.edu>

	   * emacs-lisp/easy-mmode.el (define-global-minor-mode): Use
	   `after-change-major-mode-hook' instead of `find-file-hook'.

   introduced a bug where (typically) global-font-lock chooses the wrong
   keywords (the ones of the parent mode rather than ones of the actual major
   mode).

Well, it fixed one bug, but it created more occurrences of another existing
bug.  The bug it fixed relates to timers and processes calling mode
functions.  The other existing bug that it creates more occurrences of
is that if _anything_ enables font-lock before the final mode is into
place (for instance a function added by a user to a hook) font-lock
chooses the wrong keywords.  Granted, my change makes this much more
likely to happen (of course).  But even without my change it can happen.

The following changes fix both bugs, by using post-command-hook to
check whether font-lock was enabled for the correct mode and adapting
the turn-on function to be able to correct it.  I still will have to
see whether this automatically fixes the second bug for other
functions defined with define-global-mode, or whether some of them
need a slightly different turn-on function too.

===File ~/font-core-diff====================================
*** font-core.el	22 May 2005 16:46:07 -0500	1.28
--- font-core.el	25 May 2005 21:57:50 -0500	
***************
*** 284,291 ****
      (let (inhibit-quit)
        (turn-on-font-lock))))
  
  (easy-mmode-define-global-mode
!  global-font-lock-mode font-lock-mode turn-on-font-lock-if-enabled
   :extra-args (dummy))
  
  ;;; End of Global Font Lock mode.
--- 284,295 ----
      (let (inhibit-quit)
        (turn-on-font-lock))))
  
+ (defun turn-on-font-lock-as-appropriate ()
+   (let (font-lock-set-defaults)
+     (turn-on-font-lock-if-enabled)))
+ 
  (easy-mmode-define-global-mode
!  global-font-lock-mode font-lock-mode turn-on-font-lock-as-appropriate
   :extra-args (dummy))
  
  ;;; End of Global Font Lock mode.
============================================================

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	22 May 2005 16:50:33 -0500	1.63
--- easy-mmode.el	25 May 2005 22:37:54 -0500	
***************
*** 263,268 ****
--- 263,271 ----
  ;;; make global minor mode
  ;;;
  
+ (defvar easy-mmode-stored-mode nil)
+ (make-variable-buffer-local 'easy-mmode-stored-mode)
+ 
  ;;;###autoload
  (defalias 'easy-mmode-define-global-mode 'define-global-minor-mode)
  ;;;###autoload
***************
*** 278,283 ****
--- 281,287 ----
  	 (group nil)
  	 (extra-args nil)
  	 (buffers (intern (concat global-mode-name "-buffers")))
+ 	 (check-buffers (intern (concat "check-" global-mode-name "-buffers")))
  	 (cmmh (intern (concat global-mode-name "-cmmh"))))
  
      ;; Check keys.
***************
*** 307,314 ****
--- 311,320 ----
  	 (if ,global-mode
  	     (progn
  	       (add-hook 'after-change-major-mode-hook ',buffers)
+ 	       (add-hook 'find-file-hook ',buffers)
  	       (add-hook 'change-major-mode-hook ',cmmh))
  	   (remove-hook 'after-change-major-mode-hook ',buffers)
+ 	   (remove-hook 'find-file-hook ',buffers)
  	   (remove-hook 'change-major-mode-hook ',cmmh))
  
  	 ;; Go through existing buffers.
***************
*** 325,341 ****
  
         ;; 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))))
  
  ;;;
--- 331,360 ----
  
         ;; The function that calls TURN-ON in each buffer.
         (defun ,buffers ()
! 	 (dolist (buf ,buffers)
! 	   (when (buffer-live-p buf)
! 	     (with-current-buffer buf
! 	       (unless ,mode
! 		 (setq easy-mmode-stored-mode major-mode)
! 		 (,turn-on))))))
!        (put ',buffers 'definition-name ',global-mode)
! 
!        (defun ,check-buffers ()
! 	 (remove-hook 'post-command-hook ',check-buffers)
  	 (while ,buffers
  	   (let ((buf (pop ,buffers)))
  	     (when (buffer-live-p buf)
! 	       (with-current-buffer buf
! 		 (if ,mode
! 		     (unless (eq easy-mmode-stored-mode major-mode)
! 		       (,mode -1)
! 		       (,turn-on))
! 		   (,turn-on)))))))
  
         ;; The function that catches kill-all-local-variables.
         (defun ,cmmh ()
  	 (add-to-list ',buffers (current-buffer))
! 	 (add-hook 'post-command-hook ',check-buffers))
         (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
============================================================

Re: org-mode and mode hooks.

[Stefan Monnier]

> The following changes fix both bugs, by using post-command-hook to
> check whether font-lock was enabled for the correct mode and adapting
> the turn-on function to be able to correct it.  I still will have to
> see whether this automatically fixes the second bug for other
> functions defined with define-global-mode, or whether some of them
> need a slightly different turn-on function too.

Can you explain the change?
I really don't like the

  (defun turn-on-font-lock-as-appropriate ()
    (let (font-lock-set-defaults)
      (turn-on-font-lock-if-enabled)))

since it may cause font-lock-keywords to be reset from font-lock-defaults,
thus throwing away keywords added via font-lock-add-keywords since.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   Can you explain the change?
   I really don't like the

     (defun turn-on-font-lock-as-appropriate ()
       (let (font-lock-set-defaults)
	 (turn-on-font-lock-if-enabled)))

   since it may cause font-lock-keywords to be reset from font-lock-defaults,
   thus throwing away keywords added via font-lock-add-keywords since.

I believe that I want to make one change to my original patch, namely
to add `check-buffers' rather than `buffes' to find-file-hook.  So I
will explain things using this additional change.

The problem that binding font-lock-defaults to nil tries to solve is
that (with org-mode's yesterday's version), after font-lock was turned
on in outline-mode, I could not get it correct for org-mode by
disabling and enabling font-lock-mode, font-lock just kept using
outline-mode's font-lock settings.  Note that some mode hooks
(comint-mode-hook, for instance) enable font-lock by default.  So it
would seem that a mode derived from comint-mode in a non-standard way
_already_ would have had org-mode type problems _before_ my patch to
easy-mmode.

`turn-on-font-lock-as-appropriate' would only be used by
`define-global-minor-mode'.  What I want to do is the following.  When
`after-change-major-mode' enables font-lock, it records what mode it
did it for in easy-mmode-stored-mode.  It does not remove any buffers
from the list to check. When then either find-file-hook or
post-command-hook run, they do remove buffers, but, if font-lock is
already enabled, they first check easy-mmode-stored-mode.  If it is
non-nil and different from `major-mode', we know that
after-change-major-mode ran to soon.  (Maybe we should also print an
error message in this case, which my current patch does not).  We
_must_ correct this.  (I do not know how to do that without setting
font-lock-defaults to nil.)  If it is nil and font-lock is already
enabled, we know that something else set font-lock, maybe a major mode
body or a user hook.  Here, we do not know whether or not it was set
for the right mode.  To be sure, we undo the setting anyway and do
things over to be sure that it is set for the right mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

I understand better now, thank you.

> after-change-major-mode ran to soon.  (Maybe we should also print an
> error message in this case, which my current patch does not).

Yes, I think it's more important to signal a clear warning/error than to try
and auto-fix the problem.

BTW, why use "check-" as a prefix (rather than a "-check-" infix or suffix),
thus potentially breaking the usual namespace conventions?

> (I do not know how to do that without setting
> font-lock-defaults to nil.)

Yes, in that case we're screwed either way, which is why we need to signal
a warning/error so someone can fix the problem at its source.

The auto-fix you suggest has the shortcoming I mentioned but is probably the
right "best effort" solution because other solutions probably suffer from
more common/serious problems.  Please add some comments explaining how this
code is only used to double check erroneous situations and to try and
salvage such "desperate" cases (the presence of a warning/error should
already make the code more understandable).


        Stefan


PS: By warning/error I'm not sure what I mean, but it should be more
obnoxious than a (message "foo") and less than (error "foo").
Probably something like (progn (message "foo") (ding) (sit-for 1)).

Re: org-mode and mode hooks.

[Luc Teirlinck]

Here are my proposed changes to easy-mmode.el.  Unlike my previous
proposed patch, they require some cooperation from the individual 
minor modes, but this seemed necessary to catch (and attempt to work
around) all possible ways to mess up the major mode for which the
minor mode is set up.  The only two involved modes I could find using
grepping were font-lock-mode and cwarn mode.  The patches below adapt
both.

Michael could check whether or not after my patches (the one to
cwarn.el is irrelevant for him), just disabling and enabling font-lock
is sufficient for him.  If not, then I have the impression that I
should not even try to correct this, as maybe certain font-lock-face
properties might be mode independent.

When testing this patch, do not rely on stuff like `C-h v
font-lock-set-defaults', because this is currently buggy.
Do `M-: font-lock-set-defaults', or similar, instead.

To test the error message, one could use something like:

(defun foo-mode ()
  (interactive)
  (outline-mode)
  (setq major-mode 'foo-mode)
  (setq mode-name "FOO"))

M-x foo-mode

I can install the patches below if desired.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	22 May 2005 16:50:33 -0500	1.63
--- easy-mmode.el	27 May 2005 11:00:47 -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,294 ----
  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 the minor mode's set-up depends on the major mode, the function that
! does the initial set-up should record the value of the variable `major-mode'
! in the variable MODE-stored-mode.  This is used to catch bugs where MODE is
! set up for the wrong mode. `define-global-minor-mode' provides a defvar
! for MODE-stored-hook, with initial value nil, and makes it automatically
! buffer-local.  If MODE's set-up is independent of the major mode,
! the value of the variable should stay nil."
    (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")))
! 	 (buffers-check (intern (concat global-mode-name "-check-buffers")))
! 	 (cmmh (intern (concat global-mode-name "-cmmh")))
! 	 (stored-mode (intern (concat (symbol-name mode) "-stored-mode"))))
  
      ;; Check keys.
      (while (keywordp (car keys))
***************
*** 294,299 ****
--- 304,311 ----
  				"-mode\\'" "" (symbol-name mode))))))
  
      `(progn
+        (defvar ,stored-mode nil)
+        (make-variable-buffer-local ',stored-mode)
         ;; The actual global minor-mode
         (define-minor-mode ,global-mode
  	 ,(format "Toggle %s in every buffer.
***************
*** 307,314 ****
--- 319,328 ----
  	 (if ,global-mode
  	     (progn
  	       (add-hook 'after-change-major-mode-hook ',buffers)
+ 	       (add-hook 'find-file-hook ',buffers-check)
  	       (add-hook 'change-major-mode-hook ',cmmh))
  	   (remove-hook 'after-change-major-mode-hook ',buffers)
+ 	   (remove-hook 'find-file-hook ',buffers-check)
  	   (remove-hook 'change-major-mode-hook ',cmmh))
  
  	 ;; Go through existing buffers.
***************
*** 325,341 ****
  
         ;; 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))))
  
  ;;;
--- 339,372 ----
  
         ;; The function that calls TURN-ON in each buffer.
         (defun ,buffers ()
! 	 (dolist (buf ,buffers)
! 	   (when (buffer-live-p buf)
! 	     (with-current-buffer buf
! 	       (unless ,mode
! 		 (,turn-on))))))
!        (put ',buffers 'definition-name ',global-mode)
! 
!        (defun ,buffers-check ()
! 	 (remove-hook 'post-command-hook ',buffers-check)
  	 (while ,buffers
  	   (let ((buf (pop ,buffers)))
  	     (when (buffer-live-p buf)
! 	       (with-current-buffer buf
! 		 (if (or (not ,stored-mode) (eq ,stored-mode major-mode))
! 		     (,turn-on)
! 		   (let ((old-stored-mode ,stored-mode))
! 		     (,mode -1)
! 		     (,turn-on)
! 		     (message
! 		      "%s was set up for wrong major mode: %s.
! We tried to correct.  This is a bug.  Please report it."
! 		      (symbol-name ',mode) old-stored-mode))
! 		   (sit-for 1)))))))
  
         ;; The function that catches kill-all-local-variables.
         (defun ,cmmh ()
  	 (add-to-list ',buffers (current-buffer))
! 	 (add-hook 'post-command-hook ',buffers-check))
         (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
============================================================

===File ~/font-lock-diff====================================
*** font-lock.el	22 May 2005 19:04:51 -0500	1.258
--- font-lock.el	26 May 2005 19:17:56 -0500	
***************
*** 1569,1583 ****
  	(t
  	 (car keywords))))
  
! (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)
      (let* ((defaults (or font-lock-defaults
--- 1569,1585 ----
  	(t
  	 (car keywords))))
  
! ;; The mode for which font-lock was initialized, or nil if none.
! (defvar font-lock-set-defaults)
! (defvaralias 'font-lock-set-defaults 'font-lock-mode-stored-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.
!   (unless (eq font-lock-set-defaults major-mode)
!     (setq font-lock-set-defaults major-mode)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
      (let* ((defaults (or font-lock-defaults
============================================================

===File ~/cwarn-diff========================================
*** cwarn.el	04 Apr 2005 07:03:09 -0500	1.9
--- cwarn.el	26 May 2005 19:19:59 -0500	
***************
*** 234,242 ****
--- 234,244 ----
      (back-to-indentation)
      (eq (char-after) ?#)))
  
+ (defvar cwarn-mode-stored-mode)
  (defun cwarn-font-lock-keywords (addp)
    "Install/Remove keywords into current buffer.
  If ADDP is non-nil, install else remove."
+   (when addp (setq cwarn-mode-stored-mode major-mode))
    (dolist (pair cwarn-font-lock-feature-keywords-alist)
      (let ((feature (car pair))
  	  (keywords (cdr pair)))
============================================================

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my earlier message:

   When testing this patch, do not rely on stuff like `C-h v
   font-lock-set-defaults', because this is currently buggy.

This is a consequence of a bug I reported in another thread,
`Bug in buffer-local-value'.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    ! If the minor mode's set-up depends on the major mode, the function that
    ! does the initial set-up should record the value of the variable `major-mode'
    ! in the variable MODE-stored-mode.

The reference "the function that does the initial set-up" is not concrete.
I am not completely sure which function you mean.  It would be better
to refer to it by name.

If this function is one that the user has to supply and define, then I
ask: why make the user arrange to set MODE-stored-mode?  Why not have
define-global-minor-mode automatically arrange to save the value in
that variable?  Then maybe this variable would be something internal
and it would not have to be mentioned in the doc string.

Looking at your patch, it looks like this mechanism already is
taken care of by code that is generated by define-global-minor-mode.
So why did you mention this variable in the doc string?
Just what is the user of define-global-minor-mode supposed to do?
I cannot understand it.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   The reference "the function that does the initial set-up" is not concrete.
   I am not completely sure which function you mean.  It would be better
   to refer to it by name.

In the latest version of the patch I am going to send soon, it just
says that enabling the mode in any way should set the variable.  The
author of the minor mode can decide on the details of how to do that.

   If this function is one that the user has to supply and define, then I
   ask: why make the user arrange to set MODE-stored-mode?  Why not have
   define-global-minor-mode automatically arrange to save the value in
   that variable?

Because we have to know in which major mode the minor mode was
enabled.  That could easily happen behind define-global-minor-mode's
back, by using the non-global variant.  In effect, we know of at least
two examples of major modes that do this, comint-mode and
interactive-sql-mode.  There probably are others.  They should not be
doing that, but we want to be able to catch modes that do this.
Also, if the user does something like
(add-hook 'foo-mode-hook 'turn-on-font-lock), and then tries out
global-font-lock-mode, trouble results for major modes derived in a
non-standard way from foo-mode. 

   Looking at your patch, it looks like this mechanism already is
   taken care of by code that is generated by define-global-minor-mode.

Only partially.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

      Why not have
       define-global-minor-mode automatically arrange to save the value in
       that variable?

    Because we have to know in which major mode the minor mode was
    enabled.  That could easily happen behind define-global-minor-mode's
    back, by using the non-global variant.

I think I understand that.  But...

      In effect, we know of at least
    two examples of major modes that do this, comint-mode and
    interactive-sql-mode.

This makes no sense to me.  I thought you were talking about minor
modes and what they need to do.  Now you have switched to major modes.
I cannot follow the change of subject.  Meanwhile, you have not
finished the explanation.

Anyway, how about if we change define-minor-mode so that it generates
the code to record which major mode enabled it?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   I think I understand that.  But...

	 In effect, we know of at least
       two examples of major modes that do this, comint-mode and
       interactive-sql-mode.

   This makes no sense to me.  I thought you were talking about minor
   modes and what they need to do.  Now you have switched to major modes.
   I cannot follow the change of subject.  Meanwhile, you have not
   finished the explanation.

To at least attempt to get things correctly with the current Font Lock
code, there are things the minor mode (Font Lock in this case) should
do, there are things that any major should do and there are things no
major mode should do.  Major modes should follow the conventions for
major modes rigorously.  But, in addition, they should not enable Font
Lock in any way, directly or indirectly (other than through the
define-global-minor-mode machinery), and they also should not call
font-lock-add-keywords directly or indirectly, even if this does not
enable Font Lock.  comint-mode and interactive-sql-mode both enable
Font Lock (the latter via sql-product-font-lock), creating problems
for modes derived from them.

   Anyway, how about if we change define-minor-mode so that it generates
   the code to record which major mode enabled it?

The very vast majority of minor modes have no reason whatsoever to do
this.

The problem is created by minor modes whose setup depends on the major
mode (I believe a small minority) and for whom dis- and re-enabling
the minor mode is not sufficient to correct a problem where the minor
mode is setup for the wrong mode.  The only known example (and
hopefully the only example, period) is Font Lock mode.

I will propose a new patch to easy-mmode that does _not_ use the new
variable MODE-stored-mode.  It will say that dis- and then re-enabling
the minor mode should correctly set up the minor mode for the current
major mode.  The only trouble with that is that this condition does
not apply to Font Lock.  Ideally one could make it apply.  Otherwise,
one could just use a variable internal to Font Lock.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    Major modes should follow the conventions for
    major modes rigorously.  But, in addition, they should not enable Font
    Lock in any way, directly or indirectly (other than through the
    define-global-minor-mode machinery), and they also should not call
    font-lock-add-keywords directly or indirectly, even if this does not
    enable Font Lock.

I just looked for all the calls to font-lock-add-keywords in Emacs.
In no case does any major mode call font-lock-add-keywords directly,
or even fairly directly.  But it seems plausible to me that mode hook
functions might call functions that call font-lock-add-keywords.
Would that be a problem?

      comint-mode and interactive-sql-mode both enable
    Font Lock (the latter via sql-product-font-lock), creating problems
    for modes derived from them.

I think it would be more consistent, nowadays, to turn that off.

       Anyway, how about if we change define-minor-mode so that it generates
       the code to record which major mode enabled it?

    The very vast majority of minor modes have no reason whatsoever to do
    this.

Does that matter?

If only a few of them need to do this, perhaps it is better to do
this automatically in all the minor modes (even though it is
unnecessary in most of them) than to make users do it manually
and risk bugs.

      The only known example (and
    hopefully the only example, period) is Font Lock mode.

If these complexities apply only to Font Lock mode,
perhaps we don't need to document them in the doc string
of define-global-minor-mode.

    I will propose a new patch to easy-mmode that does _not_ use the new
    variable MODE-stored-mode.  It will say that dis- and then re-enabling
    the minor mode should correctly set up the minor mode for the current
    major mode.

Ok, but...

		 The only trouble with that is that this condition does
    not apply to Font Lock.

if the problem exists only for Font Lock mode, and your proposed
solution does not solve it for Font Lock mode, what problem is
this solution solving?

    The problem is that if you call these functions with `nil' for MODE,
    this sets up Font Lock for the current major mode, which could be an
    ancestor mode (it calls font-lock-set-defaults).

I don't understand.  How can the current major mode "be" an ancestor
mode?  Do you mean that the font-lock settings came from an ancestor
mode?

      But if font-lock-add-keywords was called with `nil' for MODE,
    my patch will undo the adding of the keyword.

Suppose we make font-lock-add-keywords with nil for MODE
record its argument in some permanent buffer-local variable.
Then changing the major mode and reenabling font-lock
would use the same keywords previously added.

It's possible this would do something wrong in certain
other cases, but would it do the right thing in these cases?

    Note that even after this patch, there is one potentially troublesome
    case left.  It concerns the case of a major mode that gets enabled by
    a timer or process for a non-file visiting buffer.

Would you please explain what problem would happen in this case?
It is hard to understand your arguments about proposed solutions
when you haven't said what the problem is.

      (Even in this case
    it is only likely to occur for major modes that do not follow the new
    conventions.)

In that case, maybe it is a fine solution to tell people to fix their
major modes.  Why go to a lot of trouble about broken major modes?

    The trouble with doing that for Font Lock, given its current code, is
    what happens if after-change-major-mode never ran Font Lock, and Font
    Lock is nevertheless enabled.  Then it was set by something else.

What kind of something else could it be?

One possibility is that the major mode itself turned on font-lock
mode.  You've said that there's a problem when a major mode turns on
font-lock mode.

Aside from that, what other case could do that?

Anyway, please explain why you think it is difficult for turning font
lock off and on again to give the right results.  Can my proposed
change to font-lock-add-keywords work?  If not, why not?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   But it seems plausible to me that mode hook
   functions might call functions that call font-lock-add-keywords.
   Would that be a problem?

I am not the Font Lock maintainer and I do not know every aspect of
Font Lock.  But, in as far as I understand things, if they pass a
non-nil MODE argument everything is OK in as far as the problems we
are discussing.  Major modes, hook functions, or anything else can call
font-lock-add-keywords with a non-nil MODE argument without creating
any problems of the type we are discussing.  But if anything, mode
hooks, whatever, directly or indirectly calls font-lock-add-keywords
with a nil MODE argument at any time during a major mode function,
trouble could potentially result.

   Suppose we make font-lock-add-keywords with nil for MODE
   record its argument in some permanent buffer-local variable.
   Then changing the major mode and reenabling font-lock
   would use the same keywords previously added.

That would also change the current behavior when changing a major mode
interactively.  Currently, it gets rid of these keywords.  When
changing modes interactively, the situation is thousands of times more
likely to occur than when changing major modes inside a derived mode.
Stefan does not seem to consider getting rid of these keywords to be
a problem when changing modes interactively, although he does consider
it a problem when it happens in a derived mode.  Maybe there should be
such a variable but it should not be permanent local.  When modes
change inside a derived mode, kill-all-local-variables is not called.
It is only called when the very first mode function is called.

An alternative would be to get rid of the mysterious "subtle problems
due to the details of the implementation" mentioned in the following
quote from `(elisp)Search-based Fontification':

       *Warning:* Only use a non-`nil' MODE argument when you use
    `font-lock-add-keywords' or `font-lock-remove-keywords' in your
    `.emacs' file.  When you use these functions from a Lisp program (such
    as a minor mode), we recommend that you use `nil' for MODE (and place
    the call on a hook) to avoid subtle problems due to the details of the
    implementation.

Then we could say instead that a non-nil MODE argument should only be
used in such a way that it could never be directly or indirectly
called from a major mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

>    Suppose we make font-lock-add-keywords with nil for MODE
>    record its argument in some permanent buffer-local variable.
>    Then changing the major mode and reenabling font-lock
>    would use the same keywords previously added.

Problem is that some of those keywords should be permanent and others
shouldn't.  E.g. it depends whether they correspond to a minor mode
which is permanent or not.

> That would also change the current behavior when changing a major mode
> interactively.  Currently, it gets rid of these keywords.  When
> changing modes interactively, the situation is thousands of times more
> likely to occur than when changing major modes inside a derived mode.
> Stefan does not seem to consider getting rid of these keywords to be
> a problem when changing modes interactively, although he does consider
> it a problem when it happens in a derived mode.  Maybe there should be

The assumption is that those keywords are added via a function placed on the
major mode's hook, i.e. the keywords tweak the major mode and should thus be
removed/readded when the major mode changes.
Of course in other settings it should behave differently.

The problem is that the API is too simplistic.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   The assumption is that those keywords are added via a function placed on the
   major mode's hook, i.e. the keywords tweak the major mode and should thus be
   removed/readded when the major mode changes.
   Of course in other settings it should behave differently.

One solution is making font-lock-set-defaults itself check whether the
major mode is correct (as one of my earlier patches already did, but I
maybe mistakenly changed it later on).  That way, Font Lock gets
updated for the current major mode, _before_ any keywords get added.
So, any keywords added while hooks are being run at the end of the
actual final major mode, or during the body of the final major mode,
are not going to be lost.  Keywords added passing nil for MODE _while
an ancestor mode is running_ are lost.  It is not clear at all to me
whether or not they are supposed to be lost.  The documentation is
super-vague and cryptic.  (The other solution I proposed earlier would
have similar effects, but would be somewhat more complex.)

Concrete problem:

Suppose child-mode is derived from parent-mode.  A program adds a call to
font-lock-add-keywords passing nil for MODE to parent-mode-hook.
Do you want those keywords to apply to child-mode or not?

With the patches below, they would apply to child-mode if parent-mode
followed the major mode conventions (but not otherwise).  Currently it
seems to be nowhere documented whether or not these keywords should
apply to child-mode, so that currently the effects of using
font-lock-add-keywords passing nil for MODE seem to be undefined in as
far as derived modes are concerned.

===File ~/font-core-diff====================================
*** font-core.el	22 May 2005 16:46:07 -0500	1.28
--- font-core.el	31 May 2005 20:32:19 -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-stored-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-stored-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-stored-mode
! 		 (not (eq font-lock-mode-stored-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	31 May 2005 20:47:50 -0500	
***************
*** 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.
--- 704,711 ----
  	 (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.
***************
*** 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)
--- 1572,1585 ----
  
  (defvar font-lock-set-defaults nil)	; Whether we have set up defaults.
  
+ (defvar font-lock-mode-stored-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-stored-mode major-mode))
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
============================================================

Re: org-mode and mode hooks.

[Richard Stallman]

    >    Suppose we make font-lock-add-keywords with nil for MODE
    >    record its argument in some permanent buffer-local variable.
    >    Then changing the major mode and reenabling font-lock
    >    would use the same keywords previously added.

    Problem is that some of those keywords should be permanent and others
    shouldn't.  E.g. it depends whether they correspond to a minor mode
    which is permanent or not.

Maybe we should label each of these things by the name of the minor
mode.  That way, we could tell which of them to discard.  Or we could
give each one a flag saying whether to preserve it when changing the
major mode.

Would that solve the problem?

Re: org-mode and mode hooks.

[Luc Teirlinck]

I still do not understand exactly what a nil argument for MODE in
font-lock-{add,remove}-keywords is supposed to accomplish.  The
font-lock-add-keywords docstring contains the following mysterious
sentence:

  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.

Which subtle problems?

It seems to me that a _global_ minor mode _should_ use a non-nil MODE
argument when adding or removing a keyword.  Do the "subtle problems"
refer to problems that occur when trying to make
font-lock-keywords-alist buffer local, as a buffer local minor mode
might be tempted to do?  A permanent buffer-local minor mode that
wants to add the keyword independent of the mode could use
after-change-major-mode-hook to add it.  With my latest set of patches
that will work for modes that run that hook.  If the minor mode wants
to add it for a particular mode and all its derived modes, it can,
after my patches, add it to the mode hook, as long as the mode and its
ancestor modes follow the new major mode conventions.  Only adding it
to a mode, but not to its derived modes, is problematic after my
patches, as well as without them.

Maybe one could have a font-lock-local-keywords-alist, that would be
_in addition_ to font-lock-keywords-alist.  That should be a
non-permanent local and could be used by non-permanent buffer local minor
modes.  If one is not willing to rely on after-change-major-mode-hook,
there could be a separate font-lock-permanent-local-keywords-alist,
for permanent-local minor modes.

All of this assuming that I am guessing the purpose of a nil argument
correctly.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

> I still do not understand exactly what a nil argument for MODE in
> font-lock-{add,remove}-keywords is supposed to accomplish.

It adds the keywords for the current buffer only.

>   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.
> Which subtle problems?

Can't remember.  IIRC there were problems where adding keywords to
comint-mode didn't add them to all derivatives.  And the implementation of
the non-nil case is sufficiently horrendous to make you confident that there
will be other subtle problems.

> It seems to me that a _global_ minor mode _should_ use a non-nil MODE
> argument when adding or removing a keyword.

I'm not sure what you mean by a "global minor mode" (do you mean
define-minor-mode :global, or define-global-minor-mode).  But in any case
your statement doesn't make much sense to me: if the minor mode is global,
to *which* MODE should it add keywords? 

> Do the "subtle problems" refer to problems that occur when trying to make
> font-lock-keywords-alist buffer local, as a buffer local minor mode might
> be tempted to do?

No, I've never seen anyone try to make font-lock-keywords-alist buffer-local
(thank god).


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

   Can't remember.  IIRC there were problems where adding keywords to
   comint-mode didn't add them to all derivatives.

So you actually _want_ them added to derivatives.  That was not at all
obvious.  The actual patches I sent will add them to derivatives as
long as the mode conventions are followed.

But it you pass comint-mode as argument _in your .emacs_ (as
recommended in the docs), how will it enable them for derivatives?  I
have not read through every line of font-{lock,core}.el, so I may have
missed something, but `font-lock-set-defaults' does

    (cdr (assq major-mode font-lock-keywords-alist))

So if it contains (comint-mode . ...), it would appear to have no
effect in shell-mode.

So what is the difference between passing a non-nil MODE in your
.emacs or from a Lisp program?

In effect this is what the docstring of font-lock-keywords-alist says:

    Each element has the form (MODE KEYWORDS . APPEND).
    `font-lock-set-defaults' adds the elements in the list KEYWORDS to
    `font-lock-keywords' when Font Lock is turned on in major mode MODE.

It does not say "or in a derivative mode". 

So, instead of the current cryptic difference between .emacs and
programs, maybe the docstring should say to pass a non-nil MODE
argument if you do not want it to apply to derivatives and a nil one
if you do want it to apply to derivatives.  I believe that after such
doc fixes (which I could make) to the font-lock-{add,remove}-keyword
and the Elisp manual, my patches would do exactly what the revised docs
would say.

What happens to keywords in an _interactive_ mode change, that is
during kill-all-local-variables is a separate problem, not related to
`define-global-minor-mode'.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

>    Can't remember.  IIRC there were problems where adding keywords to
>    comint-mode didn't add them to all derivatives.
> So you actually _want_ them added to derivatives.

Typically a derivative is a minor tweak and by default it should inherit
*all* characteristics of the parent mode.  This is not specific to font-lock.

> But it you pass comint-mode as argument _in your .emacs_ (as
> recommended in the docs), how will it enable them for derivatives?

Who said it will?  BTW, why did you underscore the "in your .emacs"?
Are you implying that things might be different elsewhere?

> I have not read through every line of font-{lock,core}.el, so I may have
> missed something, but `font-lock-set-defaults' does

>     (cdr (assq major-mode font-lock-keywords-alist))

> So if it contains (comint-mode . ...), it would appear to have no
> effect in shell-mode.

And that's the problem.

> So what is the difference between passing a non-nil MODE in your
> .emacs or from a Lisp program?

I have no clue what your talking about w.r.t ".emacs vs Lisp program".

By placing the font-lock-add-keywords with a nil arg in the mode's hook, you
ensure that the font-lock-add-keywords is also run for derivatives.

> It does not say "or in a derivative mode". 

Of course not: it's a property of major-mode-derivation, not of
font-lock keywords.

> So, instead of the current cryptic difference between .emacs and
> programs, maybe the docstring should say to pass a non-nil MODE
> argument if you do not want it to apply to derivatives and a nil one
> if you do want it to apply to derivatives.

That's pretty ugly, don't you think?  It's like saying "use this form to
benefit from quirk A or that form if you prefer quirk B".


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   Who said it will?  BTW, why did you underscore the "in your .emacs"?
   Are you implying that things might be different elsewhere?

I am not implying that.  I am claiming that it makes no difference
whatsoever.  But several docstrings, the Emacs manual and the Elisp
manual all imply that there is such a difference.

   I have no clue what your talking about w.r.t ".emacs vs Lisp program".

Because the docs are talking about such a difference all over the place.

>From `(elisp)Search-based Fontification':

       *Warning:* Only use a non-`nil' MODE argument when you use
    `font-lock-add-keywords' or `font-lock-remove-keywords' in your
    `.emacs' file.  When you use these functions from a Lisp program (such
    as a minor mode), we recommend that you use `nil' for MODE (and place
    the call on a hook) to avoid subtle problems due to the details of the
    implementation.

Why should the keywords not apply to derived modes when set from
.emacs, while they have to apply when set from code?  Similar
recommendations are repeated, with different wording, all over the place.

   That's pretty ugly, don't you think?  It's like saying "use this form to
   benefit from quirk A or that form if you prefer quirk B".

If you give people a choice between two alternatives, you _have_ to
tell them what the difference between the two alternatives is.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

>        *Warning:* Only use a non-`nil' MODE argument when you use
>     `font-lock-add-keywords' or `font-lock-remove-keywords' in your
>     `.emacs' file.  When you use these functions from a Lisp program (such
>     as a minor mode), we recommend that you use `nil' for MODE (and place
>     the call on a hook) to avoid subtle problems due to the details of the
>     implementation.

> Why should the keywords not apply to derived modes when set from
> .emacs, while they have to apply when set from code?  Similar
> recommendations are repeated, with different wording, all over the place.

Oh, I see.  Sorry for being so dense.  Basically, all it says is that
a non-nil MODE argument should be avoided, but it is phrased in such a way
that users shouldn't feel too scared if they use it in their .emacs where it
can be convenient if the user is intimidated by add-hook (and where the
possible problems won't be too serious since they only affect that one
user).

>    That's pretty ugly, don't you think?  It's like saying "use this form to
>    benefit from quirk A or that form if you prefer quirk B".

> If you give people a choice between two alternatives, you _have_ to
> tell them what the difference between the two alternatives is.

I tell them the difference: one is recommended the other isn't.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

My previous patch called add-hook in the wrong way.

Corrected patch:

===File ~/font-lock-b-diff==================================
*** font-lock.el	29 May 2005 09:15:38 -0500	1.259
--- font-lock.el	01 Jun 2005 18:39:50 -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-stored-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-stored-mode major-mode))
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
============================================================

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

  My previous patch called add-hook in the wrong way.

I meant the call in the docstring example.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[David Kastrup]

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

>>        *Warning:* Only use a non-`nil' MODE argument when you use
>>     `font-lock-add-keywords' or `font-lock-remove-keywords' in your
>>     `.emacs' file.  When you use these functions from a Lisp
>>     program (such as a minor mode), we recommend that you use `nil'
>>     for MODE (and place the call on a hook) to avoid subtle
>>     problems due to the details of the implementation.
>
>> Why should the keywords not apply to derived modes when set from
>> .emacs, while they have to apply when set from code?  Similar
>> recommendations are repeated, with different wording, all over the
>> place.
>
> Oh, I see.  Sorry for being so dense.  Basically, all it says is
> that a non-nil MODE argument should be avoided, but it is phrased in
> such a way that users shouldn't feel too scared if they use it in
> their .emacs where it can be convenient if the user is intimidated
> by add-hook (and where the possible problems won't be too serious
> since they only affect that one user).

Har har har.

80% of all users use some .emacs that has been hotch-potched together
from various sources.  Their sources are somewhat more likely to have
peeked into the manual.  So if the manual does not recommend
techniques that will break when throwing together initializiation
files, there will be less chaos in need of resolution by Emacs hackers
or sysadmins.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: org-mode and mode hooks.

[Luc Teirlinck]

And my second patch contained stray punctuation.  This should finally
be correct:

===File ~/font-lock-b-diff==================================
*** font-lock.el	29 May 2005 09:15:38 -0500	1.259
--- font-lock.el	01 Jun 2005 18:59:56 -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-stored-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-stored-mode major-mode))
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
============================================================

Re: org-mode and mode hooks.

[Luc Teirlinck]

The only difference between the following patch to font-lock.el and
the previous one are two doc fixes.  After those doc fixes, everything
should work as documented when my three patches are applied.  I still
have to look at Richard's comments about my easy-mmode patch.

===File ~/font-lock-b-diff==================================
*** font-lock.el	29 May 2005 09:15:38 -0500	1.259
--- font-lock.el	01 Jun 2005 18:30:07 -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,703 ----
  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
!   (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.
--- 716,723 ----
  	 (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)
--- 787,797 ----
  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)
--- 1586,1599 ----
  
  (defvar font-lock-set-defaults nil)	; Whether we have set up defaults.
  
+ (defvar font-lock-mode-stored-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-stored-mode major-mode))
      (set (make-local-variable 'font-lock-set-defaults) t)
      (make-local-variable 'font-lock-fontified)
      (make-local-variable 'font-lock-multiline)
============================================================

Re: org-mode and mode hooks.

[Richard Stallman]

    The only difference between the following patch to font-lock.el and
    the previous one are two doc fixes.  After those doc fixes, everything
    should work as documented when my three patches are applied.

In that case, my previous comments about the previous patch
still apply to this one.


I received many long messages yesterday about this issue.
What conclusions were reached from them?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

       The only difference between the following patch to font-lock.el and
       the previous one are two doc fixes.  After those doc fixes, everything
       should work as documented when my three patches are applied.

   In that case, my previous comments about the previous patch
   still apply to this one.

There were many comments.  I tried to address the concrete ones to
easy-mmode in my latest patch for that file.  I explained why
define-derived-mode can not reliably record the major mode for which a
minor mode was enabled; if it is not supposed to be 100% reliable,
then define-global-major-mode can do just as good a job more easily.
My latest patches to define-global-major-mode do that job.

Other than that, the major remaining issue is that calling
font-lock-{add,remove}-keywords with nil for MODE only works reliably
for major modes that follow the new conventions.  But one of your
previous comments was:

	 (Even in this case
       it is only likely to occur for major modes that do not follow the new
       conventions.)

   In that case, maybe it is a fine solution to tell people to fix their
   major modes.  Why go to a lot of trouble about broken major modes?

In addition to that, there would be a problem with major modes that do
follow the conventions but call font-lock-{add,remove}-keywords with
nil for MODE in their body instead of their hook.  Major modes are not
supposed to do that.  A nil argument was not designed to be used that
way and you said:

   I just looked for all the calls to font-lock-add-keywords in Emacs.
   In no case does any major mode call font-lock-add-keywords directly,
   or even fairly directly.  But it seems plausible to me that mode hook
   functions might call functions that call font-lock-add-keywords.
   Would that be a problem?

To answer the latter question: only for major modes that do not follow
the new conventions.  Even for them, there is no problem if the calls
were added by the user, because the old docs tell to pass a non-nil
argument for MODE and the problems only occur for a nil argument.  The
new docs say that it is OK for the user to pass a nil argument as long
as he realizes that it may not work reliably for non-conforming modes.

Problems remain with for minor modes that call
font-lock-{add,remove}-keywords with nil for MODE and add it to the
hook of a major mode that does not follow the conventions or has an
ancestor that does not.  Do you consider this to be a problem that
needs to be fixed before the next release?  Fixing it would require
non-trivial changes to Font Lock.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

   I received many long messages yesterday about this issue.
   What conclusions were reached from them?

If you do not consider the remaining problems, described in my
previous message, to be a real problem, then I believe that there are
not many conclusions left to be reached.

I personally believe that instead of the current problematic
implementation of a nil argument for MODE, it would have been a lot
better to implement inheritance of keywords from other major modes by
letting major modes specify a list of major modes from which they want
to inherit.  That _would_ solve all remaining problems.  It could also
be used for major modes that are only used as substitutes for
recursive edits, which are usually _not_ derived modes.  Why should,
for instance, switching to an editable version of a normally read-only
mode get rid of all fontification?  However, implementing this would
require more extensive changes to Font Lock than my patches.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

       I received many long messages yesterday about this issue.
       What conclusions were reached from them?

    If you do not consider the remaining problems, described in my
    previous message, to be a real problem, then I believe that there are
    not many conclusions left to be reached.

I am not sure what problems those are.  There were so many messages
yesterday that just reading them would take a long time.  I hoped to
bypass that job by asking you to summarize the result of all that
discussion.

Re: org-mode and mode hooks.

[Luc Teirlinck]

   I am not sure what problems those are.  There were so many messages
   yesterday that just reading them would take a long time.  I hoped to
   bypass that job by asking you to summarize the result of all that
   discussion.

I thought I did that in the message prior to the one you responded to.
>From that message:

Other than that, the major remaining issue is that calling
font-lock-{add,remove}-keywords with nil for MODE only works reliably
for major modes that follow the new conventions.  But one of your
previous comments was:

         (Even in this case
       it is only likely to occur for major modes that do not follow the new
       conventions.)

   In that case, maybe it is a fine solution to tell people to fix their
   major modes.  Why go to a lot of trouble about broken major modes?

In addition to that, there would be a problem with major modes that do
follow the conventions but call font-lock-{add,remove}-keywords with
nil for MODE in their body instead of their hook.  Major modes are not
supposed to do that.  A nil argument was not designed to be used that
way and you said:

   I just looked for all the calls to font-lock-add-keywords in Emacs.
   In no case does any major mode call font-lock-add-keywords directly,
   or even fairly directly.

Re: org-mode and mode hooks.

[Stefan Monnier]

> Other than that, the major remaining issue is that calling
> font-lock-{add,remove}-keywords with nil for MODE only works reliably
> for major modes that follow the new conventions.  But one of your

Actually, don't worry too mucvh about that, because I think that even with
the current code things don't work right unless the major mode follows
the conventions w.r.t major-mode-derivation.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   Actually, don't worry too mucvh about that, because I think that even with
   the current code things don't work right unless the major mode follows
   the conventions w.r.t major-mode-derivation.

My changes do not introduce new problems, they fix problems with the
current code.  The problem is that the fix is not perfect.  It limits
the situations in which problems occur and makes the consequences less
severe if they occur.

With the current code problems are _much_ more likely to happen than
with my proposed changes.  For instance with the current code,
enabling Font Lock mode in a major mode body, like for instance
sql-interactive-mode does, will cause problems for derived modes, no
matter how well the major mode conventions are followed.  The fact
that sql-interactive-mode later disables Font Lock again does not
correct this.  Also the consequences are much more severe with the
current code.  With the current code, Font Lock may get set up for the
wrong major mode, giving completely incorrect fontification.  After my
changes, there is the slight probability of loosing a few keywords for
major modes that do not follow the conventions.

I believe that it is possible to implement things completely reliably
by letting major modes specify a list of major modes from which they
want to inherit keywords and using a non-nil MODE argument.  But that
would require more extensive changes to Font Lock and would also
require some (much less extensive) changes to define-derived-mode.  I
believe that a nil argument for MODE has been around for a while so
for backward compatibility, one would still need to reasonably take
care of such calls.  The question is whether the remaining problems
are worth the trouble.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> I believe that it is possible to implement things completely
> reliably by letting major modes specify a list of major modes from
> which they want to inherit keywords and using a non-nil MODE
> argument.  But that would require more extensive changes to Font
> Lock and would also require some (much less extensive) changes to
> define-derived-mode.  I believe that a nil argument for MODE has
> been around for a while so for backward compatibility, one would
> still need to reasonably take care of such calls.  The question is
> whether the remaining problems are worth the trouble.

>From the description, I'd say right now probably not.  It seems like
we should try with your current fixes for the release.  If significant
problems get reported after the release, we might still reconsider
more invasive changes as long as we have enough time for testing them.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: org-mode and mode hooks.

[Richard Stallman]

    In addition to that, there would be a problem with major modes that do
    follow the conventions but call font-lock-{add,remove}-keywords with
    nil for MODE in their body instead of their hook.  Major modes are not
    supposed to do that.  A nil argument was not designed to be used that
    way and you said:

       I just looked for all the calls to font-lock-add-keywords in Emacs.
       In no case does any major mode call font-lock-add-keywords directly,
       or even fairly directly.

No major mode calls font-lock-add-keywords directly, or even fairly
directly.  However, I have not done the studying necessary to
determine whether any of those calls could ever possibly be reached
from a major mode function.  And certainly, if a minor mode uses
font-lock-add-keywords, and then people use define-global-minor-mode
to make a global version of it, then font-lock-add-keywords will be
called from major modes through after-change-major-mode-hook.

Are you saying that kind of use will cause no problem?

If so, could you be more precise about which kinds of uses of
font-lock-add-keywords from a major mode could cause a problem?

Re: org-mode and mode hooks.

[Luc Teirlinck]

RichardStallman wrote:

   And certainly, if a minor mode uses
   font-lock-add-keywords, and then people use define-global-minor-mode
   to make a global version of it, then font-lock-add-keywords will be
   called from major modes through after-change-major-mode-hook.

   Are you saying that kind of use will cause no problem?

Not if the involved major modes follow the conventions.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   I am not sure what problems those are.

Maybe I explained just when they occurred, but not what they are.

If you call font-lock-{add,remove}-keywords with nil for MODE inside a
parent mode's body instead of its mode hook, the keyword is not
enabled in the derived mode.  If the parent mode does call
font-lock-{add,remove}-keywords in its mode hook, the keyword may not be
enabled in the derived mode if any involved mode does not follow the
new conventions.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    If you call font-lock-{add,remove}-keywords with nil for MODE inside a
    parent mode's body instead of its mode hook, the keyword is not
    enabled in the derived mode.  If the parent mode does call
    font-lock-{add,remove}-keywords in its mode hook, the keyword may not be
    enabled in the derived mode if any involved mode does not follow the
    new conventions.

Thanks.

I think the right solution is simply "don't do that, then".
Major mode functions should not call font-lock-add-keywords
or font-lock-remove-keywords (though the mode hooks may do so).

I will add this to the documentation of font-lock-add-keywords.

@strong{Warning:} major mode functions must not call
@code{font-lock-add-keywords} under any circumstances, either directly
or indirectly.  (This would lead to incorrect behavior for some minor
modes.)  They should set up for Font Lock mode by setting
@code{font-lock-keywords}.


    Problems remain with for minor modes that call
    font-lock-{add,remove}-keywords with nil for MODE and add it to the
    hook of a major mode that does not follow the conventions or has an
    ancestor that does not.  Do you consider this to be a problem that
    needs to be fixed before the next release?  Fixing it would require
    non-trivial changes to Font Lock.

We don't need to fix this.  Those modes should be fixed.


So, please install your patch.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   I will add this to the documentation of font-lock-add-keywords.

   @strong{Warning:} major mode functions must not call
   @code{font-lock-add-keywords} under any circumstances, either directly
   or indirectly.  (This would lead to incorrect behavior for some minor
   modes.)  They should set up for Font Lock mode by setting
   @code{font-lock-keywords}.

Well, it depends on _how_ they call it indirectly.  For instance, if
the indirect call adds it to a mode hook everything is fine.  I
believe that it is better to apply the patch below to modes.texi.

(I could not reply earlier due to connection problems.)

===File ~/modes.texi-diff===================================
*** modes.texi	29 May 2005 10:42:38 -0500	1.112
--- modes.texi	06 Jun 2005 18:06:28 -0500	
***************
*** 2626,2632 ****
  
  You can use @code{font-lock-add-keywords} to add additional
  search-based fontification rules to a major mode, and
! @code{font-lock-remove-keywords} to removes rules.
  
  @defun font-lock-add-keywords mode keywords &optional append
  This function adds highlighting @var{keywords} for @var{mode}.  The
--- 2626,2634 ----
  
  You can use @code{font-lock-add-keywords} to add additional
  search-based fontification rules to a major mode, and
! @code{font-lock-remove-keywords} to remove rules.  These two functions
! are meant for user customization and for use in minor mode functions.
! Major mode functions should set @code{font-lock-keywords} instead.
  
  @defun font-lock-add-keywords mode keywords &optional append
  This function adds highlighting @var{keywords} for @var{mode}.  The
***************
*** 2657,2662 ****
--- 2659,2685 ----
  @samp{FIXME}, even in comments, and another to fontify the words
  @samp{and}, @samp{or} and @samp{not} as keywords.
  
+ The above example does @emph{not} add the fontification patterns to
+ major modes derived from C mode, as is usually desired.  To add them
+ to derived modes too, pass @code{nil} for mode and add the call to
+ @var{mode}'s mode hook.
+ 
+ For example:
+ 
+ @smallexample
+ (add-hook 'c-mode-hook
+  (lambda ()
+   (font-lock-add-keywords nil
+    '(("\\<\\(FIXME\\):" 1 font-lock-warning-face prepend)
+      ("\\<\\(and\\|or\\|not\\)\\>" .
+       font-lock-keyword-face)))))
+ @end smallexample
+ 
+ The above procedure may fail to add keywords to a derived mode if
+ @var{mode} or the derived mode do not yet follow the standard
+ conventions.  File a bug report if this happens, so that the offending
+ major mode can be corrected.
+ 
  Some modes have specialized support for additional patterns.  See the
  variables @code{c-font-lock-extra-types},
  @code{c++-font-lock-extra-types}, @code{objc-font-lock-extra-types}
***************
*** 2668,2682 ****
  in @code{font-lock-add-keywords}, @var{mode} should be a major mode
  command name or @code{nil}.  If @code{nil}, the highlighting
  @var{keywords} are immediately removed in the current buffer.
- @end defun
  
! @strong{Warning:} Only use a non-@code{nil} @var{mode} argument when
! you use @code{font-lock-add-keywords} or
! @code{font-lock-remove-keywords} in your @file{.emacs} file.  When you
! use these functions from a Lisp program (such as a minor mode), we
! recommend that you use @code{nil} for @var{mode} (and place the call
! on a hook) to avoid subtle problems due to the details of the
! implementation.
  
  @node Other Font Lock Variables
  @subsection Other Font Lock Variables
--- 2691,2704 ----
  in @code{font-lock-add-keywords}, @var{mode} should be a major mode
  command name or @code{nil}.  If @code{nil}, the highlighting
  @var{keywords} are immediately removed in the current buffer.
  
! This function does @emph{not} remove the highlighting @var{keywords}
! from derived modes, as is usually desired.  To remove them from
! derived modes too, pass @code{nil} for mode and add the call to
! @var{mode}'s hook.  This may fail if @var{mode} or a derived mode do
! not yet follow the standard conventions.  File a bug report if this
! happens, so that the offending major mode can be corrected.
! @end defun
  
  @node Other Font Lock Variables
  @subsection Other Font Lock Variables
============================================================

Re: org-mode and mode hooks.

[Stefan Monnier]

> ! @strong{Warning:} Only use a non-@code{nil} @var{mode} argument when
> ! you use @code{font-lock-add-keywords} or
> ! @code{font-lock-remove-keywords} in your @file{.emacs} file.  When you
> ! use these functions from a Lisp program (such as a minor mode), we
> ! recommend that you use @code{nil} for @var{mode} (and place the call
> ! on a hook) to avoid subtle problems due to the details of the
> ! implementation.
  
The intention with the above text was to make a non-nil argument
semi-obsolete, so that misfeatures related to them don't need to be fixed.
I for one won't touch this code and will fight against "fixes" that pollute
the rest of the font-lock code,


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   > ! @strong{Warning:} Only use a non-@code{nil} @var{mode} argument when
   > ! you use @code{font-lock-add-keywords} or
   > ! @code{font-lock-remove-keywords} in your @file{.emacs} file.  When you
   > ! use these functions from a Lisp program (such as a minor mode), we
   > ! recommend that you use @code{nil} for @var{mode} (and place the call
   > ! on a hook) to avoid subtle problems due to the details of the
   > ! implementation.

   The intention with the above text was to make a non-nil argument
   semi-obsolete,

That may be what the above text _intends_ to say, but it definitely is
not what it says.  It even says to always use a non-nil argument when
used from .emacs, probably the most important use.  The text is very
vague, for instance, it says to add them to "a hook" whereas there are
only two hooks for which it will work properly (for compliant major
modes), the major mode hook and after-change-major-mode-hook (if you
want to get a "permanent local" type behavior).

The misleading text in question made me loose a lot of time.  Getting
rid of it will prevent other people from loosing a lot of time.

In my replacement text, I document the actual situation: a non-nil
argument does not apply to derived modes, a nil one must be added to a
major mode hook and if used this way, it will apply to derived modes
_if_ the major modes in question follow the conventions.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

>> ! @strong{Warning:} Only use a non-@code{nil} @var{mode} argument when
>> ! you use @code{font-lock-add-keywords} or
>> ! @code{font-lock-remove-keywords} in your @file{.emacs} file.  When you
>> ! use these functions from a Lisp program (such as a minor mode), we
>> ! recommend that you use @code{nil} for @var{mode} (and place the call
>> ! on a hook) to avoid subtle problems due to the details of the
>> ! implementation.

>    The intention with the above text was to make a non-nil argument
>    semi-obsolete,

> That may be what the above text _intends_ to say, but it definitely is
> not what it says.

Could be, but your change does not try to fix it, instead it documents the
part of the misbehavior we happen to know about, thus legitimizing the use,
rather than discouraging i.t

> It even says to always use a non-nil argument when used from .emacs,
> probably the most important use.

There's admittedly an ambiguity when it says:

   Only use a non-@code{nil} @var{mode} argument when you use
   @code{font-lock-add-keywords} or @code{font-lock-remove-keywords} in your
   @file{.emacs} file.

you understand it to mean

   When you use @code{font-lock-add-keywords} or
   @code{font-lock-remove-keywords} in your @file{.emacs} file, only use
   a non-@code{nil} @var{mode} argument.

whereas I meant it to mean:

   Use a non-@code{nil} @var{mode} argument only when you use
   @code{font-lock-add-keywords} or @code{font-lock-remove-keywords} in your
   @file{.emacs} file.

If you could fix the wording to make it more clear, I'd be grateful.
OTOH I wouldn't be grateful if you completely removed this warning like
you're suggesting to do.

> The text is very vague, for instance, it says to add them to "a hook"
> whereas there are only two hooks for which it will work properly (for
> compliant major modes), the major mode hook and
> after-change-major-mode-hook (if you want to get a "permanent local" type
> behavior).

Then please make it more precise.  E.g. by saying "mode hook".

> The misleading text in question made me lose a lot of time.

In what way, specifically?  Which hook did you try?

> In my replacement text, I document the actual situation: a non-nil
> argument does not apply to derived modes, a nil one must be added to a
> major mode hook and if used this way, it will apply to derived modes
> _if_ the major modes in question follow the conventions.

As mention, I'd like to semi-obsolete it, so I'd rather not document
it further: use at your own risk.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   Could be, but your change does not try to fix it, instead it documents the
   part of the misbehavior we happen to know about, thus legitimizing the use,
   rather than discouraging i.t

But your previous text, even when read as intended:

   whereas I meant it to mean:

      Use a non-@code{nil} @var{mode} argument only when you use
      @code{font-lock-add-keywords} or @code{font-lock-remove-keywords} in your
      @file{.emacs} file.

legitimizes the use, since it says that it is OK to use it in your
.emacs, and that is probably the main use.  It does not tell what
difference it makes if you use a nil or non-nil argument from your
.emacs.

   > The misleading text in question made me lose a lot of time.

   In what way, specifically?  Which hook did you try?

It was completely impossible to figure out what a nil MODE arg was
_trying_ to do.  You could not look at the actual behavior, because
before my patches the behavior with a nil MODE argument made no sense.
If Font Lock was for some reason enabled for the wrong mode, it was
impossible to correct reliably.  The docs clearly seemed to suggest
that a nil argument tried to enable the keywords for MODE only and not
for derived modes, like a non-nil argument does, only without the
mysterious "subtle implementation problems".

   As mention, I'd like to semi-obsolete it, so I'd rather not document
   it further: use at your own risk.

As long as it is mentioned for possible use in .emacs, it is not
semi-obsolete.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

>    Could be, but your change does not try to fix it, instead it documents the
>    part of the misbehavior we happen to know about, thus legitimizing the use,
>    rather than discouraging it.

> But your previous text, even when read as intended:

>    whereas I meant it to mean:

>       Use a non-@code{nil} @var{mode} argument only when you use
>       @code{font-lock-add-keywords} or @code{font-lock-remove-keywords} in your
>       @file{.emacs} file.

> legitimizes the use, since it says that it is OK to use it in your
> .emacs, and that is probably the main use.  It does not tell what
> difference it makes if you use a nil or non-nil argument from your
> .emacs.

>> The misleading text in question made me lose a lot of time.

>    In what way, specifically?  Which hook did you try?

> It was completely impossible to figure out what a nil MODE arg was
> _trying_ to do.

Don't know about the TeXinfo doc, but the docstring is pretty clear:

   MODE should be a symbol, the major mode command name, such as `c-mode'
   or nil.  If nil, highlighting keywords are added for the current buffer.

> You could not look at the actual behavior, because
> before my patches the behavior with a nil MODE argument made no sense.

Huh?  I've used it for many years with a nil argument and it worked
just fine.  You know very well that the problem you fixed only occurred in
some particular cases.

> If Font Lock was for some reason enabled for the wrong mode, it was
> impossible to correct reliably.

That unrelated to the TeXinfo doc.  You're here arguing for your patch,
which is a waste of time, since it's installed and nobody objected to it.

> The docs clearly seemed to suggest that a nil argument tried to enable the
> keywords for MODE only and not for derived modes,

Then the docs obviously need to be fixed, since there is no nil MODE.

>    As mention, I'd like to semi-obsolete it, so I'd rather not document
>    it further: use at your own risk.
> As long as it is mentioned for possible use in .emacs, it is not
> semi-obsolete.

No: as long as it's mentioned, it's not *obsolete*.  That's why I say
semi-obsolete.


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   Don't know about the TeXinfo doc, but the docstring is pretty clear:

      MODE should be a symbol, the major mode command name, such as `c-mode'
      or nil.  If nil, highlighting keywords are added for the current buffer.

What was not clear was what it was supposed to achieve, whether
it tried to add keywords for one single mode, or for a mode and the
modes derived from it, or be completely independent of any major mode.
Anyway, the docs have been fixed since.

   > The docs clearly seemed to suggest that a nil argument tried to enable the
   > keywords for MODE only and not for derived modes,

   Then the docs obviously need to be fixed, since there is no nil MODE.

I was somewhat unclear there.  With MODE, I meant the mode on whose
hook the call was added.  Originally, I tried to provide a complete
fix, also working for modes that do not yet follow the new conventions
and there the behavior was completely unpredictable.  (It still is for
such modes, eventually we decided to just document that.)

Anyway, the old docs confused me.  This discussion has started to
concentrate on whether or not they _should_ have confused me.
Somehow, that does not seem to be a very fruitful topic of discussion.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

I rewrote this a different way.

Re: org-mode and mode hooks.

[Carsten Dominik]

Hi guys,

could you please take me off the cc for this discussion?  Thanks.

- Carsten



>>>>> "LT" == Luc Teirlinck <teirllm@dms.auburn.edu> writes:

LT> I still do not understand exactly what a nil argument for MODE in
LT> font-lock-{add,remove}-keywords is supposed to accomplish.  The
LT> font-lock-add-keywords docstring contains the following mysterious
LT> sentence:

LT>   When used from a Lisp program (such as a minor mode), it is recommended to
LT>   use nil for mode (and place the call on a hook) to avoid subtle problems
LT>   due to details of the implementation.

LT> Which subtle problems?

LT> It seems to me that a _global_ minor mode _should_ use a non-nil MODE
LT> argument when adding or removing a keyword.  Do the "subtle problems"
LT> refer to problems that occur when trying to make
LT> font-lock-keywords-alist buffer local, as a buffer local minor mode
LT> might be tempted to do?  A permanent buffer-local minor mode that
LT> wants to add the keyword independent of the mode could use
LT> after-change-major-mode-hook to add it.  With my latest set of patches
LT> that will work for modes that run that hook.  If the minor mode wants
LT> to add it for a particular mode and all its derived modes, it can,
LT> after my patches, add it to the mode hook, as long as the mode and its
LT> ancestor modes follow the new major mode conventions.  Only adding it
LT> to a mode, but not to its derived modes, is problematic after my
LT> patches, as well as without them.

LT> Maybe one could have a font-lock-local-keywords-alist, that would be
LT> _in addition_ to font-lock-keywords-alist.  That should be a
LT> non-permanent local and could be used by non-permanent buffer local minor
LT> modes.  If one is not willing to rely on after-change-major-mode-hook,
LT> there could be a separate font-lock-permanent-local-keywords-alist,
LT> for permanent-local minor modes.

LT> All of this assuming that I am guessing the purpose of a nil argument
LT> correctly.

LT> Sincerely,

LT> Luc.


-- 
Carsten Dominik <dominik@science.uva.nl>        \ _ /
Sterrenkundig Instituut "Anton Pannekoek"        |X|               _
Kruislaan 403; NL-1098 SJ Amsterdam             /| |\   _  _     _/ \
phone +31 (20) 525-7477; FAX +31 (20) 525-7484 __|o|___/ ~~ \___/    ~~~

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

    If one is not willing to rely on after-change-major-mode-hook,
    there could be a separate font-lock-permanent-local-keywords-alist,
    for permanent-local minor modes.

Actually, that should be a list, not an alist, as it does not need to
refer to the major mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my earlier message:

  A permanent buffer-local minor mode that wants to add the keyword
  independent of the mode could use after-change-major-mode-hook to add it.

I forgot to say that it should add it _buffer-locally_ to that hook.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

> Maybe we should label each of these things by the name of the minor
> mode.  That way, we could tell which of them to discard.  Or we could
> give each one a flag saying whether to preserve it when changing the
> major mode.

> Would that solve the problem?

Yes, I think a real solution is to define a new way to add keywords.
It could look like a new var minor-mode-font-lock-keywords-alist modelled
after minor-mode-map-alist.  I think the important part is that the API
should be declarative rather than imperative, so that at any point in time
we can reconstruct font-lock-keywords from font-lock-defaults and whatever
other keyword-sources there can be (such as
minor-mode-font-lock-keywords-alist).


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   Suppose we make font-lock-add-keywords with nil for MODE
   record its argument in some permanent buffer-local variable.
   Then changing the major mode and reenabling font-lock
   would use the same keywords previously added.

As I already said, if that variable _needs_ to be permanent local, then
there are already other problems right now with Font Lock, which are
thousands of times more likely to occur than the ones we are worrying
about right now.  If it does not need to be permanent local, it
appears to duplicate font-lock-keywords-alist, so one can quite as
well use that one.

It would seem easy to make font-lock-{add,remove}-keywords, with a nil
argument for MODE, add/remove the keyword to font-lock-keywords-alist
in addition to enabling or disabling the keyword instantaneously.

Stefan can tell whether that would solve the present problem (I
believe it would) without introducing new unrelated ones (I do not
know about that).

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

	  Anyway, how about if we change define-minor-mode so that it generates
	  the code to record which major mode enabled it?

       The very vast majority of minor modes have no reason whatsoever to do
       this.

   Does that matter?

No, but what _really_ matters is that it is completely impossible for
define-minor-mode to reliably keep track of what major mode the minor
mode was set up for, just as much as it is impossible for
define-global-minor-mode to do this.  Any function could potentially
set up the minor mode for a given major mode, not just the minor mode
function.  Font Lock is a pretty wild example of that.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    No, but what _really_ matters is that it is completely impossible for
    define-minor-mode to reliably keep track of what major mode the minor
    mode was set up for,

What is the difficulty?  Please explain the reasons for this claim.

      Any function could potentially
    set up the minor mode for a given major mode, not just the minor mode
    function.

I do not understand what you're talking about.

      Font Lock is a pretty wild example of that.

Would you please be specific?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

	 Any function could potentially
       set up the minor mode for a given major mode, not just the minor mode
       function.

   I do not understand what you're talking about.

	 Font Lock is a pretty wild example of that.

   Would you please be specific?

I am talking about things like font-lock-{add,remove}-keywords, when
passing nil for MODE.

In addition to this difficulty, not all minor modes are defined with
define-minor-mode.

Note that my latest patch to easy-mmode no longer requires minor modes
to manage the MODE-stored-mode variable themselves.
define-global-minor's managing is not 100% reliable, but now the
variable is only used for efficiency's sake, not for correctness'
sake, so it no longer needs to be 100% reliable.  A minor modes can
manage MODE-stored-mode _if it wants to_.  define-global-minor mode's
managing will not interfere with the minor mode's own managing.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

       The problem is that if you call these functions with `nil' for MODE,
       this sets up Font Lock for the current major mode, which could be an
       ancestor mode (it calls font-lock-set-defaults).

   I don't understand.  How can the current major mode "be" an ancestor
   mode?  Do you mean that the font-lock settings came from an ancestor
   mode?

With "current" major mode, I mean the value of `major-mode' when
font-lock-add-keywords is called.  That could be the parent mode or
even an earlier ancestor mode of the final major mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

	   The problem is that if you call these functions with `nil' for MODE,
	   this sets up Font Lock for the current major mode, which could be an
	   ancestor mode (it calls font-lock-set-defaults).

       I don't understand.  How can the current major mode "be" an ancestor
       mode?  Do you mean that the font-lock settings came from an ancestor
       mode?

    With "current" major mode, I mean the value of `major-mode' when
    font-lock-add-keywords is called.  That could be the parent mode or
    even an earlier ancestor mode of the final major mode.

Ok, now I understand what you meant by that particular point.
Could you give a self-contained and clear explanation of
why this is a problem?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:
       
       With "current" major mode, I mean the value of `major-mode' when
       font-lock-add-keywords is called.  That could be the parent mode or
       even an earlier ancestor mode of the final major mode.

   Ok, now I understand what you meant by that particular point.
   Could you give a self-contained and clear explanation of
   why this is a problem?

Because it initializes Font Lock for the wrong major mode.  Currently,
any attempts to correct this undo prior calls to
font-lock-{add,remove}-keywords that passed nil for MODE.  Some of
these calls we actually might _want_ to undo, but others not, according
to some as yet undocumented rules.

Basically, it appears to me that the way Font Lock handles calls to
font-lock-{add,remove}-keywords that pass nil for MODE is flawed.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   Would you please explain what problem would happen in this case?

I could, but it would be moot.  The patch to easy-mmode I propose
below does not have the problem.  It assumes, however, that the
problems for Font Lock mode get taken care of.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	22 May 2005 16:50:33 -0500	1.63
--- easy-mmode.el	31 May 2005 19:34:34 -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,293 ----
  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)
  	 (buffers (intern (concat global-mode-name "-buffers")))
! 	 (buffers-check (intern (concat global-mode-name "-check-buffers")))
! 	 (cmmh (intern (concat global-mode-name "-cmmh")))
! 	 (stored-mode (intern (concat (symbol-name mode) "-stored-mode"))))
  
      ;; Check keys.
      (while (keywordp (car keys))
***************
*** 294,299 ****
--- 303,310 ----
  				"-mode\\'" "" (symbol-name mode))))))
  
      `(progn
+        (defvar ,stored-mode nil)
+        (make-variable-buffer-local ',stored-mode)
         ;; The actual global minor-mode
         (define-minor-mode ,global-mode
  	 ,(format "Toggle %s in every buffer.
***************
*** 307,314 ****
--- 318,327 ----
  	 (if ,global-mode
  	     (progn
  	       (add-hook 'after-change-major-mode-hook ',buffers)
+ 	       (add-hook 'find-file-hook ',buffers-check)
  	       (add-hook 'change-major-mode-hook ',cmmh))
  	   (remove-hook 'after-change-major-mode-hook ',buffers)
+ 	   (remove-hook 'find-file-hook ',buffers-check)
  	   (remove-hook 'change-major-mode-hook ',cmmh))
  
  	 ;; Go through existing buffers.
***************
*** 325,341 ****
  
         ;; 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))))
  
  ;;;
--- 338,374 ----
  
         ;; The function that calls TURN-ON in each buffer.
         (defun ,buffers ()
! 	 (dolist (buf ,buffers)
! 	   (when (buffer-live-p buf)
! 	     (with-current-buffer buf
! 	       (if ,mode
! 		   (unless (eq ,stored-mode major-mode)
! 		     (,mode -1)
! 		     (,turn-on)
! 		     (setq ,stored-mode major-mode))
! 		 (,turn-on)
! 		 (setq ,stored-mode major-mode))))))
!        (put ',buffers 'definition-name ',global-mode)
! 
!        (defun ,buffers-check ()
! 	 (remove-hook 'post-command-hook ',buffers-check)
  	 (while ,buffers
  	   (let ((buf (pop ,buffers)))
  	     (when (buffer-live-p buf)
! 	       (with-current-buffer buf
! 		 (if ,mode
! 		     (unless (eq ,stored-mode major-mode)
! 		       (,mode -1)
! 		       (,turn-on)
! 		       (setq ,stored-mode major-mode))
! 		   (,turn-on)
! 		   (setq ,stored-mode major-mode)))))))
!        (put ',buffers-check '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-check))
         (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
============================================================

Re: org-mode and mode hooks.

[Richard Stallman]

	     (buffers (intern (concat global-mode-name "-buffers")))
    ! 	 (buffers-check (intern (concat global-mode-name "-check-buffers")))
    ! 	 (cmmh (intern (concat global-mode-name "-cmmh")))
    ! 	 (stored-mode (intern (concat (symbol-name mode) "-stored-mode"))))

The code would be easier to read if these four variables were renamed
to

         (thismode-enable-in-buffers (intern (concat global-mode-name "-enable-in-buffers")))
    ! 	 (thismode-check-buffers (intern (concat global-mode-name "-check-buffers")))
    ! 	 (thismode-cmmh (intern (concat global-mode-name "-cmmh")))
    ! 	 (thismode-recorded-major-mode (intern (concat (symbol-name mode) "-recorded-major-mode"))))


Meanwhile, I see that the function named ,buffers and the function
named ,buffers-check are almost identical.  The only difference, it
appears, is that the latter sets ,buffers to nil.  Why not make
it call the other one?

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)
============================================================

Re: org-mode and mode hooks.

[Richard Stallman]

This patch looks good to me.  Please install it.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   This patch looks good to me.  Please install it.

Just to avoid confusion: you mean that all three patches are OK to
install?  The one to easy-mmode _and_ the ones to font-lock and
font-core?  I can not install the patch to easy-mmode without
installing the other two, or without some equivalent change to Font
Lock.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    Just to avoid confusion: you mean that all three patches are OK to
    install?  The one to easy-mmode _and_ the ones to font-lock and
    font-core?

Yes.

Re: org-mode and mode hooks.

[Luc Teirlinck]

I probably did not quote enough in my latest reply.  The latest patch to
easy-mmode, which I just come to send, no longer has the following problem:

       Note that even after this patch, there is one potentially troublesome
       case left.  It concerns the case of a major mode that gets enabled by
       a timer or process for a non-file visiting buffer.

   Would you please explain what problem would happen in this case?

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Here is still another patch to easy-mmode.  It gets rid of the new
MODE-stored-mode variable, replacing it with the condition that dis-
and re-enabling MODE should adapt MODE reliably to the current major
mode.  That means that one way or the other, we have to take care of
Font Lock, which currently does not satisfy this condition.  Either we
make it satisfy the condition, or we still need a
font-lock-mode-stored-hook and an associated error message.

Note that even after this patch, there is one potentially troublesome
case left.  It concerns the case of a major mode that gets enabled by
a timer or process for a non-file visiting buffer.  (Even in this case
it is only likely to occur for major modes that do not follow the new
conventions.)  It is easy to get rid of this remaining problem.
However, it would require that not only the final check by
find-file-hook or after-command-hook, but also the check by
after-change-major-mode-hook, disables the mode if it already is
enabled and then re-enables it.  The patch below will sometimes, but
only in a _very_ small minority of the cases, _unnecessarily_ disable
and re-enable the mode.  To get rid of the remaining problem, we would
have to do such an unnecessary procedure nearly every time and
occasionally we would disable and re-enable unnecessarily more than
once.  In practice, the remaining case probably only affects
autoreverting of non-file buffers, so it means that each time we
autorevert an additional non-file type, we would need to check that
its usual major mode and its parent modes adhere to the new major mode
conventions and do not cause trouble in some other freakish way.  Again,
if we are willing to pay a price in efficiency, getting rid of the
remaining (rare) problem is easy.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	22 May 2005 16:50:33 -0500	1.63
--- easy-mmode.el	29 May 2005 17:52:33 -0500	
***************
*** 271,283 ****
  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.
--- 271,291 ----
  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)
  	 (buffers (intern (concat global-mode-name "-buffers")))
+ 	 (buffers-check (intern (concat global-mode-name "-check-buffers")))
  	 (cmmh (intern (concat global-mode-name "-cmmh"))))
  
      ;; Check keys.
***************
*** 307,314 ****
--- 315,324 ----
  	 (if ,global-mode
  	     (progn
  	       (add-hook 'after-change-major-mode-hook ',buffers)
+ 	       (add-hook 'find-file-hook ',buffers-check)
  	       (add-hook 'change-major-mode-hook ',cmmh))
  	   (remove-hook 'after-change-major-mode-hook ',buffers)
+ 	   (remove-hook 'find-file-hook ',buffers-check)
  	   (remove-hook 'change-major-mode-hook ',cmmh))
  
  	 ;; Go through existing buffers.
***************
*** 325,341 ****
  
         ;; 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))))
  
  ;;;
--- 335,361 ----
  
         ;; The function that calls TURN-ON in each buffer.
         (defun ,buffers ()
! 	 (dolist (buf ,buffers)
! 	   (when (buffer-live-p buf)
! 	     (with-current-buffer buf
! 	       (unless ,mode
! 		 (,turn-on))))))
!        (put ',buffers 'definition-name ',global-mode)
! 
!        (defun ,buffers-check ()
! 	 (remove-hook 'post-command-hook ',buffers-check)
  	 (while ,buffers
  	   (let ((buf (pop ,buffers)))
  	     (when (buffer-live-p buf)
! 	       (with-current-buffer buf
! 		 (when ,mode (,mode -1))
! 		 (,turn-on))))))
!        (put ',buffers-check '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-check))
         (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
============================================================

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

   Note that even after this patch, there is one potentially troublesome
   case left...

Sorry, I was confused.  That entire long paragraph makes no sense
(sorry about that).  Without MODE-stored-mode, I can not avoid some
inefficiency for non-file buffers (unnecessarily dis- and re-enable
the mode once), unless I somehow can recognize whether we are running
from a timer or process.  (I can not immediately think of way to do
that.  Is there away?)  Getting rid of the one remaining bug I
described _and_ avoiding a similar inefficiency for file buffers can
be accomplished using the following alternate ,buffers function, so it
seems definitely better than the one in my previous patch:

       ;; The function that calls TURN-ON in each buffer.
       (defun ,buffers ()
	 (dolist (buf ,buffers)
	   (and (buffer-live-p buf)
		(not buffer-file-name)
		(with-current-buffer buf
		  (when mode (,mode -1))
		  (,turn-on)))))

It is actually _here_ (in after-change-major-mode-hook) that disabling
and re-enabling is unlikely to occur, so getting rid of the remaining
bug carries no real price, but getting rid of MODE-stored-hook does
carry some price, for non-file buffers.  I will still think of this further.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

   unless I somehow can recognize whether we are running
   from a timer or process.  (I can not immediately think of way to do
   that.  Is there away?)

To answer my own question, I guess that checking whether
`this-command' is nil should work reliably enough.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

      unless I somehow can recognize whether we are running
      from a timer or process.  (I can not immediately think of way to do
      that.  Is there away?)

   To answer my own question, I guess that checking whether
   `this-command' is nil should work reliably enough.

No, too many functions can reset it.

I guess that `this-original-command' could _potentially_ be better.
However, apparently, it does not get reset to nil when this-command
gets reset to nil (why not?), making it useless for the purpose. 

If real_this_command would be exported to Lisp, it would probably be
ideal, assuming there is no other reliable way to check whether code
is being run from a timer or asynchronous process.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

>From my previous message:

   If real_this_command would be exported to Lisp, it would probably be
   ideal, assuming there is no other reliable way to check whether code
   is being run from a timer or asynchronous process.

Well, a timer or asynchronous process can run during a command, in
which case all the this-command style functions would be non-nil,
but fontification being delayed until the actual command finishes
might not be too confusing in this infrequent situation.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Maybe I could still use MODE-stored-mode as a define-global-minor-mode
internal variable for efficiency reasons (unless it is easy to
recognize whether we are running from a process or timer, in which
case it is not necessary).  That way, we are checking whether
_after-change-major-mode-hook_ got the major mode correct.  If it did,
nothing could have messed it up later, since we are already in the
final mode.  Thus, post-command-hook does not need to dis- and
re-enable the minor mode.  This is what will happen in the vast
majority of the cases.  Otherwise, we dis- and re-enable.

The trouble with doing that for Font Lock, given its current code, is
what happens if after-change-major-mode never ran Font Lock, and Font
Lock is nevertheless enabled.  Then it was set by something else.
Without help from Font Lock itself, we can not know whether that was for
the correct major mode or not.  This will happen infrequently enough
that we do not need to worry about a small and unlikely inefficiency
here.  However, the trouble with the current Font Lock code is that
trying to correct the major mode is a last resort desperation measure
that may not work reliably, so we must be completely sure that it is
really necessary.  For Font Lock, we are not talking about a small
inefficiency, but about real trouble.  That would no longer be the
case if dis- and re-enabling Font Lock reliably adapted it for the
current mode.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Here are the latest versions of my patches to easy-mmode and
font-core.  The patch to cwarn remains unchanged from the revised
version I sent earlier.

The docstring of `define-global-minor-mode' now says that if the minor
mode's setup depends on the major mode for which it was enabled, it
should first of all try to make sure that dis- and then re-enabling
the minor mode corrects for the new major mode, in which case no
warning is necessary.  Unless I misunderstood, this is already the
case for cwarn.  For some reason it is apparently impossible to
achieve this for Font Lock, which is what causes a lot of the trouble.
The new version says that any mode that can not arrange for a reliable
correction to the new mode, should warn the user if the problem
occurs.  Hopefully, we are only talking about Font Lock.  The new
patch to font-core makes font-lock-mode issue a warning, thereby
taking care of Stefan's objection.  `define-global-minor-mode' no
longer warns, as this would hopefully be unnecessary for most minor
modes and we do not want to warn twice about the same problem for Font
Lock.  The Font Lock problem (and the problem for hypothetical other
modes in the same situation) can occur outside of define-global-minor-mode's
control, and hence `define-global-minor-mode' can not handle all
necessary warnings.

===File ~/easy-mmode-diff===================================
*** easy-mmode.el	22 May 2005 16:50:33 -0500	1.63
--- easy-mmode.el	28 May 2005 17:12:33 -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,306 ----
  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 enabling MODE should store the value of
! the variable `major-mode' in the variable MODE-stored-mode.
! If `define-global-minor-mode' notices that MODE is configured
! for the wrong major mode, it disables and then reenables MODE.
! Normally, you should write MODE in such a way that this will
! make MODE work correctly with the current major mode.  If this is
! impossible, then enabling the MODE should check the value of
! MODE-stored-mode and warn the user if it is not the current major mode.
! 
! The above is important to make MODE work correctly with \"derived\"
! major modes, that is major modes that call another major mode in their body.
! When switching major modes interactively, `kill-local-variables'
! should eliminate the old major mode's setup.
! 
! `define-global-minor-mode' provides a defvar for MODE-stored-hook,
! with initial value nil, and makes it automatically buffer-local.
! If MODE's set-up is independent of the major mode in effect when it
! was enabled, the value of the variable should stay nil."
    (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")))
! 	 (buffers-check (intern (concat global-mode-name "-check-buffers")))
! 	 (cmmh (intern (concat global-mode-name "-cmmh")))
! 	 (stored-mode (intern (concat (symbol-name mode) "-stored-mode"))))
  
      ;; Check keys.
      (while (keywordp (car keys))
***************
*** 294,299 ****
--- 316,323 ----
  				"-mode\\'" "" (symbol-name mode))))))
  
      `(progn
+        (defvar ,stored-mode nil)
+        (make-variable-buffer-local ',stored-mode)
         ;; The actual global minor-mode
         (define-minor-mode ,global-mode
  	 ,(format "Toggle %s in every buffer.
***************
*** 307,314 ****
--- 331,340 ----
  	 (if ,global-mode
  	     (progn
  	       (add-hook 'after-change-major-mode-hook ',buffers)
+ 	       (add-hook 'find-file-hook ',buffers-check)
  	       (add-hook 'change-major-mode-hook ',cmmh))
  	   (remove-hook 'after-change-major-mode-hook ',buffers)
+ 	   (remove-hook 'find-file-hook ',buffers-check)
  	   (remove-hook 'change-major-mode-hook ',cmmh))
  
  	 ;; Go through existing buffers.
***************
*** 325,341 ****
  
         ;; 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))))
  
  ;;;
--- 351,378 ----
  
         ;; The function that calls TURN-ON in each buffer.
         (defun ,buffers ()
! 	 (dolist (buf ,buffers)
! 	   (when (buffer-live-p buf)
! 	     (with-current-buffer buf
! 	       (unless ,mode
! 		 (,turn-on))))))
!        (put ',buffers 'definition-name ',global-mode)
! 
!        (defun ,buffers-check ()
! 	 (remove-hook 'post-command-hook ',buffers-check)
  	 (while ,buffers
  	   (let ((buf (pop ,buffers)))
  	     (when (buffer-live-p buf)
! 	       (with-current-buffer buf
! 		 (or (not ,stored-mode)
! 		     (eq ,stored-mode major-mode)
! 		     (,mode -1))
! 		 (,turn-on))))))
  
         ;; The function that catches kill-all-local-variables.
         (defun ,cmmh ()
  	 (add-to-list ',buffers (current-buffer))
! 	 (add-hook 'post-command-hook ',buffers-check))
         (put ',cmmh 'definition-name ',global-mode))))
  
  ;;;
============================================================

===File ~/font-core-diff====================================
*** font-core.el	22 May 2005 16:46:07 -0500	1.28
--- font-core.el	28 May 2005 19:34:05 -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-stored-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,172 ----
    ;; 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
!     (or (not font-lock-mode-stored-mode)
! 	(eq font-lock-mode-stored-mode major-mode)
! 	(message
! 	 "Font Lock was set up for wrong major mode: %s.
! We tried to correct.  This is a bug.  Please report it."
! 	 font-lock-mode-stored-mode))
!     (setq font-lock-mode-stored-mode major-mode)))
  
  ;; Get rid of fontification for the old major mode.
  ;; We do this when changing major modes.
***************
*** 175,180 ****
--- 185,191 ----
  				      '(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
***************
*** 203,209 ****
    (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 ()
--- 214,232 ----
    (when (or font-lock-defaults
  	    (and (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-stored-mode
! 		 (not (eq font-lock-mode-stored-mode major-mode))))
!     (and mode
! 	 font-lock-mode-stored-mode
! 	 (not (eq font-lock-mode-stored-mode major-mode))
! 	 ;; This may not work completely correctly, but it is the best
! 	 ;; we can do in the given situation.  We will warn the user
! 	 ;; about the problem later, in the `font-lock-mode' function.
! 	 (setq font-lock-set-defaults nil))
      (font-lock-mode-internal mode)))
  
  (defun turn-on-font-lock ()
============================================================

Re: org-mode and mode hooks.

[Richard Stallman]

      Hopefully, we are only talking about Font Lock.  The new
    patch to font-core makes font-lock-mode issue a warning, thereby
    taking care of Stefan's objection.

It sounds like you are saying that with your patch, Font Lock mode
still works wrong.  And all the patch does is warn the user when it
works wrong.

Do I understand right?

If so, that means this fix doesn't fix things.
We need to figure out what to do about Font Lock mode.

Can you explain the problem with Font Lock mode
that remains after your proposed patch?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

   It sounds like you are saying that with your patch, Font Lock mode
   still works wrong.

Essentially, yes.  It can only work correctly if all major modes not
only adhere strictly to our new Emacs 22 conventions, but in addition
carefully avoid anything that enables font-lock (other than
through the define-global-minor-mode machinery) or that calls
font-lock-{add,remove}-keywords, directly or indirectly.

   If so, that means this fix doesn't fix things.
   We need to figure out what to do about Font Lock mode.

The _one_ big problem is that dis- and then re-enabling Font Lock is
not sufficient to reliably adapt Font Lock for the current major mode
if it has already been configured earlier in an ancestor mode.  (In
fact there seems to be currently _no way whatsoever_ to reliably
correct a prior configuration for an ancestor mode.)  Without this
problem Font Lock does _not_ need to keep track of the major mode.  In
fact, even major modes that do not adhere to the new conventions no
longer would present problems.

I believe that it is definitely theoretically possible to redesign
Font Lock in such a way that dis- and re-enabling it would configure
reliably for the current major mode, even with prior misconfiguration.
This would seem highly desirable.  But it seems non-trivial and
requires a much better knowledge of all the intricate details of Font
Lock than I personally have.  So somebody else would have to do it.

   If so, that means this fix doesn't fix things.
   We need to figure out what to do about Font Lock mode.

   Can you explain the problem with Font Lock mode
   that remains after your proposed patch?

Stefan knows the Font Lock code better than I do.  But if I understand
things correctly (otherwise, Stefan can correct me) the problem is this
excerpt from `(elisp)Search-based Fontification', talking about
font-lock-{add,remove}-keywords:

  When you use these functions from a Lisp program (such
  as a minor mode), we recommend that you use `nil' for MODE (and place
  the call on a hook) to avoid subtle problems due to the details of the
  implementation.

The problem is that if you call these functions with `nil' for MODE,
this sets up Font Lock for the current major mode, which could be an
ancestor mode (it calls font-lock-set-defaults).  This problem also
occurs if the ancestor mode enables font-lock-mode.  But the problem
with font-lock-add-keywords with `nil' for MODE is _way worse_.  If
all that happened was that font-lock-mode was enabled too soon, but
without font-lock-{add,remove}-keywords being ever called, then I
believe that my patch will reliably correct for the actual final major
mode.  But if font-lock-add-keywords was called with `nil' for MODE,
my patch will undo the adding of the keyword.  However, a `nil'
argument means that the adding should be independent of the MODE, so
we are in trouble.

The only way out of this trouble, without non-trivial changes to Font
Lock, is a convention that major modes should not directly or
indirectly call font-lock-{add,remove}-keywords.  I do not even know
whether that convention would be reasonable.  But even if we adopt
that convention, we should hunt for modes that do not follow it and
correct them, hence the error message.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

I still found some problems with my latest patches to font-core.  I
have corrected patches for font-core and font-lock.  However, since I
do not know whether or not we will instead opt to make dis- and
re-enable Font Lock take care of the problem (which it currently does
not), I do not send the corrected patches.  But I could send them if
we want to use the font-lock-mode-stored-mode method, either
permanently, or, more likely, temporarily until a better solution is
implemented.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

My original patch for font-lock.el had problems.  It did not work well
for modes that only use font-lock for font-lock-face and it did not
work for modes with a non-standard font-lock-function.  I now propose
to leave font-lock.el unchanged and to apply the patch below to
font-core.el instead.

I also propose a slightly changed second paragraph to the
`define-global-minor-mode' docstring.  The revised patch to cwarn.el
is completely equivalent to my original.  It just adapts the changes
to the revised `define-global-minor-mode' docstring.

Revised second paragraph of `define-global-minor-mode' docstring:

If the minor mode's set-up depends on the major mode, then enabling
the minor mode should store the value of the variable `major-mode' in
the variable MODE-stored-mode.  This is used to catch bugs where MODE
is set up for the wrong mode.  `define-global-minor-mode' provides
a defvar for MODE-stored-hook, with initial value nil, and makes it
automatically buffer-local.  If MODE's set-up is independent of
the major mode, the value of the variable should stay nil.

New patches to font-core.el and cwarn.el:

===File ~/font-core-diff====================================
*** font-core.el	22 May 2005 16:46:07 -0500	1.28
--- font-core.el	27 May 2005 20:01:05 -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-stored-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,165 ----
    ;; 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-stored-mode major-mode)))
  
  ;; Get rid of fontification for the old major mode.
  ;; We do this when changing major modes.
***************
*** 203,209 ****
    (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,217 ----
    (when (or font-lock-defaults
  	    (and (boundp 'font-lock-keywords) font-lock-keywords)
  	    (with-no-warnings
! 	      (cdr (assq major-mode font-lock-defaults-alist)))
! 	    (and (boundp 'font-lock-set-defaults)
! 		 font-lock-set-defaults
! 		 (not (eq font-lock-mode-stored-mode major-mode))))
!     (unless (eq font-lock-mode-stored-mode major-mode)
!       (setq  font-lock-set-defaults nil))
      (font-lock-mode-internal mode)))
  
  (defun turn-on-font-lock ()
============================================================

===File ~/cwarn-diff========================================
*** cwarn.el	04 Apr 2005 07:03:09 -0500	1.9
--- cwarn.el	27 May 2005 20:03:24 -0500	
***************
*** 184,189 ****
--- 184,190 ----
  ;;}}}
  ;;{{{ The modes
  
+ (defvar cwarn-mode-stored-mode)
  ;;;###autoload
  (define-minor-mode cwarn-mode
    "Minor mode that highlights suspicious C and C++ constructions.
***************
*** 195,201 ****
  With ARG, turn CWarn mode on if and only if arg is positive."
    :group 'cwarn :lighter cwarn-mode-text
    (cwarn-font-lock-keywords cwarn-mode)
!   (if font-lock-mode (font-lock-fontify-buffer)))
  
  ;;;###autoload
  (defun turn-on-cwarn-mode ()
--- 196,203 ----
  With ARG, turn CWarn mode on if and only if arg is positive."
    :group 'cwarn :lighter cwarn-mode-text
    (cwarn-font-lock-keywords cwarn-mode)
!   (if font-lock-mode (font-lock-fontify-buffer))
!   (when cwarn-mode (setq cwarn-mode-stored-mode major-mode)))
  
  ;;;###autoload
  (defun turn-on-cwarn-mode ()
============================================================

Re: org-mode and mode hooks.

[Michael Mauger]

Luc Teirlinck <teirllm <at> dms.auburn.edu> writes:

> 
> Stefan Monnier wrote:
> 
>    Can you explain the change?
>    I really don't like the
> 
>      (defun turn-on-font-lock-as-appropriate ()
>        (let (font-lock-set-defaults)
> 	 (turn-on-font-lock-if-enabled)))
> 
>    since it may cause font-lock-keywords to be reset from font-lock-defaults,
>    thus throwing away keywords added via font-lock-add-keywords since.
> 
> The problem that binding font-lock-defaults to nil tries to solve is
> that (with org-mode's yesterday's version), after font-lock was turned
> on in outline-mode, I could not get it correct for org-mode by
> disabling and enabling font-lock-mode, font-lock just kept using
> outline-mode's font-lock settings.  

I've run into the same problem in sql-mode; the need to alter the font-lock 
rules after font-lock-mode has been enabled.  My current solution is as follows:

    ;; Force font lock to reinitialize if it is already on
    ;; Otherwise, we can wait until it can be started.
    (when (and (fboundp 'font-lock-mode)
	       (boundp 'font-lock-mode)
	       font-lock-mode)
      (font-lock-defontify)
      (font-lock-mode -1)
      (font-lock-mode 1))

The `defontify' seems to be the ticket to get font-lock to recalc its settings 
and actually reperform the fontification.  My original solution used internals 
from font-lock that were not version stable.

Stefan, can you confirm that this approach should work?  The docs for `font-
lock-defontify' certainly imply that this function should do what we are 
looking for.  Thanks.

-- Michael

Re: org-mode and mode hooks.

[Luc Teirlinck]

Michael Mauger wrote:

       ;; Force font lock to reinitialize if it is already on
       ;; Otherwise, we can wait until it can be started.
       (when (and (fboundp 'font-lock-mode)
		  (boundp 'font-lock-mode)
		  font-lock-mode)
	 (font-lock-defontify)
	 (font-lock-mode -1)
	 (font-lock-mode 1))

   The `defontify' seems to be the ticket to get font-lock to recalc
   its settings and actually reperform the fontification.  My original
   solution used internals from font-lock that were not version
   stable.

It seems to me, both from docstring and code, that font-lock-defontify
only takes care of the font-lock-face text property.

After changes I will propose later today to fix two bugs in
define-global-minor-mode, (progn (font-lock-mode -1) (font-lock-mode 1))
will reset font-lock for the current mode, if necessary.  (After the
change, font-lock remembers which mode it was set up for.)  I do not
believe that this is an incompatible change, as having font-lock
enabled for the wrong mode makes no sense.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Michael Mauger wrote:

       ;; Force font lock to reinitialize if it is already on
       ;; Otherwise, we can wait until it can be started.
       (when (and (fboundp 'font-lock-mode)
		  (boundp 'font-lock-mode)
		  font-lock-mode)
	 (font-lock-defontify)
	 (font-lock-mode -1)
	 (font-lock-mode 1))

   The `defontify' seems to be the ticket to get font-lock to recalc
   its settings and actually reperform the fontification.  My original
   solution used internals from font-lock that were not version
   stable.

Actually, contrary to what I said before, the change I will soon
propose might not automatically take care of thew `font-lock-face'
text property.  I believe that font-lock relies on a major mode that
sets font-lock-face to disable it, if appropriate, in
change-major-mode-hook.  From the docstring:

  Clear out all `font-lock-face' properties in current buffer.
  A major mode that uses `font-lock-face' properties might want to put
  this function onto `change-major-mode-hook'.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Stefan Monnier]

> I've run into the same problem in sql-mode; the need to alter the
> font-lock  rules after font-lock-mode has been enabled.  My current
> solution is as follows:

>     ;; Force font lock to reinitialize if it is already on
>     ;; Otherwise, we can wait until it can be started.
>     (when (and (fboundp 'font-lock-mode)
> 	       (boundp 'font-lock-mode)
> 	       font-lock-mode)
>       (font-lock-defontify)
>       (font-lock-mode -1)
>       (font-lock-mode 1))

Does this work?  I'm surprised since I don't think any one of those
3 functions resets font-lock-set-defaults.

> The `defontify' seems to be the ticket to get font-lock to recalc its
> settings  and actually reperform the fontification.

Huh?  Actually you probably shouldn't call font-lock-defontify, especially
since you don't seem to understand what it does.

> Stefan, can you confirm that this approach should work?  The docs for `font-
> lock-defontify' certainly imply that this function should do what we are 
> looking for.  Thanks.

I don't think any approach will work except for something like:

  (font-lock-mode -1)
  (kill-local-variable 'font-lock-set-default)
  (font-lock-mode 1)

Although this of course depends a lot on how you change the settings.
The best way (which doesn't mean it's good) to change settings currently is
probably with font-lock-add-keywords.
A simpler alternative is to use a function-matcher so you don't even have to
tell font-lock to update anything, you can instead just change some of the
variables used by your function (this is what I used in sh-script).


        Stefan

Re: org-mode and mode hooks.

[Luc Teirlinck]

Both Stefan and me might have partially misunderstood your problem.

`sql-interactive-mode' calls comint-mode without enclosing it in a
delay-mode-hooks form, as it should.  To standardize your mode, you
should either define it using `define-derived-mode' (see `(elisp)Derived
Modes') or read `(elisp)Mode Hooks'.

The reason why you need `font-lock-defontify' is because I believe (I
did not check this in detail) that all comint uses font-lock for is
font-lock-face, which is exactly what `font-lock-defontify' undoes.
My patches do not try to mend this because most comint derived modes
probably _want_ to keep comint's font-lock-face defaults.

After standardizing your mode, you _maybe_ might still have to run
font-lock-defontify to remove comint's font-lock-face properties.
(I did not study your code in detail, nor am I completely sure about
what your exact problem is.)

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Michael Mauger]

--- Luc Teirlinck <teirllm@dms.auburn.edu> wrote:

> Both Stefan and me might have partially misunderstood your problem.
> 

That's okay, I've only understood about 2% of this whole thread. :)

> `sql-interactive-mode' calls comint-mode without enclosing it in a
> delay-mode-hooks form, as it should.  To standardize your mode, you
> should either define it using `define-derived-mode' (see
> `(elisp)Derived
> Modes') or read `(elisp)Mode Hooks'.
> 

I've added the `delay-mode-hooks' to the code, so this should be correct
going forward.  I inherited this mode so I'm still grasping some of it's
inner workings.  

In fact, it hasn't been `sql-interactive-mode' that has had the font-lock
problems.  The font-lock setup needed in sql-interactive-mode is known
when the mode starts, so it can be setup during initialization of the
mode.  I also verified that sql-interactive-mode does not enable
font-lock-mode.  It may flip it off and on (after killing
font-lock-set-defaults, to get it to reinitialize), iff it is already
turned on.

The other mode in sql.el is `sql-mode' which is designed to provide
editting support of sql scripts is where font-lock has been a challenge. 
This is not a derived mode so that is not the cause.  The need here is to
change the keyword rules based upon a local variable section setting or a
menu selection.  I can easily recalculate the font-lock-defaults needed,
the problem was getting font-lock to refresh its internal configuration
based on new font-lock-defaults settings.  My suggestion to use
`font-lock-defontify' was the result of several late nite debugging
sessions about six months ago.  It seems to no longer be necessary. 
Stefan's suggestion to kill `font-lock-set-keywords' was already in place
but it didn't seem to have a visual impact unless I called
`font-lock-defontify'.

> The reason why you need `font-lock-defontify' is because I believe (I
> did not check this in detail) that all comint uses font-lock for is
> font-lock-face, which is exactly what `font-lock-defontify' undoes.
> My patches do not try to mend this because most comint derived modes
> probably _want_ to keep comint's font-lock-face defaults.
> 
> After standardizing your mode, you _maybe_ might still have to run
> font-lock-defontify to remove comint's font-lock-face properties.
> (I did not study your code in detail, nor am I completely sure about
> what your exact problem is.)
> 

I think sql.el is now properly behaving in terms of its responsibilities
relative to derived and minor modes.  I'll continue to test it until
changes in this area settle down.  I will release the patch once I am
comfortable that things are stable again...

Re: org-mode and mode hooks.

[Luc Teirlinck]

To clarify, this is how comint enables Font Lock:

(defcustom comint-mode-hook '(turn-on-font-lock)
...

Without delay-mode-hooks, this is executed at the end of the call to
comint-mode.  With delay-mode-hooks, it is run during the call to
run-mode-hooks, just before your own hook.  Note that a user can
customize comint-mode-hook and remove the enabling of font-lock-mode.

I did check that comint-mode does not load font-lock, so it only uses
font-lock for font-lock-face.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Richard Stallman]

    To clarify, this is how comint enables Font Lock:

    (defcustom comint-mode-hook '(turn-on-font-lock)

Shouldn't we remove that?  Is it a desirable thing, nowadays,
for some modes to enable Font Lock on their own, instead of letting
the user do so if he wants?

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

       To clarify, this is how comint enables Font Lock:

       (defcustom comint-mode-hook '(turn-on-font-lock)

   Shouldn't we remove that?  Is it a desirable thing, nowadays,
   for some modes to enable Font Lock on their own, instead of letting
   the user do so if he wants?

At first view, I would vote for removing it, but let us see what
Miles, who added it, has to say.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallman wrote:

       To clarify, this is how comint enables Font Lock:

       (defcustom comint-mode-hook '(turn-on-font-lock)

   Shouldn't we remove that?  Is it a desirable thing, nowadays,
   for some modes to enable Font Lock on their own, instead of letting
   the user do so if he wants?

Unless Miles objects, I will install the following mini-patch to comint.el:

===File ~/comint-diff=======================================
*** comint.el	26 May 2005 09:05:21 -0500	1.319
--- comint.el	06 Jun 2005 20:01:06 -0500	
***************
*** 423,429 ****
  (make-obsolete-variable 'comint-use-prompt-regexp-instead-of-fields
  			'comint-use-prompt-regexp "22.1")
  
! (defcustom comint-mode-hook '(turn-on-font-lock)
    "Hook run upon entry to `comint-mode'.
  This is run before the process is cranked up."
    :type 'hook
--- 423,429 ----
  (make-obsolete-variable 'comint-use-prompt-regexp-instead-of-fields
  			'comint-use-prompt-regexp "22.1")
  
! (defcustom comint-mode-hook nil
    "Hook run upon entry to `comint-mode'.
  This is run before the process is cranked up."
    :type 'hook
============================================================

Re: org-mode and mode hooks.

[Miles Bader]

On 6/7/05, Luc Teirlinck <teirllm@dms.auburn.edu> wrote:
>        (defcustom comint-mode-hook '(turn-on-font-lock)
> 
>    Shouldn't we remove that?  Is it a desirable thing, nowadays,
>    for some modes to enable Font Lock on their own, instead of letting
>    the user do so if he wants?
> 
> Unless Miles objects, I will install the following mini-patch to comint.el:

I have no idea.

If I recall correctly, the reason I added that is because I replaced
some previous ad-hoc highlighting code -- which was always turned on
-- with font-lock.

In Emacs 21, the behavior was that comint prompts were _always_
highlighted, but font-lock could be turned on to add other faces
(e.g., colorization of error messages).  When I changed the
prompt-highlighting code I turned on font-lock by default because
highlighted prompts were the previous default behavior, and I figured
the prompts were the most "important" part of comint highlighting.

So this patch would change the default appearance of comint prompts in
Emacs 22.  I don't know if the average user would be annoyed/confused
("my prompts disappeared!") or happy ("ah those evil colorized prompts
are gone!") by such a change.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Miles Bader wrote:
   
   So this patch would change the default appearance of comint prompts in
   Emacs 22.  I don't know if the average user would be annoyed/confused
   ("my prompts disappeared!") or happy ("ah those evil colorized prompts
   are gone!") by such a change.

Because recently several face properties got changed to font-lock-face
properties, exactly the same remarks apply to several other types of
buffers, for instance, the Buffer Menu.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Miles Bader]

On 6/7/05, Luc Teirlinck <teirllm@dms.auburn.edu> wrote:
>    So this patch would change the default appearance of comint prompts in
>    Emacs 22.  I don't know if the average user would be annoyed/confused
>    ("my prompts disappeared!") or happy ("ah those evil colorized prompts
>    are gone!") by such a change.
> 
> Because recently several face properties got changed to font-lock-face
> properties, exactly the same remarks apply to several other types of
> buffers, for instance, the Buffer Menu.

Ok, then it's a general question.

I suppose the answer is really "it depends".  Many people probably
like some highlighting but not all.  It seems the sort of thing that
might actually be best handled by the concept of font-lock levels, but
my impression is that they are not really used these days, and that
it's fruit-salad or nothing.

-Miles
-- 
Do not taunt Happy Fun Ball.

Re: org-mode and mode hooks.

[Stefan Monnier]

> I suppose the answer is really "it depends".  Many people probably
> like some highlighting but not all.  It seems the sort of thing that
> might actually be best handled by the concept of font-lock levels, but
> my impression is that they are not really used these days, and that
> it's fruit-salad or nothing.

I hate font-lock levels.  They're very poorly defined and thus can't be used
globally (you have to set them separately for each major mode) and they
never give me the control I want (I generally want to turn off some parts of
some level).

One of the problems is that levels are generally chosen by the implementer
to reflect the amount of work it takes to figure out what to highlight, so
the user can reduce the level if the highlighting slows down his Emacs
session.  In my experience I want expensive highlighting but little of it
(i.e. no fruit salad), so I often would rather turn off parts of the level-1
highlighting while I'd rather keep most/all of level 3 (for example).

E.g. it's cheap to highlight every reserved keyword, so it's in level-1, but
I generally can't care less about them.  So I end up tweaking
font-lock-keyword-face instead of font-lock-maximum-decoration.


        Stefan

Re: org-mode and mode hooks.

[Stefan Monnier]

> In Emacs 21, the behavior was that comint prompts were _always_
> highlighted, but font-lock could be turned on to add other faces
> (e.g., colorization of error messages).  When I changed the

In Emacs-20 the behavior was: no highlighting unless you turn on font-lock.
So the patch just gets us back to Emacs-20 behavior in this regard.


        Stefan

Re: org-mode and mode hooks.

[Richard Stallman]

    So this patch would change the default appearance of comint prompts in
    Emacs 22.

That change is ok.  We have decided to move towards a more consistent
system where buffer-specific highlighting can be enabled and disabled
by calling font-lock-mode.  This inevitably means an incompatible
change for modes that used to do highlighting unconditionally.

Re: org-mode and mode hooks.

[Richard Stallman]

    The following changes fix both bugs, by using post-command-hook to
    check whether font-lock was enabled for the correct mode and adapting
    the turn-on function to be able to correct it.

That is a clever solution.  However, I think there may be a bug in the
code.  The reason is that any number of different minor modes could be
defined using define-global-minor-mode.  If all of them try to use the
buffer-local binding for easy-mmode-stored-mode, the various
check-buffers functions will step over each other.

Thus, I think each global minor mode needs to have its own
variable for this purpose.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Richard Stallmn wrote:

   Thus, I think each global minor mode needs to have its own
   variable for this purpose.

That is correct.  I will change my patch to implement this, once we
are sure that my patch is really basically the way to go.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Well, the second bug can still occur in combination with the first,
namely if the mode is run from a timer.  Maybe I could make
find-file-hook do checking too.  In that case a bug could only still
occur if it already occurred before my change.

Sincerely,

Luc.

Re: org-mode and mode hooks.

[Lute Kamstra]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

[...]

> I also do not immediately understand why find-file-hook could not
> potentially run too early too.

If I understand things correctly, after-find-file first calls the
major mode function (by calling normal-mode) and then runs
find-file-hook.

Lute.

Re: org-mode and mode hooks.

[Lute Kamstra]

Hi Carsten,

> I remember trying to do define-derived mode, but not doing it for a
> number of reasons which I do not exactly remember.  The argument was
> one reason.  Another was that older versions of define-derived-mode
> did not allow a BODY arguments which I needed.

> A third one was that org-mode hacks away the Show and Hide menus of
> outline mode, and I did not want a mode hood to fail because is
> tries to add entries to non-existing menus.

With my patch, running outline-mode's mode hook (and
after-change-major-mode-hook) is delayed; it is run at the end of
org-mode.  So it introduces the problem that a mode hook will fail
when it tries to change those menus.

> An important problem always is that I am writing code for Emacs and
> XEmacs, so my code needs to be compatible with both distributions.

AFAIK, XEmacs has delay-mode-hooks and run-mode-hooks as well.

> I did not know about delay-mode-hooks and run-mode-hooks - I'll check
> it out.
>
> What is actually the problem created by the current setup?  What are
> you trying to do in the outline-mode-hook, and how does it interfer
> with what org-mode is doing?

My problem is not with outline-mode-hook but with
after-change-major-mode-hook.

In Emacs, global-font-lock-mode (and any other global minor mode
defined with define-global-minor-mode) adds one function to
change-major-mode-hook and a second function to
after-change-major-mode-hook to enable Font Lock mode.  The first
functions adds the current buffer to a list variable.  The second
function enables Font Lock mode for all buffers in that list and
clears the list.

change-major-mode-hook is run by kill-all-local-variables and
after-change-major-mode-hook is run by run-mode-hooks.  So normally, a
major mode calls the first function once as the first thing it does
and the second function once as the last thing it does.  However,
org-mode calls the second function halfway, before it sets up its own
Font-Lock stuff.  As a result Font Lock doesn't work properly for
org-mode.

Lute.


PS.  Putting the second function on after-change-major-mode-hook is a
     recent change, it used to be on find-file-hook.

Re: org-mode and mode hooks.

[Carsten Dominik]

Hi Lute,

OK, I see now what the problem is, and I will switch to
define-derived-mode, and to run-mode-hooks (for org-agenda-mode).
This will indeed create a problem if someone uses outline-mode-hook to
modify one of the three outline menus (Headings, Show, Hide).  We will
just have to see if anyone gets biten by this.  I will implement the
change tomorrow - need to do some testing first.

- Carsten



>>>>> "LK" == Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:

LK> Hi Carsten,
>> I remember trying to do define-derived mode, but not doing it for a
>> number of reasons which I do not exactly remember.  The argument was
>> one reason.  Another was that older versions of define-derived-mode
>> did not allow a BODY arguments which I needed.

>> A third one was that org-mode hacks away the Show and Hide menus of
>> outline mode, and I did not want a mode hood to fail because is
>> tries to add entries to non-existing menus.

LK> With my patch, running outline-mode's mode hook (and
LK> after-change-major-mode-hook) is delayed; it is run at the end of
LK> org-mode.  So it introduces the problem that a mode hook will fail
LK> when it tries to change those menus.

>> An important problem always is that I am writing code for Emacs and
>> XEmacs, so my code needs to be compatible with both distributions.

LK> AFAIK, XEmacs has delay-mode-hooks and run-mode-hooks as well.

>> I did not know about delay-mode-hooks and run-mode-hooks - I'll check
>> it out.
>> 
>> What is actually the problem created by the current setup?  What are
>> you trying to do in the outline-mode-hook, and how does it interfer
>> with what org-mode is doing?

LK> My problem is not with outline-mode-hook but with
LK> after-change-major-mode-hook.

LK> In Emacs, global-font-lock-mode (and any other global minor mode
LK> defined with define-global-minor-mode) adds one function to
LK> change-major-mode-hook and a second function to
LK> after-change-major-mode-hook to enable Font Lock mode.  The first
LK> functions adds the current buffer to a list variable.  The second
LK> function enables Font Lock mode for all buffers in that list and
LK> clears the list.

LK> change-major-mode-hook is run by kill-all-local-variables and
LK> after-change-major-mode-hook is run by run-mode-hooks.  So normally, a
LK> major mode calls the first function once as the first thing it does
LK> and the second function once as the last thing it does.  However,
LK> org-mode calls the second function halfway, before it sets up its own
LK> Font-Lock stuff.  As a result Font Lock doesn't work properly for
LK> org-mode.

LK> Lute.


LK> PS.  Putting the second function on after-change-major-mode-hook is a
LK>      recent change, it used to be on find-file-hook.

-- 
Carsten Dominik <dominik@science.uva.nl>        \ _ /
Sterrenkundig Instituut "Anton Pannekoek"        |X|               _
Kruislaan 403; NL-1098 SJ Amsterdam             /| |\   _  _     _/ \
phone +31 (20) 525-7477; FAX +31 (20) 525-7484 __|o|___/ ~~ \___/    ~~~

Re: org-mode and mode hooks.

[Richard Stallman]

This change looks correct to me.  Thanks.

Re: org-mode and mode hooks.

[Luc Teirlinck]

Stefan Monnier wrote:

   > !     (unless (eq font-lock-mode-stored-mode major-mode)
   > !       (setq  font-lock-set-defaults nil))

   As I said earlier, the above "fix up" may break things.  It's OK because at
   that point there's no much else we can do, but we really need to inform the
   user about the problem so she can install a real fix.

The latest version I am going to send soon takes care of that.

Sincerely,

Luc.