GNU bug report logs - #64329
29.0.92; treesit/fill-paragraph syntax highlighting problem

Previous Next

Full log

Message #11 received at 64329 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Troy Brown <brownts <at> troybrown.dev>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64329 <at> debbugs.gnu.org
Subject: Re: bug#64329: 29.0.92; treesit/fill-paragraph syntax highlighting
 problem
Date: Wed, 28 Jun 2023 17:17:14 -0700

[Message part 1 (text/plain, inline)]

> On Jun 28, 2023, at 2:23 PM, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> 
>> On Jun 28, 2023, at 9:46 AM, Troy Brown <brownts <at> troybrown.dev> wrote:
>> 
>> I've noticed this problem on multiple tree-sitter major modes including
>> c-ts-mode, c++-ts-mode, java-ts-mode, bash-ts-mode.  I haven't tried
>> others, but I suspect those might also suffer from this problem.
>> 
>> The issue occurs when attempting to fill the paragraph of a comment
>> block.  The following comment block can be used as an example to
>> reproduce the problem and happens with "emacs -Q" (assuming
>> corresponding tree-sitter libraries are available).
>> 
>> --8<---------------cut here---------------start------------->8---
>> // The quick brown fox jumps over the
>> // lazy dog.
>> // The quick brown fox jumps over the lazy dog.
>> --8<---------------cut here---------------end--------------->8---
>> 
>> Switch to one of the tree-sitter modes (e.g., M-x java-ts-mode).  Move
>> point to the first line of the comment block above and then execute the
>> fill-paragraph command (i.e., M-q).
>> 
>> The text which is wrapped onto the first line of the comment block will
>> be highlighted incorrectly.  The results appear as if the comment
>> delimiter was removed, fontification occurred, then the text was moved
>> to the first line of the comment block and never refontified with the
>> comment face.
> 
> Thank you very much! It’s funny that how long this went under the radar, presumably because we always use block comment.
> 
> The culprit is the subst-char-in-region function used by the filling function. It has a branch:
> 
> if (xxx)
>  {
> replace_range (pos, pos + 1, string, ...);
>  }
> else
>  {
> for (i = 0; i < len; i++) *p++ = tostr[i];
>  }
> 
> I overlooked the else branch and thought subst-char-in-region always calls replace_range. replace_range notifies tree-sitter of the change it makes; but when subst-char-in-region manually replaces the text in the else branch, those edits are not notified to tree-sitter.

Prompted by this, I went over all the functions that calls signal_after_change again, and found two other editfns.c functions that are missing calls to treesit_record_change. Please see the attached patches that follows the previous one. Sorry for the overlook. I believe I’ve found all places that needs to call treesit_record_change now.

> Please see the attached patch. Eli, is it more preferable to add a subroutine in insdel.c that does what "for (i = 0; i < len; i++) *p++ = tostr[I];” does, plus calling treesit_record_change, and make subst-char-in-region call that subroutine? (This way editfns.c don’t need to include treesit.h and call treesit_record_change itself.)

Since now there are three functions in editfns.c that needs to call treesit_record_change, we might as well just include treesit.h and call treesit_record_change directly.

Yuan

[add-two-other-missing-calls.patch (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.