Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

[Stefan Monnier]

>     * cc-mode.el (c-fl-decl-start): Renamed from c-set-fl-decl-start.

Please use the present tense, even for "rename" ;-)

>     (c-extend-after-change-region): Explicitly apply 'fontified properties
>     to the extended bits of the font lock region.

Sounds like this is working around a problem elsewhere, so better add
a comment explaining what's going on.


        Stefan

Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

[Alan Mackenzie]

Hello, Stefan.

On Mon, Mar 30, 2015 at 05:08:38PM -0400, Stefan Monnier wrote:
> >     * cc-mode.el (c-fl-decl-start): Renamed from c-set-fl-decl-start.

> Please use the present tense, even for "rename" ;-)

Do you mean I should change the commit message on that commit?  Can I
actually do that?

> >     (c-extend-after-change-region): Explicitly apply 'fontified properties
> >     to the extended bits of the font lock region.

> Sounds like this is working around a problem elsewhere, so better add
> a comment explaining what's going on.

jit-lock-force-redisplay (that function which is set to run on the
expiry of a 0 second timer, when jit-lock-fontify-now suspects that some
area which has already been rendered requires to be re-rendered) doesn't
seem to work.  At least, I couldn't get it to work.

Its source is this:

1. (defun jit-lock-force-redisplay (start end)
2.   "Force the display engine to re-render START's buffer from START to END.
3. This applies to the buffer associated with marker START."
4.   (when (marker-buffer start)
5.     (with-current-buffer (marker-buffer start)
6.       (with-buffer-prepared-for-jit-lock
7.        (when (> end (point-max))
8.          (setq end (point-max) start (min start end)))
9.        (when (< start (point-min))
10.         (setq start (point-min) end (max start end)))
11.       ;; Don't cause refontification (it's already been done), but just do
12.       ;; some random buffer change, so as to force redisplay.
13.       (put-text-property start end 'fontified t)))))

The exact locations `start' and `end' used in L13 appear to be a red
herring - when I tried (artifically) subtracting 100 from start, to
ensure (start end) covered the location where re-rendering was needed,
the re-rendering still failed to take place.

However, when I tried changing L13 to set 'fontified to nil, the
re-rendering happened even when the location was far before `start'.

I don't think making this non-change change to the buffer is sufficient to
trigger a redisplay, though I was unable to get deeply enough into
xdisp.c to verify this.

*************************************************************************

Assuming that jit-lock-force-redisplay is not going to work, it follows
that any buffer bits to be fontified which come earlier than
after-change-functions's `beg' need to have their 'fontified text
property set to nil, and this needs to happen at after-change time
(during redisplay is too late).  This necessity even applies to
extending change regions to whole lines.

The only functions which can do this extending are those on
jit-lock-after-change-extend-region-functions, i.e. the single function
font-lock-extend-jit-lock-region-after-change.  By the way, I am of the
opinion that the extending of regions to whole lines in
f-l-extend-j-l-r-after-change is not currently an optimisation, but a
crucial part of the functioning of jit-lock.  Without it, after a buffer
change in the middle of a line, the beginning of that line doesn't get
rendered (at least, not until context fontification kicks in).  This was
what Daniel C. saw and complained about.  You recently suggested doing
away with f-l-e-j-l-r-a-c.  I don't think you can do this without
putting something similar, running at after-change time, in its place.

f-l-extend-j-l-r-after-change has a recently fixed bug, where the
extended region is only returned to the caller in certain circumstances.
Those circumstances no longer hold in CC Mode.  To allow CC Mode to
continue to work in already released Emacs versions, it is necessary to
set the 'fontified text-property to nil from
c-extend-after-change-region.

All this would make a rather long comment in lisp/ChangeLog.  ;-(

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

[Stefan Monnier]

>> Please use the present tense, even for "rename" ;-)
> Do you mean I should change the commit message on that commit?  Can I
> actually do that?

No, what's done is done.

> ensure (start end) covered the location where re-rendering was needed,
> the re-rendering still failed to take place.

How can you tell.  What does "re-render" mean for you?
`jit-lock-force-redisplay' is not meant to re-run font-lock.  It is only
meant to refresh the display so that it reflects the `face' properties
that were already set by an earlier jit-lock run.

> However, when I tried changing L13 to set 'fontified to nil, the
> re-rendering happened even when the location was far before `start'.

Setting it to nil will cause that forces redisplay to not only re-render
the buffer but also re-fontify (i.e. re-run font-lock).  This is a very
different need from the one jit-lock-force-redisplay is designed to address.

> Assuming that jit-lock-force-redisplay is not going to work, it follows
> that any buffer bits to be fontified which come earlier than
> after-change-functions's `beg' need to have their 'fontified text
> property set to nil, and this needs to happen at after-change time
> (during redisplay is too late).  This necessity even applies to
> extending change regions to whole lines.

Yes and no.  The current code indeed has a weakness in that there's no
way for a jit-lock-function (e.g. font-lock) to tell jit-lock which part
of the buffer it actually did re-fontify (so that jit-lock can in turn
use jit-lock-force-redisplay when needed).  I've been meaning to address
this for years, but never got around to it.  This is only a problem when
a change on one line affects the display on a *previous* line (and this is
done via font-lock-extend-region-functions), so it's rarely problematic.

This said, if a change on line N needs some other line M<N to be
refreshed, and yet font-lock-extend-region-functions doesn't move beg
from N to M (because the two don't need to be refreshed at the same
time, for example), there's yet another way to handle it, which is to
use jit-lock-multiline (which affects jit-lock-context's fontification,
so line M will be refreshed after jit-lock-context-time).


        Stefan

Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

[Stefan Monnier]

>> >     (c-extend-after-change-region): Explicitly apply 'fontified properties
>> >     to the extended bits of the font lock region.

>> Sounds like this is working around a problem elsewhere, so better add
>> a comment explaining what's going on.

> jit-lock-force-redisplay (that function which is set to run on the
> expiry of a 0 second timer, when jit-lock-fontify-now suspects that some
> area which has already been rendered requires to be re-rendered) doesn't
> seem to work.  At least, I couldn't get it to work.

That doesn't explain why you need to set the `fontified' property
manually, which is normally done by the code that runs
c-extend-after-change-region (via
font-lock-extend-after-change-region-function).

> f-l-extend-j-l-r-after-change has a recently fixed bug, where the
> extended region is only returned to the caller in certain circumstances.
> Those circumstances no longer hold in CC Mode.  To allow CC Mode to
> continue to work in already released Emacs versions, it is necessary to
> set the 'fontified text-property to nil from
> c-extend-after-change-region.

Ah, so that's what's going on.  This is the part that needs to be
documented in a comment (and I really mean a comment, not in a ChangeLog).


        Stefan

Re: [Emacs-diffs] master 05a5a94: Correct calculation of CC Mode's font-lock region.

[Stefan Monnier]

> Yes and no.  The current code indeed has a weakness in that there's no
> way for a jit-lock-function (e.g. font-lock) to tell jit-lock which part
> of the buffer it actually did re-fontify (so that jit-lock can in turn
> use jit-lock-force-redisplay when needed).  I've been meaning to address
> this for years, but never got around to it.

Alright, now it's done.


        Stefan