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

[Yuan Fu <casouri@gmail.com>]

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



> On Dec 14, 2023, at 11:12 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Dec 14, 2023, at 5:01 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
>> 
>> On 14/12/2023 10:29, Yuan Fu wrote:
>>>> On Dec 13, 2023, at 5:43 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
>>>> 
>>>> On 13/12/2023 05:28, Yuan Fu wrote:
>>>>>>> c-ts-mode--emacs-set-ranges is registered as a range rule, so many tree-sitter function calls it before doing anything to make sure range is up-to-date. treesit-font-lock-fontify-region calls treesit-update-ranges at the beginning of its body, and treesit-update-ranges calls c-ts-mode--emacs-set-ranges.
>>>>>> 
>>>>>> That seems to mean that any feature accessing the parse tree should call treesit-update-ranges first. Including syntax-propertize-functions and *-extend-region-functions, which we currently don't do.
>>>>>> 
>>>>>> But which boundaries is it supposed to use? Should treesit--syntax-extend-region call treesit-update-ranges, when wait for the parser updates, then possibly call treesit-update-ranges again on the extended boundaries, and so on, until the parser stops sending notifications? Will it stop?
>>>>> It'll stop. If the ranges don't change, no reparse will happen. And only buffer content change can cause range change. Reparse itself can't. So at most you'll see reparse -> update ranges -> reparse.
>>>>>> Anyway, could you try my patch? Like I said, I'm not sure if the insufficient fontification I'm observing in c-ts-mode is due to the problem with the solution, or due to the other redisplay-related problems on my system.
>>>>> Yeah, it doesn't solve the problem in c-ts-mode regarding block comments.
>>>>> We might need to run (progn (force-parse) (update-ranges) (force-parse)) before jit-lock-fontify-now and sytax-ppss.
>>>> 
>>>> Well... I've replaced
>>>> 
>>>> (treesit-buffer-root-node (treesit-language-at (point)))
>>>> 
>>>> in treesit--syntax-extend-region with
>>>> 
>>>> (treesit-buffer-root-node (treesit-language-at (point)))
>>>> (treesit-update-ranges beg end)
>>>> (treesit-buffer-root-node (treesit-language-at (point)))
>>>> 
>>>> and even tried commenting out the call to 'treesit-update-ranges' inside treesit-font-lock-fontify-region, but the result looks unchanged. And the region does get extended, according to my print-debugging inside font-lock-default-fontify-region.
>>>> 
>>>> Could you check if you're seeing the same?
>>> It should be
>>> (treesit-buffer-root-node 'emacs-c)
>>> (treesit-update-ranges)
>>> (treesit-buffer-root-node ‘c)
>> 
>> This might be difficult for treesit--syntax-extend-region to do, since it's supposed to work across modes. But I suppose it could iterate through (mapcar #'treesit-parser-language (treesit-parser-list)).
> 
> Actually, only calling (treesit-update-ranges) should be enough (and is generic), the range update function surely would access the parser, which would force the reparse, and the update function will access the parser in the correct order: host language first, then embedded languages.
> 
>> 
>>> I tried, and it doesn’t make a difference. But I must admit I’m not very clear on what your patch tries to do. Does it try to extend the to-be-fontified region before jit-lock runs?
>> 
>> Yes, it extends the region to-be-fontified. If you set treesit--font-lock-verbose to t, you should see the appropriate messages "Fontifying region: x-y" in the message log where x is the position before the start of the comment (printed by treesit-font-lock-fontify-region).
> 
> Hmm, let me have a closer look tomorrow. It can’t be that difficult, we must have missed something.

Please see this patch, which basically does what you did in your patch. I just started from scratch when experimenting. I didn’t do the context thing, that’s a bit confusing for me. I save updated ranges in a local hash table and access it in extend-region functions to extend the region. Ideally treesit should have a function that returns the updated ranges for a parser, we don’t have that yet, so I’m using the hash table to simulate it.

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.

Yuan


[-- Attachment #2: extend-region.diff --]
[-- Type: application/octet-stream, Size: 2696 bytes --]

diff --git a/lisp/treesit.el b/lisp/treesit.el
index 8a07f5023a9..a6ca6334133 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1080,13 +1080,39 @@ treesit--font-lock-notifier
   "Ensures updated parts of the parse-tree are refontified.
 RANGES is a list of (BEG . END) ranges, PARSER is the tree-sitter
 parser notifying of the change."
-  (with-current-buffer (treesit-parser-buffer parser)
-    (dolist (range ranges)
-      (when treesit--font-lock-verbose
-        (message "Notifier received range: %s-%s"
-                 (car range) (cdr range)))
-      (with-silent-modifications
-        (put-text-property (car range) (cdr range) 'fontified nil)))))
+  ;; (with-current-buffer (treesit-parser-buffer parser)
+  ;;   (dolist (range ranges)
+  ;;     (when treesit--font-lock-verbose
+  ;;       (message "Notifier received range: %s-%s"
+  ;;                (car range) (cdr range)))
+  ;;     (with-silent-modifications
+  ;;       (put-text-property (car range) (cdr range) 'fontified nil))))
+  )
+
+(defun treesit--font-lock-extend-region ()
+  (treesit-update-ranges)
+  (dolist (parser (treesit-parser-list nil nil t))
+    (treesit-parser-root-node parser))
+  (let ((old-beg font-lock-beg)
+        (old-end font-lock-end))
+    (dolist (parser (treesit-parser-list nil nil t))
+      (when-let ((table treesit--parser-updated-ranges)
+                 (ranges (gethash parser table)))
+        (dolist (range ranges)
+          (setq font-lock-beg (max (min font-lock-beg (car range)) (point-min)))
+          (setq font-lock-end (min (max font-lock-end (cdr range)) (point-max))))))
+    (put-text-property font-lock-beg font-lock-end 'fontified nil)
+    (message "extend region: %s-%s %s-%s this-command: %s" old-beg old-end
+             font-lock-beg font-lock-end this-command)))
+
+(defvar-local treesit--parser-updated-ranges nil)
+(defun treesit--general-notifier (ranges parser)
+  "Record that PARSER's updated ranges is RANGES."
+  (message "notifier range: %s %s" parser ranges)
+  (unless treesit--parser-updated-ranges
+    (setq treesit--parser-updated-ranges
+          (make-hash-table :weakness 'key)))
+  (puthash parser ranges treesit--parser-updated-ranges))
 
 ;;; Indent
 
@@ -2422,7 +2448,13 @@ treesit-major-mode-setup
   ;; Imenu.
   (when treesit-simple-imenu-settings
     (setq-local imenu-create-index-function
-                #'treesit-simple-imenu)))
+                #'treesit-simple-imenu))
+
+  (dolist (parser (treesit-parser-list))
+    (treesit-parser-add-notifier
+     parser #'treesit--general-notifier))
+  (add-hook 'font-lock-extend-region-functions
+            #'treesit--font-lock-extend-region nil t))
 
 ;;; Debugging
 

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]