bug#66732: tree-sitter fontification doesn't update multi-line syntax reliably

[Dmitry Gutov]

On 18/12/2023 21:12, Stefan Monnier wrote:
> > > > I found that if you don’t set text prop fontified to nil for the whole
> > > > extended region, redisplay doesn’t seem to, well, redisplay the full
> > > > region, even thought the new face has been correctly applied to them.
> > > That sounds like a bug in font-lock? At the end of jit-lock-fontify-now,
> > > there is a call creating a timer with jit-lock-force-redisplay.
> > > And that function ends with this:
> > >       ;; 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)))))
> > > If I just change t to nil there (or to some other value, like 42), either
> > > of our patches starts behaving well. Perhaps Stefan could comment.
>
> This chunk of code is present so as to cause a re-render (i.e. recompute
> the glyph matrices) of the affected region, but not a re-fontification.
> So a nil value would be wrong.
> I'm surprised that 42 behaves differently from t.

This patch also changes the visible behavior, fixing the problem that 
I'm seeing:

diff --git a/lisp/jit-lock.el b/lisp/jit-lock.el
index 452cbd1ca51..43db2d31856 100644
--- a/lisp/jit-lock.el
+++ b/lisp/jit-lock.el
@@ -499,6 +499,7 @@ jit-lock-force-redisplay
          (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 nil)
        (put-text-property start end 'fontified t)))))
 
 ;;; Stealth fontification.

So it must be not about the eventual value of the property, but about 
triggering some counter, like (buffer-modified-tick). Which does get 
incremented after the above sequence (by 2).

> What we really need here is to force the redisplay to consider that this
> part of the buffer needs to be re-rendered.  In practice changing any
> text-property on that chunk of text should do the trick, in my
> experience, but from what you describe it seems that some optimisation
> is sufficiently clever to notice that the old value was t and the new
> value is identical so the region is not marked as modified?
>
> Indeed, in `add_text_properties_1` I see we skip over intervals which
> already have the right property values, so that might be what's happening.
> I suggest we introduce a separate function with a name indicating what
> we intend it to do (like `force-region-update`) so the code will
> be clearer.
> And its implementation could consist in adding and then removing some
> dummy text property (tho a better implementation would go and modify
> the underlying C-level variables in the buffer like BUF_*_UNCHANGED).

Adding an extra 'put-text-property' call to jit-lock-force-redisplay 
seems cheap enough, but something even faster could be good too.