bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hi, Emacs

In font-lock-extend-jit-lock-region-after-change in font-lock.el, the
new fontification region is calculated by funcalling
font-lock-extend-after-change-region-function.  The result of this
funcall is assigned to internal variables beg and end.

To return these values to the caller, they should be written to the
dynamically bound variables jit-lock-start and jit-lock-end.  This is
only done in the case

    (memq 'font-lock-extend-region-wholelines
           font-lock-extend-region-functions)

.  It should be done in all cases.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

> In font-lock-extend-jit-lock-region-after-change in font-lock.el, the
> new fontification region is calculated by funcalling
> font-lock-extend-after-change-region-function.  The result of this
> funcall is assigned to internal variables beg and end.

> To return these values to the caller, they should be written to the
> dynamically bound variables jit-lock-start and jit-lock-end.  This is
> only done in the case

>     (memq 'font-lock-extend-region-wholelines
>            font-lock-extend-region-functions)

> .  It should be done in all cases.

I'm thinking of removing font-lock-extend-after-change-region-function
altogether anyway: cc-mode is the only user and it has a workaround in
place for when the "feature" is absent, so it's really just useless.


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello, Stefan.

On Fri, Mar 20, 2015 at 10:20:24AM -0400, Stefan Monnier wrote:
> > In font-lock-extend-jit-lock-region-after-change in font-lock.el, the
> > new fontification region is calculated by funcalling
> > font-lock-extend-after-change-region-function.  The result of this
> > funcall is assigned to internal variables beg and end.

> > To return these values to the caller, they should be written to the
> > dynamically bound variables jit-lock-start and jit-lock-end.  This is
> > only done in the case

> >     (memq 'font-lock-extend-region-wholelines
> >            font-lock-extend-region-functions)

> > .  It should be done in all cases.

> I'm thinking of removing font-lock-extend-after-change-region-function
> altogether anyway: cc-mode is the only user and it has a workaround in
> place for when the "feature" is absent, so it's really just useless.

Well, I've fixed it anyhow.

I don't think you should remove f-l-exent-a-c-region-function.  The only
alternative is, as you say, a workaround, one that involves advising
functions.  Back in 2006 when the facility was implemented, the need to
use advice was taken to be a deficiency in Emacs, a bug to be fixed.
This particular bug was fixed.  Also, the fact the only one package uses
something is not a good reason to remove features.  If it were, Emacs
might well be a lot slimmer than it is.

Right at the moment I am plagued by Font/Jit Lock's messing around with
the font-lock region's start position.  Despite CC Mode quite definitely
setting this region (via the said
font-lock-extend-after-change-region-function), Font/Jit Lock insists on
starting at a different place, one with a different syntactic context.

Perhaps we could implement the convention that when a major mode has
positively set the font-lock region's start and end points, these should
be accepted by F/J-lock, but when not, F/J-lock should be free to alter
them (as it typically does now).  The existence of
font-lock-extend-after-change-region-function makes this distinction
possible.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

> Perhaps we could implement the convention that when a major mode has
> positively set the font-lock region's start and end points, these should
> be accepted by F/J-lock, but when not, F/J-lock should be free to alter
> them (as it typically does now).

No the core of the API is font-lock-fontify-region and it should work
with *any* bounds (i.e. if these need to be extended, it should be done
by font-lock-extend-region-function).

Jit-lock is implemented on top of that API and is hence free to use any
bounds it sees fit.

If you rely on more specific bounds being passed to
font-lock-fontify-region, that means you have a problem on your side.

> The existence of font-lock-extend-after-change-region-function makes
> this distinction possible.

The existence of font-lock-extend-after-change-region-function is an
error on my part (More specifically the result of a weakness on my part:
when you requested this feature, I added
font-lock-extend-region-function (which was the right fix) and
reluctantly accepted to also add
font-lock-extend-after-change-region-function just out of tiredness of
arguing that it was the wrong solution).


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello again, Stefan.

On Fri, Mar 20, 2015 at 03:39:10PM -0400, Stefan Monnier wrote:
> > Perhaps we could implement the convention that when a major mode has
> > positively set the font-lock region's start and end points, these should
> > be accepted by F/J-lock, but when not, F/J-lock should be free to alter
> > them (as it typically does now).

> No the core of the API is font-lock-fontify-region and it should work
> with *any* bounds (i.e. if these need to be extended, it should be done
> by font-lock-extend-region-function).

However, when the bounds are set by the major mode, those bounds should
be respected by Font Lock.  As I pointed out to Daniel, the major mode is
the expert in where font-locking must start whereas Font Lock itself is
just an enthusiastic amateur, completely lacking the structural knowledge
the major mode has of the buffer.

> Jit-lock is implemented on top of that API and is hence free to use any
> bounds it sees fit.

But surely not to pass bounds to the major mode that the major mode must,
for a second time, adjust.  This is crazy.

> If you rely on more specific bounds being passed to
> font-lock-fontify-region, that means you have a problem on your side.

There is a problem, yes, but on which side it is is a matter for
discussion.  CC Mode has always relied on the specific bounds that it
provides.  This is the whole point of the 2006 interface.

You yourself pointed out that CC Mode is the sole user of the
functionality for expanding FL regions.  The main point of expanding
these regions, from CC Mode's point of view is to instruct Font Lock
where to begin, so that CC Mode's font-locking then functions correctly.

If CC Mode cannot rely on Font Lock being obedient, then it must
internally store the correct starting and ending points and somehow
substitute these for the inaccurate ones supplied by Font Lock.

> > The existence of font-lock-extend-after-change-region-function makes
> > this distinction possible.

> The existence of font-lock-extend-after-change-region-function is an
> error on my part (More specifically the result of a weakness on my part:
> when you requested this feature, I added
> font-lock-extend-region-function (which was the right fix) and
> reluctantly accepted to also add
> font-lock-extend-after-change-region-function just out of tiredness of
> arguing that it was the wrong solution).

Yes, it was an exhausting discussion back in 2006.  But
f-l-extend-after-change-r-f works well.  If you change the interface to
have only font-lock-extend-region-functions, then you rule out what
somebody (was it Daniel?) recently called "edge triggered" fontification,
leaving only "level triggered".

AWK Mode (if not others) uses edge triggered fontification:  For the
calculation of its FL region, it uses `beg' and `end' from
before-change-functions and `beg', `end', and `old-len' from
after-change-functions.  If f-l-extend-after-change-r-f vanishes, some
other means will have to be found to transmit this info to Font Lock -
the ugly advice used by earlier Emacs versions, for example.


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 2352 bytes --]

On 03/20/2015 05:00 PM, Alan Mackenzie wrote:
>> The existence of font-lock-extend-after-change-region-function is an
>> error on my part (More specifically the result of a weakness on my part:
>> when you requested this feature, I added
>> font-lock-extend-region-function (which was the right fix) and
>> reluctantly accepted to also add
>> font-lock-extend-after-change-region-function just out of tiredness of
>> arguing that it was the wrong solution).
> 
> Yes, it was an exhausting discussion back in 2006.  But
> f-l-extend-after-change-r-f works well.  If you change the interface to
> have only font-lock-extend-region-functions, then you rule out what
> somebody (was it Daniel?) recently called "edge triggered" fontification,
> leaving only "level triggered".
> 
> AWK Mode (if not others) uses edge triggered fontification:  For the
> calculation of its FL region, it uses `beg' and `end' from
> before-change-functions and `beg', `end', and `old-len' from
> after-change-functions.  If f-l-extend-after-change-r-f vanishes, some
> other means will have to be found to transmit this info to Font Lock -
> the ugly advice used by earlier Emacs versions, for example.

Level-triggered fontification is the only correct scheme. You don't need
fine-grained control over the font-lock region. You need better cache
invalidation.

Font-lock can ask for the right to ask for the fontification of any
range of characters. If I want to, I can install customization that
changes the font-lock region to a whole paragraph, a whole defun, or a
whole file. None of that should matter.

Some modes might have caches that reflect buffer contents --- they
should invalidate these caches in before- and after-change-functions,
before font-lock even runs.

Let me put it another way: a highlighter's job is to find the correct
face for a given buffer position. In order to not drive the user insane,
that face must be a function solely of the contents of the buffer and
cached information about the contents of the buffer. Otherwise,
fontification will change depending on scrolling, jit-lock chunk size,
and other factors. None of these things should affect the faces that we
ultimately apply to characters.

Maybe we should have some tests that do fontification one character at a
time, or in random order.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello, Daniel.

On Fri, Mar 20, 2015 at 06:06:55PM -0700, Daniel Colascione wrote:
> On 03/20/2015 05:00 PM, Alan Mackenzie wrote:
> >> The existence of font-lock-extend-after-change-region-function is an
> >> error on my part (More specifically the result of a weakness on my part:
> >> when you requested this feature, I added
> >> font-lock-extend-region-function (which was the right fix) and
> >> reluctantly accepted to also add
> >> font-lock-extend-after-change-region-function just out of tiredness of
> >> arguing that it was the wrong solution).

> > Yes, it was an exhausting discussion back in 2006.  But
> > f-l-extend-after-change-r-f works well.  If you change the interface to
> > have only font-lock-extend-region-functions, then you rule out what
> > somebody (was it Daniel?) recently called "edge triggered" fontification,
> > leaving only "level triggered".

> > AWK Mode (if not others) uses edge triggered fontification:  For the
> > calculation of its FL region, it uses `beg' and `end' from
> > before-change-functions and `beg', `end', and `old-len' from
> > after-change-functions.  If f-l-extend-after-change-r-f vanishes, some
> > other means will have to be found to transmit this info to Font Lock -
> > the ugly advice used by earlier Emacs versions, for example.

> Level-triggered fontification is the only correct scheme.

Can you offer any evidence, or argumentation for this opinion?  As I
said, edge-triggered fontification works in AWK Mode and works well.  I'm
not quite sure at the moment whether the other CC Mode modes use it.

> You don't need fine-grained control over the font-lock region.

Major modes need absolute control over where font-locking analysis starts
- they must be able to chose a position with a neutral syntactic context.
For example, when Font Lock asks for fontification starting in the
inside of a C++ declaration, C++ Mode needs to be able to say STOP!  GO
BACK!  CARRY ON!

> You need better cache invalidation.

When, where, of what?

> Font-lock can ask for the right to ask for the fontification of any
> range of characters. If I want to, I can install customization that
> changes the font-lock region to a whole paragraph, a whole defun, or a
> whole file. None of that should matter.

Of course.  But AFTER that selection, the major mode decides where to
start analysing based on the selection.  As I pointed out to Stefan, we
don't distinguish between "place to start analysing" and "place to start
applying face properties", so we can only talk about "the Font Lock
region".  I think the critical point is:  Several things can choose,
expand (?or contract) a region to fontify.  But the major mode must be
the last entity that does so.

> Some modes might have caches that reflect buffer contents --- they
> should invalidate these caches in before- and after-change-functions,
> before font-lock even runs.

Not quite sure exactly what sort of caches you're thinking about, but
they will get updated, rather than invalidated, in the
before/after-change-functions functions, surely?

> Let me put it another way: a highlighter's job is to find the correct
> face for a given buffer position. In order to not drive the user insane,
> that face must be a function solely of the contents of the buffer and
> cached information about the contents of the buffer. Otherwise,
> fontification will change depending on scrolling, jit-lock chunk size,
> and other factors. None of these things should affect the faces that we
> ultimately apply to characters.

Of course.  But they affect the way we calculate those faces.

> Maybe we should have some tests that do fontification one character at a
> time, or in random order.

Now there's an idea.  :-)

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Daniel Colascione]

[-- Attachment #1: Type: text/plain, Size: 4960 bytes --]



On 03/21/2015 03:58 AM, Alan Mackenzie wrote:
> Hello, Daniel.
> 
> On Fri, Mar 20, 2015 at 06:06:55PM -0700, Daniel Colascione wrote:
>> On 03/20/2015 05:00 PM, Alan Mackenzie wrote:
>>>> The existence of font-lock-extend-after-change-region-function is an
>>>> error on my part (More specifically the result of a weakness on my part:
>>>> when you requested this feature, I added
>>>> font-lock-extend-region-function (which was the right fix) and
>>>> reluctantly accepted to also add
>>>> font-lock-extend-after-change-region-function just out of tiredness of
>>>> arguing that it was the wrong solution).
> 
>>> Yes, it was an exhausting discussion back in 2006.  But
>>> f-l-extend-after-change-r-f works well.  If you change the interface to
>>> have only font-lock-extend-region-functions, then you rule out what
>>> somebody (was it Daniel?) recently called "edge triggered" fontification,
>>> leaving only "level triggered".
> 
>>> AWK Mode (if not others) uses edge triggered fontification:  For the
>>> calculation of its FL region, it uses `beg' and `end' from
>>> before-change-functions and `beg', `end', and `old-len' from
>>> after-change-functions.  If f-l-extend-after-change-r-f vanishes, some
>>> other means will have to be found to transmit this info to Font Lock -
>>> the ugly advice used by earlier Emacs versions, for example.
> 
>> Level-triggered fontification is the only correct scheme.
> 
> Can you offer any evidence, or argumentation for this opinion?  As I
> said, edge-triggered fontification works in AWK Mode and works well.  I'm
> not quite sure at the moment whether the other CC Mode modes use it.

If fontification depends on recent buffer history, then fontification
depends on recent buffer history. That's non-determinism. I think it
ought to be obvious why we want to highlight the same characters with
the same faces no matter how those characters came to be in the buffer.

>> You don't need fine-grained control over the font-lock region.
> 
> Major modes need absolute control over where font-locking analysis starts
> - they must be able to chose a position with a neutral syntactic context.
> For example, when Font Lock asks for fontification starting in the
> inside of a C++ declaration, C++ Mode needs to be able to say STOP!  GO
> BACK!  CARRY ON!

The mode can start analyzing wherever it wants. There is no rule saying
that a mode's font-locking functions can't look at buffer positions
before `beg'. font-lock is telling you were it wants you to apply
fontification. You can go back further than that to analyze.

This analysis does not requiring that we forbid font-lock from
arbitrarily extending the region. Why can't cc-mode figure out a
"neutral syntactic position" before an arbitrary point?

>> You need better cache invalidation.
> 
> When, where, of what?

Whatever you're trying to achieve by constricting the font-lock region,
you can achieve equally well by maintaining the right caches of buffer
context and consulting these caches when servicing font-lock
fontification requests.

>> Font-lock can ask for the right to ask for the fontification of any
>> range of characters. If I want to, I can install customization that
>> changes the font-lock region to a whole paragraph, a whole defun, or a
>> whole file. None of that should matter.
> 
> Of course.  But AFTER that selection, the major mode decides where to
> start analysing based on the selection.  As I pointed out to Stefan, we
> don't distinguish between "place to start analysing" and "place to start
> applying face properties", so we can only talk about "the Font Lock
> region".  I think the critical point is:  Several things can choose,
> expand (?or contract) a region to fontify.  But the major mode must be
> the last entity that does so.

Like I said, the mode can analyze whatever buffer text it wants. Why
does the analysis region (which is implicit in program logic) depend on
the font-lock region?

>> Some modes might have caches that reflect buffer contents --- they
>> should invalidate these caches in before- and after-change-functions,
>> before font-lock even runs.
> 
> Not quite sure exactly what sort of caches you're thinking about, but
> they will get updated, rather than invalidated, in the
> before/after-change-functions functions, surely?
> 
>> Let me put it another way: a highlighter's job is to find the correct
>> face for a given buffer position. In order to not drive the user insane,
>> that face must be a function solely of the contents of the buffer and
>> cached information about the contents of the buffer. Otherwise,
>> fontification will change depending on scrolling, jit-lock chunk size,
>> and other factors. None of these things should affect the faces that we
>> ultimately apply to characters.
> 
> Of course.  But they affect the way we calculate those faces.

Why?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

>> > Perhaps we could implement the convention that when a major mode has
>> > positively set the font-lock region's start and end points, these should
>> > be accepted by F/J-lock, but when not, F/J-lock should be free to alter
>> > them (as it typically does now).
>> No the core of the API is font-lock-fontify-region and it should work
>> with *any* bounds (i.e. if these need to be extended, it should be done
>> by font-lock-extend-region-function).
> However, when the bounds are set by the major mode, those bounds should
> be respected by Font Lock.

The major mode sets font-lock-extend-region-function and this functions'
result should be (and is) respected by the rest of font-lock.

But callers of font-lock-fontify-region (such as
font-lock-after-change-function, or jit-lock) can choose *any* bounds
they feel like and font-lock-fontify-region should behave correctly.

> As I pointed out to Daniel, the major mode is the expert in where
> font-locking must start

Indeed, and it instructs font-lock on how to extend the region by
setting font-lock-extend-region-function.

>> Jit-lock is implemented on top of that API and is hence free to use any
>> bounds it sees fit.
> But surely not to pass bounds to the major mode that the major mode must,
> for a second time, adjust.  This is crazy.

No, it is just good design to keep complexity under check.

>> If you rely on more specific bounds being passed to
>> font-lock-fontify-region, that means you have a problem on your side.
> There is a problem, yes, but on which side it is is a matter for
> discussion.  CC Mode has always relied on the specific bounds that it
> provides.

AFAIK CC-mode does not provide any bounds.  Instead it uses
font-lock-extend-after-change-region-function which changes the part of
the buffer that is invalidated, which is something different.

The relationship between the two is brittle and subtle, so relying on it
is crazy and a good way to get impenetrable code with special cases
tacked on top of special cases with lots of unspecified assumptions that
only hold in common cases.

> You yourself pointed out that CC Mode is the sole user of the
> functionality for expanding FL regions.

Indeed.  Others use font-lock-extend-region-function.

font-lock-extend-after-change-region-function can be useful when
a *change* on line N can cause a highlighting change on a line <N, or
when it causes a highlighting change on a line >N and you don't like the
jit-lock-context-time delay (or you want it to work correctly even if
jit-lock(-context) is not in use).


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello, Stefan.

On Fri, Mar 20, 2015 at 10:29:16PM -0400, Stefan Monnier wrote:
> >> > Perhaps we could implement the convention that when a major mode has
> >> > positively set the font-lock region's start and end points, these should
> >> > be accepted by F/J-lock, but when not, F/J-lock should be free to alter
> >> > them (as it typically does now).
> >> No the core of the API is font-lock-fontify-region and it should work
> >> with *any* bounds (i.e. if these need to be extended, it should be done
> >> by font-lock-extend-region-function).
> > However, when the bounds are set by the major mode, those bounds should
> > be respected by Font Lock.

> The major mode sets font-lock-extend-region-function and this functions'
> result should be (and is) respected by the rest of font-lock.

It is not.  font-lock-extend-region-functions (note the "s") is plural,
and all functions on it are run repeatedly until none makes a change.  So
when the major mode sets the region, this is instantly violated by the
other functions in f-l-extend-r-f.  This is what caused bug #19669, and
I'm still struggling to find a way round it.

The problem is the mixing up of Font Lock's internal functions with the
major mode's in f-l-extend-region-functions.  The major mode's function
is not given the priority it should have.

Is there any particular reason for the repeated running of the functions
on f-l-extend-region-functions?  Why are they not simply each run once?
Is there any use case where it is helpful for one of these functions to
make a second (or subsequent) change to the font-lock region?  Why don't
we change this bit to a simple `run-hook' form?

> But callers of font-lock-fontify-region (such as
> font-lock-after-change-function, or jit-lock) can choose *any* bounds
> they feel like and font-lock-fontify-region should behave correctly.

If the major mode is going to get *any* bounds rather than the ones it
has already specified by its function on f-l-extend-region-functions, it
will need to adjust those bounds again, to a syntactically acceptable
place.  This can't be the Right Thing.

> >> Jit-lock is implemented on top of that API and is hence free to use
> >> any bounds it sees fit.
> > But surely not to pass bounds to the major mode that the major mode
> > must, for a second time, adjust.  This is crazy.

> No, it is just good design to keep complexity under check.

???

> >> If you rely on more specific bounds being passed to
> >> font-lock-fontify-region, that means you have a problem on your side.
> > There is a problem, yes, but on which side it is is a matter for
> > discussion.  CC Mode has always relied on the specific bounds that it
> > provides.

> AFAIK CC-mode does not provide any bounds.  Instead it uses
> font-lock-extend-after-change-region-function which changes the part of
> the buffer that is invalidated, which is something different.

No it's not different.  The bounds CC Mode provides are those around the
region which is to be invalidated, and later refontified.  I think you're
picking nits here.

> The relationship between the two is brittle and subtle, so relying on it
> is crazy and a good way to get impenetrable code with special cases
> tacked on top of special cases with lots of unspecified assumptions that
> only hold in common cases.

What is the alternative?  CC Mode knows exactly what portion of the
buffer needs refontifying, Font Lock doesn't, and can't.  Any chance of a
robust way of communicating those region bounds to Font Lock?  

> > You yourself pointed out that CC Mode is the sole user of the
> > functionality for expanding FL regions.

> Indeed.  Others use font-lock-extend-region-function.

> font-lock-extend-after-change-region-function can be useful when
> a *change* on line N can cause a highlighting change on a line <N, or
> when it causes a highlighting change on a line >N and you don't like the
> jit-lock-context-time delay (or you want it to work correctly even if
> jit-lock(-context) is not in use).

Yes.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

>> The major mode sets font-lock-extend-region-function and this functions'
>> result should be (and is) respected by the rest of font-lock.
> It is not.  font-lock-extend-region-functions (note the "s") is plural,
> and all functions on it are run repeatedly until none makes a change.  So
> when the major mode sets the region, this is instantly violated by the
> other functions in f-l-extend-r-f.  This is what caused bug #19669, and
> I'm still struggling to find a way round it.

The whole set of functions is under the control of the major-mode.
So if you don't like the other two functions, you can remove them just
fine (as you've done).

But as you've now seen in this bug#20146, removing
font-lock-extend-region-wholelines is probably not a good idea because
your own font-lock rules rely on it.

And font-lock-extend-region-multiline has no effect if you don't use
set the `font-lock-multiline' property, so removing it would only affect
performance, not behavior.

> Is there any use case where it is helpful for one of these functions to
> make a second (or subsequent) change to the font-lock region?

Of course: most font-lock-keywords will misbehave if the region is not
made up of whole lines.  So if font-lock-extend-region-multiline extends
the region to something that's not made of whole lines we have
a problem.
Similarly, if font-lock-extend-region-wholelines extends the region to
start or end in the middle of a font-lock-multiline property we have
a problem.
So they need to be cycled.

>> But callers of font-lock-fontify-region (such as
>> font-lock-after-change-function, or jit-lock) can choose *any* bounds
>> they feel like and font-lock-fontify-region should behave correctly.
> If the major mode is going to get *any* bounds rather than the ones it
> has already specified by its function on f-l-extend-region-functions,

f-l-extend-region-functions is run *after* font-lock-fontify-region is
called, so I don't understand what you mean by "already".
And those bounds aren't changed afterwards.

>> No, it is just good design to keep complexity under check.
> ???

For example, it means, that if the highlighting is incorrect, it *can't*
be because of a bug in jit-lock.  A highlighting problem can only come
from jit-lock in case the highlighting has simply not been (re)applied.

>> AFAIK CC-mode does not provide any bounds.  Instead it uses
>> font-lock-extend-after-change-region-function which changes the part of
>> the buffer that is invalidated, which is something different.
> No it's not different.  The bounds CC Mode provides are those around the
> region which is to be invalidated, and later refontified.  I think you're
> picking nits here.

I'm definitely not picking nits.  Those two concepts are similar yet
different and independent.

> What is the alternative?  CC Mode knows exactly what portion of the
> buffer needs refontifying, Font Lock doesn't, and can't.  Any chance of a
> robust way of communicating those region bounds to Font Lock?

Yes: font-lock-extend-region-functions.


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hi, Stefan.

On Fri, Mar 20, 2015 at 10:29:16PM -0400, Stefan Monnier wrote:
> >> No the core of the API is font-lock-fontify-region and it should work
> >> with *any* bounds (i.e. if these need to be extended, it should be done
> >> by font-lock-extend-region-function).
> > However, when the bounds are set by the major mode, those bounds should
> > be respected by Font Lock.

> The major mode sets font-lock-extend-region-function and this functions'
> result should be (and is) respected by the rest of font-lock.

If only, but this is simply not the case at the moment.
jit-lock-fontify-now has a hard-coded extension to whole lines,
regardless of the contents of font-lock-extend-region-functions.  It is
this hard-coded extended region that jit-lock-fontify-now passes to
font-lock-fontify-region, which then attempts to extend this region
further.

This is surely a bug.

           ;; Until someone has a better idea, let's start
           ;; at the start of the line containing START and
           ;; stop at the start of the line following NEXT.

As a better idea, why not have jit-lock-fontify-now use
f-l-extend-region-functions, and then pass the original `start' and
`next' to f-l-fontify-region.  That way, we would be sure that the two
functions, one setting/clearing text properties, the other doing the
fontification, would actually be working on the same portion of the
buffer.  It would bring jit-lock closer to behaving the same as
unassisted font-lock.  And it would allow a major mode to set an
arbitrary font-lock region, of course.

> But callers of font-lock-fontify-region (such as
> font-lock-after-change-function, or jit-lock) can choose *any* bounds
> they feel like and font-lock-fontify-region should behave correctly.

This seems to be true.  When I (setq font-lock-support-mode nil), things
seem to behave properly.  It seems a poor workaround, though.

[ ... ]

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

>> The major mode sets font-lock-extend-region-function and this functions'
>> result should be (and is) respected by the rest of font-lock.
> If only, but this is simply not the case at the moment.
> jit-lock-fontify-now has a hard-coded extension to whole lines,
> regardless of the contents of font-lock-extend-region-functions.

There's no relationship between the two.  The bounds that
font-lock-fontify-region are not under the major mode's control.
What is under the major mode's control is how those bounds are extended
by font-lock-extend-region-functions.

> This is surely a bug.

Nope.

> As a better idea, why not have jit-lock-fontify-now use
> f-l-extend-region-functions, and then pass the original `start' and
> `next' to f-l-fontify-region.

No, you have that backwards: it's f-l-fontify-region which runs
f-l-extend-region-functions, specifically so that you don't need to care
about what jit-lock (and other callers of f-l-fontify-region) might do
(you just need to make sure that f-l-fontify-region works correctly for
*any* bounds).

>> But callers of font-lock-fontify-region (such as
>> font-lock-after-change-function, or jit-lock) can choose *any* bounds
>> they feel like and font-lock-fontify-region should behave correctly.
> This seems to be true.  When I (setq font-lock-support-mode nil), things
> seem to behave properly.  It seems a poor workaround, though.

I don't see how what you say relates to what you quoted.


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello, Stefan.

On Sat, Mar 21, 2015 at 06:30:26PM -0400, Stefan Monnier wrote:
> >> The major mode sets font-lock-extend-region-function and this functions'
> >> result should be (and is) respected by the rest of font-lock.
> > If only, but this is simply not the case at the moment.
> > jit-lock-fontify-now has a hard-coded extension to whole lines,
> > regardless of the contents of font-lock-extend-region-functions.

> There's no relationship between the two.  The bounds that
> font-lock-fontify-region [gets] are not under the major mode's control.

Yes they are.  They can be set by
font-lock-extend-after-change-region-function, as described in the elisp
manual.

> What is under the major mode's control is how those bounds are extended
> by font-lock-extend-region-functions.

But by that time, jit-lock has already played fast and loose with the
bounds supplied by the major mode.  It is too late by that time for
anything in font-lock-fontify-region to repair the damage.

> > This is surely a bug.

> Nope.

Yes.  I think you've said, indeed quite often, that font-lock has the
same behaviour regardless of whether jit-lock is enabled or not.  Here,
there is a marked difference of behaviour: different regions are
provided to font-lock-fontify-region in the two cases.

> > As a better idea, why not have jit-lock-fontify-now use
> > f-l-extend-region-functions, and then pass the original `start' and
> > `next' to f-l-fontify-region.

> No, you have that backwards: it's f-l-fontify-region which runs
> f-l-extend-region-functions, specifically so that you don't need to care
> about what jit-lock (and other callers of f-l-fontify-region) might do
> (you just need to make sure that f-l-fontify-region works correctly for
> *any* bounds).

I need to care a great deal about what jit-lock does, for the reasons in
this thread.  One way out of the current problem might involve testing
font-lock-support-mode in the CC Mode code.  This would not be ideal.

Looking more critically at jit-lock-fontify-now, there is simply no need
for it to expand the region to whole lines.  The only things it ever does
itself with (start next) are
(i) Update jit-lock-context-unfontify-pos;
(ii) Set fontified text properties;
(iii) Call f-l-fontify-region (and other functions on
  jit-lock-functions).

Not extending (start next) to whole lines
(i) wouldn't make any difference, or if it would, it could be handled at
  the place where jit-lock-context-unfontify-pos is set;
(ii) wouldn't make any difference at all;
(iii) wouldn't make any difference to f-l-fontify-region, which itself
  does all the requisite region extending.  The other functions
  (bug-reference-fontify, glasses-change, goto-address-fontify-region)
  may need a little adjustment, but probably not.

As a bonus, not extending (start next) would completely prevent the
condition that triggers (via a timer) jit-lock-force-redisplay, so we
could remove that part of the code and j-l-f-redisplay itself.  (It is
true that buffer positions before start could have been refontified, but
that is also true of the existing code).

Likewise, there is then no longer any purpose in extending to whole
lines in font-lock-extend-jit-lock-region-after-change, so we can remove
that bit, too.

All in all, a massive simplification, and it allows #19669 to be fixed
easily, at least for Emacs 25.1.

Abstractly seen, splitting the action of extension between two modules
is poor design, leading to problems.  Part of this extending MUST be
done in f-l-fontify-region, so it makes sense to do it only there.

> >> But callers of font-lock-fontify-region (such as
> >> font-lock-after-change-function, or jit-lock) can choose *any* bounds
> >> they feel like and font-lock-fontify-region should behave correctly.
> > This seems to be true.  When I (setq font-lock-support-mode nil), things
> > seem to behave properly.  It seems a poor workaround, though.

> I don't see how what you say relates to what you quoted.

Sorry, it was a bit opaque.  I think I was saying that disabling
jit-lock enables the major mode to chose the bounds given to
f-l-fontify-region (via f-l-extend-after-change-r-f), and that f-l-f-r
does indeed behave correctly w.r.t. whatever bounds it gets.  But
disabling jit-lock would be a poor workaround to the problem.

So, as an opener, I propose the following improvement:


diff --git a/lisp/font-lock.el b/lisp/font-lock.el
index 1838a0f..c62945d 100644
--- a/lisp/font-lock.el
+++ b/lisp/font-lock.el
@@ -1316,20 +1316,6 @@ This function does 2 things:
                 ;; the line was deleted.  Or a \n was inserted in the middle
                 ;; of a line.
                 (1+ end))))
-      ;; Finally, pre-enlarge the region to a whole number of lines, to try
-      ;; and anticipate what font-lock-default-fontify-region will do, so as to
-      ;; avoid double-redisplay.
-      ;; We could just run `font-lock-extend-region-functions', but since
-      ;; the only purpose is to avoid the double-redisplay, we prefer to
-      ;; do here only the part that is cheap and most likely to be useful.
-      (when (memq 'font-lock-extend-region-wholelines
-                  font-lock-extend-region-functions)
-        (goto-char beg)
-        (setq beg (min jit-lock-start (line-beginning-position)))
-        (goto-char end)
-        (setq end
-              (max jit-lock-end
-                   (if (bolp) (point) (line-beginning-position 2)))))
       (setq jit-lock-start beg
 	    jit-lock-end end))))
 
diff --git a/lisp/jit-lock.el b/lisp/jit-lock.el
index 788646c..933bbe3 100644
--- a/lisp/jit-lock.el
+++ b/lisp/jit-lock.el
@@ -366,7 +366,7 @@ Defaults to the whole buffer.  END can be out of bounds."
      ;; from the end of a buffer to its start, can do repeated
      ;; `parse-partial-sexp' starting from `point-min', which can
      ;; take a long time in a large buffer.
-     (let ((orig-start start) next)
+     (let (next)
        (save-match-data
 	 ;; Fontify chunks beginning at START.  The end of a
 	 ;; chunk is either `end', or the start of a region
@@ -376,15 +376,6 @@ Defaults to the whole buffer.  END can be out of bounds."
 	   (setq next (or (text-property-any start end 'fontified t)
 			  end))
 
-	   ;; Decide which range of text should be fontified.
-	   ;; The problem is that START and NEXT may be in the
-	   ;; middle of something matched by a font-lock regexp.
-	   ;; Until someone has a better idea, let's start
-	   ;; at the start of the line containing START and
-	   ;; stop at the start of the line following NEXT.
-	   (goto-char next)  (setq next (line-beginning-position 2))
-	   (goto-char start) (setq start (line-beginning-position))
-
            ;; Make sure the contextual refontification doesn't re-refontify
            ;; what's already been refontified.
            (when (and jit-lock-context-unfontify-pos
@@ -409,35 +400,9 @@ Defaults to the whole buffer.  END can be out of bounds."
 	     (quit (put-text-property start next 'fontified nil)
 		   (funcall 'signal (car err) (cdr err))))
 
-           ;; The redisplay engine has already rendered the buffer up-to
-           ;; `orig-start' and won't notice if the above jit-lock-functions
-           ;; changed the appearance of any part of the buffer prior
-           ;; to that.  So if `start' is before `orig-start', we need to
-           ;; cause a new redisplay cycle after this one so that any changes
-           ;; are properly reflected on screen.
-           ;; To make such repeated redisplay happen less often, we can
-           ;; eagerly extend the refontified region with
-           ;; jit-lock-after-change-extend-region-functions.
-           (when (< start orig-start)
-	     (run-with-timer 0 nil #'jit-lock-force-redisplay
-                             (copy-marker start) (copy-marker orig-start)))
-
 	   ;; Find the start of the next chunk, if any.
 	   (setq start (text-property-any next end 'fontified nil))))))))
 
-(defun jit-lock-force-redisplay (start end)
-  "Force the display engine to re-render START's buffer from START to END.
-This applies to the buffer associated with marker START."
-  (when (marker-buffer start)
-    (with-current-buffer (marker-buffer start)
-      (with-buffer-prepared-for-jit-lock
-       (when (> end (point-max))
-         (setq end (point-max) start (min start end)))
-       (when (< start (point-min))
-         (setq start (point-min) end (max start end)))
-       ;; Don't cause refontification (it's already been done), but just do
-       ;; some random buffer change, so as to force redisplay.
-       (put-text-property start end 'fontified t)))))
 \f
 ;;; Stealth fontification.
 

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

>> >> The major mode sets font-lock-extend-region-function and this functions'
>> >> result should be (and is) respected by the rest of font-lock.
>> > If only, but this is simply not the case at the moment.
>> > jit-lock-fontify-now has a hard-coded extension to whole lines,
>> > regardless of the contents of font-lock-extend-region-functions.
>> There's no relationship between the two.  The bounds that
>> font-lock-fontify-region [gets] are not under the major mode's control.
> Yes they are.  They can be set by
> font-lock-extend-after-change-region-function, as described in the elisp
> manual.

That's where you're confused:
font-lock-extend-after-change-region-function affects the region that's
declared as "in need of refresh because of a change".  But that doesn't
mean that this region will then be passed to font-lock-fontify-region.
For example, if that region is not currently visible in any window, then
jit-lock will typically do nothing.  And if only some part of that
region is visible in a window, then maybe only that part will be then
passed to font-lock-fontify-region so as to actually refresh it.
And maybe that part will be combined with some other parts that were
also in need of refresh and are now newly visible.  Or maybe that part
will actually be fontified in several steps (i.e. several calls to
font-lock-fontify-region).

> Yes.  I think you've said, indeed quite often, that font-lock has the
> same behaviour regardless of whether jit-lock is enabled or not.

That's not exactly what I said.  What I said is that any highlighting bug (other
than a "lack of highlighting") you see with jit-lock can also be
reproduced without jit-lock (in the worst case: with manual calls to
font-lock-fontify-region).

> I need to care a great deal about what jit-lock does, for the reasons in
> this thread.

If you need to care about it, it can only be because of incorrect
assumptions in your font-lock code.

> Looking more critically at jit-lock-fontify-now, there is simply no need
> for it to expand the region to whole lines.

I agree with this statement (I actually have such a change in my local
tree), but that won't help you.
Looking at jit-lock as the source of your pain is a waste of time.

> As a bonus, not extending (start next) would completely prevent the
> condition that triggers (via a timer) jit-lock-force-redisplay, so we
> could remove that part of the code and j-l-f-redisplay itself.  (It is
> true that buffer positions before start could have been refontified, but
> that is also true of the existing code).

> Likewise, there is then no longer any purpose in extending to whole
> lines in font-lock-extend-jit-lock-region-after-change, so we can remove
> that bit, too.

Obviously you still haven't understood the comment:
If we don't extend to whole lines in
font-lock-extend-jit-lock-region-after-change, then any change on a line
which causes a previous char's highlighting to be changed will require
a second redisplay before this change is visible.
The current code could handle it right (i.e. use
jit-lock-force-redisplay to make sure this second redisplay happens
right away), but if you additionally get rid of
jit-lock-force-redisplay, then the incorrect display would stay visible
until something else causes a redisplay.

IOW extending to whole lines in
font-lock-extend-jit-lock-region-after-change is an optimization, but is
definitely not indispensable.

> So, as an opener, I propose the following improvement:

Non-starter, especially given that you have shown repeatedly that you
don't understand the very fundamental design choice that it's perfectly
correct to call font-lock-fontify-region with any bounds whatsoever.


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Alan Mackenzie]

Hello, Stefan.

On Sun, Mar 22, 2015 at 10:01:53PM -0400, Stefan Monnier wrote:
> >> >> The major mode sets font-lock-extend-region-function and this functions'
> >> >> result should be (and is) respected by the rest of font-lock.
> >> > If only, but this is simply not the case at the moment.
> >> > jit-lock-fontify-now has a hard-coded extension to whole lines,
> >> > regardless of the contents of font-lock-extend-region-functions.
> >> There's no relationship between the two.  The bounds that
> >> font-lock-fontify-region [gets] are not under the major mode's control.
> > Yes they are.  They can be set by
> > font-lock-extend-after-change-region-function, as described in the elisp
> > manual.

> That's where you're confused:
> font-lock-extend-after-change-region-function affects the region that's
> declared as "in need of refresh because of a change".  But that doesn't
> mean that this region will then be passed to font-lock-fontify-region.
> For example, if that region is not currently visible in any window, then
> jit-lock will typically do nothing.  And if only some part of that
> region is visible in a window, then maybe only that part will be then
> passed to font-lock-fontify-region so as to actually refresh it.
> And maybe that part will be combined with some other parts that were
> also in need of refresh and are now newly visible.  Or maybe that part
> will actually be fontified in several steps (i.e. several calls to
> font-lock-fontify-region).

Yes, I was confused about that.  I hope I've got it now.  Thanks for the
help.

[ ... ]

> > Looking more critically at jit-lock-fontify-now, there is simply no need
> > for it to expand the region to whole lines.

> I agree with this statement (I actually have such a change in my local
> tree), but that won't help you.
> Looking at jit-lock as the source of your pain is a waste of time.

OK.  But I have to be aware of how jit-lock works: in particular how the
actions in after-change-functions influence those of
jit-lock-fontify-now and font-lock-fontify-region.

[ ... ]

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Stefan Monnier]

> OK.  But I have to be aware of how jit-lock works: in particular how the
> actions in after-change-functions influence those of
> jit-lock-fontify-now and font-lock-fontify-region.

No, that's the wrong way to go about it.
Just assume jit-lock is an *adversary*, which chooses the bounds it passes
to font-lock-fontify-region trying to find holes in your code.


        Stefan

bug#20146: font-lock-extend-jit-lock-region-after-change: results are discarded instead of being returned.

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

>> I'm thinking of removing font-lock-extend-after-change-region-function
>> altogether anyway: cc-mode is the only user and it has a workaround in
>> place for when the "feature" is absent, so it's really just useless.
>
> Well, I've fixed it anyhow.

If I read this thread correctly, the bug in question here was fixed, so
I'm closing this bug report.  Please reopen if that's incorrect.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no