unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* First fontification of a buffer happens before font lock is fully initialised.
@ 2012-01-25 12:48 Alan Mackenzie
  2012-01-25 17:46 ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Mackenzie @ 2012-01-25 12:48 UTC (permalink / raw)
  To: emacs-devel; +Cc: Hannu Koivisto

Hi, Emacs.

(define-minor-mode font-lock-mode ....), calls
  font-lock-default-function   which calls
  font-lock-mode-internal      which calls
  font-lock-fontify-buffer.

This call of f-l-fontify-buffer happens before font-lock-mode-hook is
run, since the defined-minor-mode runs the hook as the _very_ last
thing.  Thus the buffer's f-l-stuff is potentially not fully initialised
at this first call of f-l-fontify-buffer.

This situation was found and analysed by Hannu Koivisto after CC Mode
crashed for this reason on a C++ buffer.  Why it doesn't happen for
every CC Mode buffer is not yet clear.

Suggested fix:
(i) Amend define-minor-mode to have an extra parameter :run-hooks with
  default t.
(ii) Set it to nil for f-l-mode, and explicitly write the hook run in
  the appropriate place.

Incidentally, XEmacs does run this hook in the correct place.

-- 
Alan Mackenzie (Nuremberg, Germany.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-25 12:48 First fontification of a buffer happens before font lock is fully initialised Alan Mackenzie
@ 2012-01-25 17:46 ` Stefan Monnier
  2012-01-25 18:26   ` Alan Mackenzie
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2012-01-25 17:46 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

> This situation was found and analysed by Hannu Koivisto after CC Mode
> crashed for this reason on a C++ buffer.  Why it doesn't happen for
> every CC Mode buffer is not yet clear.

I think the behavior depends on the use of font-lock-support-mode.
With jit-lock, fontification takes place after running the mode-hook,
whereas without it, fontification takes place before.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-25 17:46 ` Stefan Monnier
@ 2012-01-25 18:26   ` Alan Mackenzie
  2012-01-25 19:22     ` Hannu Koivisto
  2012-01-26  1:43     ` Stefan Monnier
  0 siblings, 2 replies; 15+ messages in thread
From: Alan Mackenzie @ 2012-01-25 18:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

Hello, Stefan.

On Wed, Jan 25, 2012 at 12:46:04PM -0500, Stefan Monnier wrote:
> > This situation was found and analysed by Hannu Koivisto after CC Mode
> > crashed for this reason on a C++ buffer.  Why it doesn't happen for
> > every CC Mode buffer is not yet clear.

> I think the behavior depends on the use of font-lock-support-mode.
> With jit-lock, fontification takes place after running the mode-hook,
> whereas without it, fontification takes place before.

Surely this cannot be - the hooks are run at the end of the
define-minor-mode expansion no matter what.  Surely the hooks are not
being run twice.  Though I admit I haven't tracked down the stages in
initialising jit-lock-mode.

Hannu's bug report said nothing about disabling jit-lock, so I presume it
was enabled.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-25 18:26   ` Alan Mackenzie
@ 2012-01-25 19:22     ` Hannu Koivisto
  2012-01-25 20:22       ` Alan Mackenzie
  2012-01-26  1:43     ` Stefan Monnier
  1 sibling, 1 reply; 15+ messages in thread
From: Hannu Koivisto @ 2012-01-25 19:22 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Stefan Monnier, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello, Stefan.
>
> On Wed, Jan 25, 2012 at 12:46:04PM -0500, Stefan Monnier wrote:
>> > This situation was found and analysed by Hannu Koivisto after CC Mode
>> > crashed for this reason on a C++ buffer.  Why it doesn't happen for
>> > every CC Mode buffer is not yet clear.

Indeed yesterday, after my initial report, I debugged the problem a
bit more and also noticed that it happens with some files but not
with others.  It seemed that there was asynchronous behaviour at
play that might explain that but I didn't have time to track why
the asynchronity happened with some files but with others
font-lock-mode ended up calling font-lock-fontify-buffer before
running the hooks.

>> I think the behavior depends on the use of font-lock-support-mode.
>> With jit-lock, fontification takes place after running the mode-hook,
>> whereas without it, fontification takes place before.

So do you agree that the code should be changed so that
fontification always happens after running the mode-hook?

> Surely this cannot be - the hooks are run at the end of the
> define-minor-mode expansion no matter what.  Surely the hooks are not
> being run twice.  Though I admit I haven't tracked down the stages in
> initialising jit-lock-mode.
>
> Hannu's bug report said nothing about disabling jit-lock, so I presume it
> was enabled.

I didn't talk about my configuration at all because I was hoping to
eventually reproduce the problem starting with emacs -q if my
observation about what I felt was clearly problematic call chain
didn't lead to a fix.  My configuration uses lazy-lock.  Gladly I
can see from your mail to bug-cc-mode that you have been able to
reproduce the problem with emacs -q and font-lock-support-mode set
to nil with small files.

I'd guess that with lazy-lock it decides to postpone fontification
with some files but not with others.  I can't help but wonder why
the problem doesn't occur with all files when
font-lock-support-mode is set to nil.  Then again, I don't know
what font-lock-support-mode being nil really means, I'm just
assuming that in that case fontification should never be postponed.

-- 
Hannu



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-25 19:22     ` Hannu Koivisto
@ 2012-01-25 20:22       ` Alan Mackenzie
  0 siblings, 0 replies; 15+ messages in thread
From: Alan Mackenzie @ 2012-01-25 20:22 UTC (permalink / raw)
  To: Hannu Koivisto; +Cc: Stefan Monnier, emacs-devel

Hi, Hannu.

On Wed, Jan 25, 2012 at 09:22:52PM +0200, Hannu Koivisto wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > On Wed, Jan 25, 2012 at 12:46:04PM -0500, Stefan Monnier wrote:
> >> > This situation was found and analysed by Hannu Koivisto after CC
> >> > Mode crashed for this reason on a C++ buffer.  Why it doesn't
> >> > happen for every CC Mode buffer is not yet clear.

> Indeed yesterday, after my initial report, I debugged the problem a bit
> more and also noticed that it happens with some files but not with
> others.  It seemed that there was asynchronous behaviour at play that
> might explain that but I didn't have time to track why the asynchronity
> happened with some files but with others font-lock-mode ended up
> calling font-lock-fontify-buffer before running the hooks.

It will only happen on the first file (or a initial sequence of files),
since it is a global variable not being set which triggered the crash.

Looking at lazy-lock.el, there appears to be a minimum size below which
lazy-lock won't be active.  That size is 25,600 by default.  Could it be
that files smaller than this cause the crash, files bigger don't?

> >> I think the behavior depends on the use of font-lock-support-mode.
> >> With jit-lock, fontification takes place after running the mode-hook,
> >> whereas without it, fontification takes place before.

> So do you agree that the code should be changed so that fontification
> always happens after running the mode-hook?

Yes, absolutely.

> > Surely this cannot be - the hooks are run at the end of the
> > define-minor-mode expansion no matter what.  Surely the hooks are not
> > being run twice.  Though I admit I haven't tracked down the stages in
> > initialising jit-lock-mode.

> > Hannu's bug report said nothing about disabling jit-lock, so I presume it
> > was enabled.

> I didn't talk about my configuration at all because I was hoping to
> eventually reproduce the problem starting with emacs -q if my
> observation about what I felt was clearly problematic call chain didn't
> lead to a fix.  My configuration uses lazy-lock.  Gladly I can see from
> your mail to bug-cc-mode that you have been able to reproduce the
> problem with emacs -q and font-lock-support-mode set to nil with small
> files.

> I'd guess that with lazy-lock it decides to postpone fontification
> with some files but not with others.  I can't help but wonder why
> the problem doesn't occur with all files when
> font-lock-support-mode is set to nil.  Then again, I don't know
> what font-lock-support-mode being nil really means, I'm just
> assuming that in that case fontification should never be postponed.

Pretty much.

> -- 
> Hannu

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-25 18:26   ` Alan Mackenzie
  2012-01-25 19:22     ` Hannu Koivisto
@ 2012-01-26  1:43     ` Stefan Monnier
  2012-01-26 10:58       ` Alan Mackenzie
  2012-02-04 12:03       ` Alan Mackenzie
  1 sibling, 2 replies; 15+ messages in thread
From: Stefan Monnier @ 2012-01-26  1:43 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

>> > This situation was found and analysed by Hannu Koivisto after CC Mode
>> > crashed for this reason on a C++ buffer.  Why it doesn't happen for
>> > every CC Mode buffer is not yet clear.
>> I think the behavior depends on the use of font-lock-support-mode.
>> With jit-lock, fontification takes place after running the mode-hook,
>> whereas without it, fontification takes place before.
> Surely this cannot be

I'm pretty sure this is the source of the difference.

> the hooks are run at the end of the define-minor-mode expansion no
> matter what.

Yes, but fontification can take place even later, depending on
font-lock-support-mode.

> Surely the hooks are not being run twice.

No, indeed, they should only be run once.

> Though I admit I haven't tracked down the stages in initialising
> jit-lock-mode.

jit-lock postpones fontification to whenever the text is actually
displayed, i.e. much later than initialization; it may very well never
fontify any part of the text at all, if the buffer is never displayed.

> So do you agree that the code should be changed so that
> fontification always happens after running the mode-hook?

Yes, I agree.  This was the behavior in Emacs-20, and is also the
behavior with jit-lock (i.e. the default behavior).  I'm not sure what's
the best way to get that result, tho: adding yet-another keyword to
define-minor-mode is something I'd rather avoid.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-26  1:43     ` Stefan Monnier
@ 2012-01-26 10:58       ` Alan Mackenzie
  2012-02-04 12:03       ` Alan Mackenzie
  1 sibling, 0 replies; 15+ messages in thread
From: Alan Mackenzie @ 2012-01-26 10:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

On Wed, Jan 25, 2012 at 08:43:48PM -0500, Stefan Monnier wrote:

> jit-lock postpones fontification to whenever the text is actually
> displayed, i.e. much later than initialization; it may very well never
> fontify any part of the text at all, if the buffer is never displayed.

Ah, OK.

> > So do you agree that the code should be changed so that
> > fontification always happens after running the mode-hook?

> Yes, I agree.  This was the behavior in Emacs-20, and is also the
> behavior with jit-lock (i.e. the default behavior).  I'm not sure what's
> the best way to get that result, tho: adding yet-another keyword to
> define-minor-mode is something I'd rather avoid.

I can understand that.  How about writing `font-lock-mode' explicitly
rather than using `define-minor-mode'?  The explicit fontification of the
whole buffer could be removed from jit-lock (?and lazy-lock) since it
will be redundant, and possibly because that explicit fontification might
somehow depend on something in font-lock-mode-hook.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-01-26  1:43     ` Stefan Monnier
  2012-01-26 10:58       ` Alan Mackenzie
@ 2012-02-04 12:03       ` Alan Mackenzie
  2012-02-06  1:39         ` Stefan Monnier
  1 sibling, 1 reply; 15+ messages in thread
From: Alan Mackenzie @ 2012-02-04 12:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

Hello, Stefan

On Wed, Jan 25, 2012 at 08:43:48PM -0500, Stefan Monnier wrote:
> >> > This situation was found and analysed by Hannu Koivisto after CC Mode
> >> > crashed for this reason on a C++ buffer.  Why it doesn't happen for
> >> > every CC Mode buffer is not yet clear.
> >> I think the behavior depends on the use of font-lock-support-mode.
> >> With jit-lock, fontification takes place after running the mode-hook,
> >> whereas without it, fontification takes place before.
> > Surely this cannot be

> I'm pretty sure this is the source of the difference.

> > the hooks are run at the end of the define-minor-mode expansion no
> > matter what.

> Yes, but fontification can take place even later, depending on
> font-lock-support-mode.

> > Surely the hooks are not being run twice.

> No, indeed, they should only be run once.

> > Though I admit I haven't tracked down the stages in initialising
> > jit-lock-mode.

> jit-lock postpones fontification to whenever the text is actually
> displayed, i.e. much later than initialization; it may very well never
> fontify any part of the text at all, if the buffer is never displayed.

> > So do you agree that the code should be changed so that
> > fontification always happens after running the mode-hook?

> Yes, I agree.  This was the behavior in Emacs-20, and is also the
> behavior with jit-lock (i.e. the default behavior).  I'm not sure what's
> the best way to get that result, tho: adding yet-another keyword to
> define-minor-mode is something I'd rather avoid.

I can only see one other way to deal with this: that is to write
`font-lock-mode' explicitly; or rather, expand the macro by hand, tidy it
up, and move the `run-hooks'.

I've done this in the patch below.  To see the difference between before
and after, do

emacs -Q
M-: (setq font-lock-support-mode nil)
C-c C-f random.c

Before the change, an error is thrown.  After the change, it works.
Should I commit this patch?



=== modified file 'lisp/font-core.el'
*** lisp/font-core.el	2012-01-19 07:21:25 +0000
--- lisp/font-core.el	2012-02-04 11:59:45 +0000
***************
*** 87,93 ****
  ;; The mode for which font-lock was initialized, or nil if none.
  (defvar font-lock-major-mode)
  
! (define-minor-mode font-lock-mode
    "Toggle syntax highlighting in this buffer (Font Lock mode).
  With a prefix argument ARG, enable Font Lock mode if ARG is
  positive, and disable it otherwise.  If called from Lisp, enable
--- 87,100 ----
  ;; The mode for which font-lock was initialized, or nil if none.
  (defvar font-lock-major-mode)
  
! (defvar font-lock-mode nil
!   "Non-nil if Font-Lock mode is enabled.
! Use the command `font-lock-mode' to change this variable.")
! (make-variable-buffer-local 'font-lock-mode)
! 
! ;; Note: `define-minor-mode' cannot be used here, since the mode hooks
! ;; need to be run before the first fontification of a buffer.
! (defun font-lock-mode (&optional arg)
    "Toggle syntax highlighting in this buffer (Font Lock mode).
  With a prefix argument ARG, enable Font Lock mode if ARG is
  positive, and disable it otherwise.  If called from Lisp, enable
***************
*** 137,152 ****
  The above is the default behavior of `font-lock-mode'; you may specify
  your own function which is called when `font-lock-mode' is toggled via
  `font-lock-function'. "
!   nil nil nil
!   ;; Don't turn on Font Lock mode if we don't have a display (we're running a
!   ;; batch job) or if the buffer is invisible (the name starts with a space).
!   (when (or noninteractive (eq (aref (buffer-name) 0) ?\s))
!     (setq font-lock-mode nil))
!   (funcall font-lock-function font-lock-mode)
!   ;; 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.
--- 144,185 ----
  The above is the default behavior of `font-lock-mode'; you may specify
  your own function which is called when `font-lock-mode' is toggled via
  `font-lock-function'. "
!   (interactive (list (or current-prefix-arg 'toggle)))
!   (let ((last-message (current-message)))
!     (setq font-lock-mode
! 	  (cond ((eq arg 'toggle)
! 		 (not font-lock-mode))
! 		(arg (> (prefix-numeric-value arg) 0))
! 		(t (if (null font-lock-mode)
! 		       t
! 		     (message "Toggling %s off; better pass an explicit argument."
! 			      'font-lock-mode)
! 		     nil))))
! 
!     (when (or noninteractive (eq (aref (buffer-name) 0) 32))
!       (setq font-lock-mode nil))
! 
!     (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))
!     (run-hooks 'font-lock-mode-hook
! 	       (if font-lock-mode
! 		   'font-lock-mode-on-hook
! 		 'font-lock-mode-off-hook))
!     (funcall font-lock-function font-lock-mode)
!    (if (called-interactively-p 'any)
! 	(progn nil
! 	       (unless (and (current-message)
! 			    (not (equal last-message (current-message))))
! 		 (message "Font-Lock mode %sabled"
! 			  (if font-lock-mode "en" "dis"))))))
! 
!   (force-mode-line-update) font-lock-mode)
! 
! :autoload-end
! (add-minor-mode 'font-lock-mode nil
! 		(if (boundp 'font-lock-mode-map)
! 		    font-lock-mode-map))
  
  ;; Get rid of fontification for the old major mode.
  ;; We do this when changing major modes.



>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-04 12:03       ` Alan Mackenzie
@ 2012-02-06  1:39         ` Stefan Monnier
  2012-02-07 10:10           ` Alan Mackenzie
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2012-02-06  1:39 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

>> Yes, I agree.  This was the behavior in Emacs-20, and is also the
>> behavior with jit-lock (i.e. the default behavior).  I'm not sure what's
>> the best way to get that result, tho: adding yet-another keyword to
>> define-minor-mode is something I'd rather avoid.
> I can only see one other way to deal with this: that is to write
> `font-lock-mode' explicitly; or rather, expand the macro by hand, tidy it
> up, and move the `run-hooks'.

Moving away from define-minor-mode is also something I'd rather avoid
(so we don't have to update font-lock-mode every time we change
define-minor-mode).
Some other thoughts:
- use some trick to delay the fontification to after the minor-mode hook
  is run (e.g. add to post-command-hook or something like that).
  post-command-hook is not a good choice and I can't think of much
  better, so maybe this is out.
- maybe it's OK to add yet another argument to define-minor-mode.
  Note that an arg that says "turn off this standard thingy" (as
  in :no-hook) is out of the question, since a large part of the reason
  why we have define-minor-mode is to try and auto-enforce the coding
  conventions of minor modes.  But maybe we can have an :after-hook.
  Some major modes would also appreciate such a feature, so maybe this
  same arg can be added to both define-minor-mode and
  define-derived-mode.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-06  1:39         ` Stefan Monnier
@ 2012-02-07 10:10           ` Alan Mackenzie
  2012-02-07 18:02             ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Mackenzie @ 2012-02-07 10:10 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

Hello, Stefan.

On Sun, Feb 05, 2012 at 08:39:00PM -0500, Stefan Monnier wrote:
> >> Yes, I agree.  This was the behavior in Emacs-20, and is also the
> >> behavior with jit-lock (i.e. the default behavior).  I'm not sure what's
> >> the best way to get that result, tho: adding yet-another keyword to
> >> define-minor-mode is something I'd rather avoid.
> > I can only see one other way to deal with this: that is to write
> > `font-lock-mode' explicitly; or rather, expand the macro by hand, tidy it
> > up, and move the `run-hooks'.

> Moving away from define-minor-mode is also something I'd rather avoid
> (so we don't have to update font-lock-mode every time we change
> define-minor-mode).

OK.

> Some other thoughts:
> - use some trick to delay the fontification to after the minor-mode hook
>   is run (e.g. add to post-command-hook or something like that).
>   post-command-hook is not a good choice and I can't think of much
>   better, so maybe this is out.

post-command-hook _is_ ugly, and we don't want one-time code running in
it.

> - maybe it's OK to add yet another argument to define-minor-mode.

define-minor-mode is insufficiently general, so something has to give.

>   Note that an arg that says "turn off this standard thingy" (as
>   in :no-hook) is out of the question, since a large part of the reason
>   why we have define-minor-mode is to try and auto-enforce the coding
>   conventions of minor modes.

Can we not trust the good taste and sense of hackers generally?  We can
certainly trust the Emacs developers.

>   But maybe we can have an :after-hook.  Some major modes would also
>   appreciate such a feature, so maybe this same arg can be added to
>   both define-minor-mode and define-derived-mode.

Or, perhaps, even (defmacro run-hooks-here () ...), which would preserve
the order of the initialisation forms in the mode.  But I'd be happy
enough with :after-hook.  Would you like me to implement it?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-07 10:10           ` Alan Mackenzie
@ 2012-02-07 18:02             ` Stefan Monnier
  2012-02-08 21:43               ` Alan Mackenzie
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2012-02-07 18:02 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

>> Some other thoughts:
>> - use some trick to delay the fontification to after the minor-mode hook
>> is run (e.g. add to post-command-hook or something like that).
>> post-command-hook is not a good choice and I can't think of much
>> better, so maybe this is out.
> post-command-hook _is_ ugly, and we don't want one-time code running
> in it.

post-command-hook is often used for "one-time code", actually (e.g. for
define-globalized-minor-mode).

> Can we not trust the good taste and sense of hackers generally?

Have you ever looked at the Elisp code out there ;-)?

> We can certainly trust the Emacs developers.

I certainly wouldn't.  Especially not the maintainers.

>> But maybe we can have an :after-hook.  Some major modes would also
>> appreciate such a feature, so maybe this same arg can be added to
>> both define-minor-mode and define-derived-mode.
> Or, perhaps, even (defmacro run-hooks-here () ...), which would preserve
> the order of the initialisation forms in the mode.  But I'd be happy
> enough with :after-hook.  Would you like me to implement it?

Ah, yes, some way to force "run it here rather than elsewhere" would
be elegant.  The problem, is how to implement it in a way that's not
completely disgusting.
The :after-hook has the advantage of being straightforward.

If we want to fix it for 24.1, "straightforward" is important.

Patch welcome, either way,


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-07 18:02             ` Stefan Monnier
@ 2012-02-08 21:43               ` Alan Mackenzie
  2012-02-10 14:27                 ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Mackenzie @ 2012-02-08 21:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

Hello, Stefan.

On Tue, Feb 07, 2012 at 01:02:07PM -0500, Stefan Monnier wrote:

> > Can we not trust the good taste and sense of hackers generally?

> Have you ever looked at the Elisp code out there ;-)?

_Looked_ at it?  ;-)

> > We can certainly trust the Emacs developers.

> I certainly wouldn't.  Especially not the maintainers.

Hey, thanks!!  :-)

> >> But maybe we can have an :after-hook.  Some major modes would also
> >> appreciate such a feature, so maybe this same arg can be added to
> >> both define-minor-mode and define-derived-mode.
> > Or, perhaps, even (defmacro run-hooks-here () ...), which would preserve
> > the order of the initialisation forms in the mode.  But I'd be happy
> > enough with :after-hook.  Would you like me to implement it?

> Ah, yes, some way to force "run it here rather than elsewhere" would
> be elegant.  The problem, is how to implement it in a way that's not
> completely disgusting.

Having tried this this afternoon, I don't see how it can be done
elegantly this way.

> The :after-hook has the advantage of being straightforward.

> If we want to fix it for 24.1, "straightforward" is important.

> Patch welcome, either way,

OK, Here goes:


=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el	2012-02-07 08:26:54 +0000
--- lisp/emacs-lisp/easy-mmode.el	2012-02-08 21:22:45 +0000
***************
*** 135,140 ****
--- 135,142 ----
  		the new state, and sets it.  If you specify a :variable,
  		this function does not define a MODE variable (nor any of
  		the terms used in :variable).
+ :after-hooks    A single lisp form which is evaluated after the mode hooks
+                 have been run.  It should be quoted.
  
  For example, you could write
    (define-minor-mode foo-mode \"If enabled, foo on you!\"
***************
*** 170,175 ****
--- 172,178 ----
           (setter nil)            ;The function (if any) to set the mode var.
           (modefun mode)          ;The minor mode function name we're defining.
  	 (require t)
+ 	 (after-hooks nil)
  	 (hook (intern (concat mode-name "-hook")))
  	 (hook-on (intern (concat mode-name "-on-hook")))
  	 (hook-off (intern (concat mode-name "-off-hook")))
***************
*** 197,202 ****
--- 200,206 ----
               (setq mode variable)
             (setq mode (car variable))
             (setq setter (cdr variable))))
+ 	(:after-hooks (setq after-hooks (pop body)))
  	(t (push keyw extra-keywords) (push (pop body) extra-keywords))))
  
      (setq keymap-sym (if (and keymap (symbolp keymap)) keymap
***************
*** 265,271 ****
             ,@body
             ;; The on/off hooks are here for backward compatibility only.
             (run-hooks ',hook (if ,mode ',hook-on ',hook-off))
!            (if (called-interactively-p 'any)
                 (progn
                   ,(if (and globalp (symbolp mode))
                        `(customize-mark-as-set ',mode))
--- 269,276 ----
             ,@body
             ;; The on/off hooks are here for backward compatibility only.
             (run-hooks ',hook (if ,mode ',hook-on ',hook-off))
! 	   ,@(when after-hooks `(,(eval after-hooks)))
! 	   (if (called-interactively-p 'any)
                 (progn
                   ,(if (and globalp (symbolp mode))
                        `(customize-mark-as-set ',mode))


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-08 21:43               ` Alan Mackenzie
@ 2012-02-10 14:27                 ` Stefan Monnier
  2012-02-23 11:39                   ` Alan Mackenzie
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2012-02-10 14:27 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

> + :after-hooks    A single lisp form which is evaluated after the mode hooks
> +                 have been run.  It should be quoted.

Don't quote it.

> ! 	   ,@(when after-hooks `(,(eval after-hooks)))

And don't call `eval'.
Also, I'd move the after-hooks code to after the customize-mark-as-set&message.


        Stefan



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-10 14:27                 ` Stefan Monnier
@ 2012-02-23 11:39                   ` Alan Mackenzie
  2012-02-23 14:34                     ` Stefan Monnier
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Mackenzie @ 2012-02-23 11:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Hannu Koivisto, emacs-devel

Hello, Stefan.

[Quick summary: adding the new parameter ":after-hook" to
define-minor-mode, and adapting font-core/font-lock to use it.]

On Fri, Feb 10, 2012 at 09:27:20AM -0500, Stefan Monnier wrote:
> > + :after-hooks    A single lisp form which is evaluated after the mode hooks
> > +                 have been run.  It should be quoted.

> Don't quote it.

OK

> > ! 	   ,@(when after-hooks `(,(eval after-hooks)))

> And don't call `eval'.

OK2.

> Also, I'd move the after-hooks code to after the customize-mark-as-set&message.

OK3.

Here's an amended patch for easy-mmode.el and a patch for font-core/lock
which uses the :after-hook param.  Should I install it?



=== modified file 'doc/lispref/modes.texi'
*** doc/lispref/modes.texi	2012-02-19 05:54:33 +0000
--- doc/lispref/modes.texi	2012-02-22 20:51:42 +0000
***************
*** 1594,1608 ****
  @var{place} can also be a cons @code{(@var{get} . @var{set})},
  where @var{get} is an expression that returns the current state,
  and @var{set} is a function of one argument (a state) that sets it.
  @end table
  
  Any other keyword arguments are passed directly to the
  @code{defcustom} generated for the variable @var{mode}.
  
! The command named @var{mode} first performs the standard actions such
! as setting the variable named @var{mode} and then executes the
! @var{body} forms, if any.  It finishes by running the mode hook
! variable @code{@var{mode}-hook}.
  @end defmac
  
    The initial value must be @code{nil} except in cases where (1) the
--- 1594,1613 ----
  @var{place} can also be a cons @code{(@var{get} . @var{set})},
  where @var{get} is an expression that returns the current state,
  and @var{set} is a function of one argument (a state) that sets it.
+ 
+ @item :after-hook @var{after-hook}
+ This defines a single lisp form which is evaluated after the mode hooks
+ have run.  It should not be quoted.
  @end table
  
  Any other keyword arguments are passed directly to the
  @code{defcustom} generated for the variable @var{mode}.
  
! The command named @var{mode} first performs the standard actions such as
! setting the variable named @var{mode} and then executes the @var{body}
! forms, if any.  It then runs the mode hook variable
! @code{@var{mode}-hook} and finishes by evaluating any form in
! @code{:after-hook}.
  @end defmac
  
    The initial value must be @code{nil} except in cases where (1) the

=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el	2012-02-07 08:26:54 +0000
--- lisp/emacs-lisp/easy-mmode.el	2012-02-22 20:11:14 +0000
***************
*** 135,140 ****
--- 135,142 ----
  		the new state, and sets it.  If you specify a :variable,
  		this function does not define a MODE variable (nor any of
  		the terms used in :variable).
+ :after-hook     A single lisp form which is evaluated after the mode hooks
+                 have been run.  It should not be quoted.
  
  For example, you could write
    (define-minor-mode foo-mode \"If enabled, foo on you!\"
***************
*** 170,175 ****
--- 172,178 ----
           (setter nil)            ;The function (if any) to set the mode var.
           (modefun mode)          ;The minor mode function name we're defining.
  	 (require t)
+ 	 (after-hook nil)
  	 (hook (intern (concat mode-name "-hook")))
  	 (hook-on (intern (concat mode-name "-on-hook")))
  	 (hook-off (intern (concat mode-name "-off-hook")))
***************
*** 197,202 ****
--- 200,206 ----
               (setq mode variable)
             (setq mode (car variable))
             (setq setter (cdr variable))))
+ 	(:after-hook (setq after-hook (pop body)))
  	(t (push keyw extra-keywords) (push (pop body) extra-keywords))))
  
      (setq keymap-sym (if (and keymap (symbolp keymap)) keymap
***************
*** 275,281 ****
                                (not (equal ,last-message
                                            (current-message))))
                     (message ,(format "%s %%sabled" pretty-name)
!                             (if ,mode "en" "dis"))))))
  	 (force-mode-line-update)
  	 ;; Return the new setting.
  	 ,mode)
--- 279,286 ----
                                (not (equal ,last-message
                                            (current-message))))
                     (message ,(format "%s %%sabled" pretty-name)
!                             (if ,mode "en" "dis")))))
! 	   ,@(when after-hook `(,after-hook)))
  	 (force-mode-line-update)
  	 ;; Return the new setting.
  	 ,mode)

=== modified file 'lisp/font-core.el'
*** lisp/font-core.el	2012-01-19 07:21:25 +0000
--- lisp/font-core.el	2012-02-23 11:10:32 +0000
***************
*** 138,143 ****
--- 138,144 ----
  your own function which is called when `font-lock-mode' is toggled via
  `font-lock-function'. "
    nil nil nil
+   :after-hook (if font-lock-mode (font-lock-initial-fontify))
    ;; Don't turn on Font Lock mode if we don't have a display (we're running a
    ;; batch job) or if the buffer is invisible (the name starts with a space).
    (when (or noninteractive (eq (aref (buffer-name) 0) ?\s))

=== modified file 'lisp/font-lock.el'
*** lisp/font-lock.el	2012-02-10 15:59:29 +0000
--- lisp/font-lock.el	2012-02-23 11:15:14 +0000
***************
*** 629,649 ****
    ;; Shut up the byte compiler.
    (defvar font-lock-face-attributes))	; Obsolete but respected if set.
  
  (defun font-lock-mode-internal (arg)
    ;; Turn on Font Lock mode.
    (when arg
      (add-hook 'after-change-functions 'font-lock-after-change-function t t)
      (font-lock-set-defaults)
!     (font-lock-turn-on-thing-lock)
!     ;; Fontify the buffer if we have to.
!     (let ((max-size (font-lock-value-in-major-mode font-lock-maximum-size)))
!       (cond (font-lock-fontified
! 	     nil)
! 	    ((or (null max-size) (> max-size (buffer-size)))
! 	     (font-lock-fontify-buffer))
! 	    (font-lock-verbose
! 	     (message "Fontifying %s...buffer size greater than font-lock-maximum-size"
! 		      (buffer-name))))))
    ;; Turn off Font Lock mode.
    (unless font-lock-mode
      (remove-hook 'after-change-functions 'font-lock-after-change-function t)
--- 629,652 ----
    ;; Shut up the byte compiler.
    (defvar font-lock-face-attributes))	; Obsolete but respected if set.
  
+ (defun font-lock-initial-fontify ()
+   ;; The first fontification after turning the mode on.  This must
+   ;;  only be called after the mode hooks have been run.
+   (let ((max-size (font-lock-value-in-major-mode font-lock-maximum-size)))
+     (cond (font-lock-fontified
+ 	   nil)
+ 	  ((or (null max-size) (> max-size (buffer-size)))
+ 	   (font-lock-fontify-buffer))
+ 	  (font-lock-verbose
+ 	   (message "Fontifying %s...buffer size greater than font-lock-maximum-size"
+ 		    (buffer-name))))))
+ 
  (defun font-lock-mode-internal (arg)
    ;; Turn on Font Lock mode.
    (when arg
      (add-hook 'after-change-functions 'font-lock-after-change-function t t)
      (font-lock-set-defaults)
!     (font-lock-turn-on-thing-lock))
    ;; Turn off Font Lock mode.
    (unless font-lock-mode
      (remove-hook 'after-change-functions 'font-lock-after-change-function t)



>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: First fontification of a buffer happens before font lock is fully initialised.
  2012-02-23 11:39                   ` Alan Mackenzie
@ 2012-02-23 14:34                     ` Stefan Monnier
  0 siblings, 0 replies; 15+ messages in thread
From: Stefan Monnier @ 2012-02-23 14:34 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Hannu Koivisto, emacs-devel

> Here's an amended patch for easy-mmode.el and a patch for font-core/lock
> which uses the :after-hook param.  Should I install it?

Looks good to me, yes,
Note that none of the :keywords need to be quoted, which is to be
expected since define-minor-mode is a macro, not a function, so
I wouldn't mention "It should not be quoted".  But if you find it
useful, by all means keep it.


        Stefan


> === modified file 'doc/lispref/modes.texi'
> *** doc/lispref/modes.texi	2012-02-19 05:54:33 +0000
> --- doc/lispref/modes.texi	2012-02-22 20:51:42 +0000
> ***************
> *** 1594,1608 ****
>   @var{place} can also be a cons @code{(@var{get} . @var{set})},
>   where @var{get} is an expression that returns the current state,
>   and @var{set} is a function of one argument (a state) that sets it.
>   @end table
  
>   Any other keyword arguments are passed directly to the
>   @code{defcustom} generated for the variable @var{mode}.
  
> ! The command named @var{mode} first performs the standard actions such
> ! as setting the variable named @var{mode} and then executes the
> ! @var{body} forms, if any.  It finishes by running the mode hook
> ! variable @code{@var{mode}-hook}.
>   @end defmac
  
>     The initial value must be @code{nil} except in cases where (1) the
> --- 1594,1613 ----
>   @var{place} can also be a cons @code{(@var{get} . @var{set})},
>   where @var{get} is an expression that returns the current state,
>   and @var{set} is a function of one argument (a state) that sets it.
> + 
> + @item :after-hook @var{after-hook}
> + This defines a single lisp form which is evaluated after the mode hooks
> + have run.  It should not be quoted.
>   @end table
  
>   Any other keyword arguments are passed directly to the
>   @code{defcustom} generated for the variable @var{mode}.
  
> ! The command named @var{mode} first performs the standard actions such as
> ! setting the variable named @var{mode} and then executes the @var{body}
> ! forms, if any.  It then runs the mode hook variable
> ! @code{@var{mode}-hook} and finishes by evaluating any form in
> ! @code{:after-hook}.
>   @end defmac
  
>     The initial value must be @code{nil} except in cases where (1) the

> === modified file 'lisp/emacs-lisp/easy-mmode.el'
> *** lisp/emacs-lisp/easy-mmode.el	2012-02-07 08:26:54 +0000
> --- lisp/emacs-lisp/easy-mmode.el	2012-02-22 20:11:14 +0000
> ***************
> *** 135,140 ****
> --- 135,142 ----
>   		the new state, and sets it.  If you specify a :variable,
>   		this function does not define a MODE variable (nor any of
>   		the terms used in :variable).
> + :after-hook     A single lisp form which is evaluated after the mode hooks
> +                 have been run.  It should not be quoted.
  
>   For example, you could write
>     (define-minor-mode foo-mode \"If enabled, foo on you!\"
> ***************
> *** 170,175 ****
> --- 172,178 ----
>            (setter nil)            ;The function (if any) to set the mode var.
>            (modefun mode)          ;The minor mode function name we're defining.
>   	 (require t)
> + 	 (after-hook nil)
>   	 (hook (intern (concat mode-name "-hook")))
>   	 (hook-on (intern (concat mode-name "-on-hook")))
>   	 (hook-off (intern (concat mode-name "-off-hook")))
> ***************
> *** 197,202 ****
> --- 200,206 ----
>                (setq mode variable)
>              (setq mode (car variable))
>              (setq setter (cdr variable))))
> + 	(:after-hook (setq after-hook (pop body)))
>   	(t (push keyw extra-keywords) (push (pop body) extra-keywords))))
  
>       (setq keymap-sym (if (and keymap (symbolp keymap)) keymap
> ***************
> *** 275,281 ****
>                                 (not (equal ,last-message
>                                             (current-message))))
>                      (message ,(format "%s %%sabled" pretty-name)
> !                             (if ,mode "en" "dis"))))))
>   	 (force-mode-line-update)
>   	 ;; Return the new setting.
>   	 ,mode)
> --- 279,286 ----
>                                 (not (equal ,last-message
>                                             (current-message))))
>                      (message ,(format "%s %%sabled" pretty-name)
> !                             (if ,mode "en" "dis")))))
> ! 	   ,@(when after-hook `(,after-hook)))
>   	 (force-mode-line-update)
>   	 ;; Return the new setting.
>   	 ,mode)

> === modified file 'lisp/font-core.el'
> *** lisp/font-core.el	2012-01-19 07:21:25 +0000
> --- lisp/font-core.el	2012-02-23 11:10:32 +0000
> ***************
> *** 138,143 ****
> --- 138,144 ----
>   your own function which is called when `font-lock-mode' is toggled via
>   `font-lock-function'. "
>     nil nil nil
> +   :after-hook (if font-lock-mode (font-lock-initial-fontify))
>     ;; Don't turn on Font Lock mode if we don't have a display (we're running a
>     ;; batch job) or if the buffer is invisible (the name starts with a space).
>     (when (or noninteractive (eq (aref (buffer-name) 0) ?\s))

> === modified file 'lisp/font-lock.el'
> *** lisp/font-lock.el	2012-02-10 15:59:29 +0000
> --- lisp/font-lock.el	2012-02-23 11:15:14 +0000
> ***************
> *** 629,649 ****
>     ;; Shut up the byte compiler.
>     (defvar font-lock-face-attributes))	; Obsolete but respected if set.
  
>   (defun font-lock-mode-internal (arg)
>     ;; Turn on Font Lock mode.
>     (when arg
>       (add-hook 'after-change-functions 'font-lock-after-change-function t t)
>       (font-lock-set-defaults)
> !     (font-lock-turn-on-thing-lock)
> !     ;; Fontify the buffer if we have to.
> !     (let ((max-size (font-lock-value-in-major-mode font-lock-maximum-size)))
> !       (cond (font-lock-fontified
> ! 	     nil)
> ! 	    ((or (null max-size) (> max-size (buffer-size)))
> ! 	     (font-lock-fontify-buffer))
> ! 	    (font-lock-verbose
> ! 	     (message "Fontifying %s...buffer size greater than font-lock-maximum-size"
> ! 		      (buffer-name))))))
>     ;; Turn off Font Lock mode.
>     (unless font-lock-mode
>       (remove-hook 'after-change-functions 'font-lock-after-change-function t)
> --- 629,652 ----
>     ;; Shut up the byte compiler.
>     (defvar font-lock-face-attributes))	; Obsolete but respected if set.
  
> + (defun font-lock-initial-fontify ()
> +   ;; The first fontification after turning the mode on.  This must
> +   ;;  only be called after the mode hooks have been run.
> +   (let ((max-size (font-lock-value-in-major-mode font-lock-maximum-size)))
> +     (cond (font-lock-fontified
> + 	   nil)
> + 	  ((or (null max-size) (> max-size (buffer-size)))
> + 	   (font-lock-fontify-buffer))
> + 	  (font-lock-verbose
> + 	   (message "Fontifying %s...buffer size greater than font-lock-maximum-size"
> + 		    (buffer-name))))))
> + 
>   (defun font-lock-mode-internal (arg)
>     ;; Turn on Font Lock mode.
>     (when arg
>       (add-hook 'after-change-functions 'font-lock-after-change-function t t)
>       (font-lock-set-defaults)
> !     (font-lock-turn-on-thing-lock))
>     ;; Turn off Font Lock mode.
>     (unless font-lock-mode
>       (remove-hook 'after-change-functions 'font-lock-after-change-function t)



>> Stefan

> -- 
> Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-02-23 14:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-25 12:48 First fontification of a buffer happens before font lock is fully initialised Alan Mackenzie
2012-01-25 17:46 ` Stefan Monnier
2012-01-25 18:26   ` Alan Mackenzie
2012-01-25 19:22     ` Hannu Koivisto
2012-01-25 20:22       ` Alan Mackenzie
2012-01-26  1:43     ` Stefan Monnier
2012-01-26 10:58       ` Alan Mackenzie
2012-02-04 12:03       ` Alan Mackenzie
2012-02-06  1:39         ` Stefan Monnier
2012-02-07 10:10           ` Alan Mackenzie
2012-02-07 18:02             ` Stefan Monnier
2012-02-08 21:43               ` Alan Mackenzie
2012-02-10 14:27                 ` Stefan Monnier
2012-02-23 11:39                   ` Alan Mackenzie
2012-02-23 14:34                     ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).