GNU bug report logs - #66223
treesit-major-mode-setup should not call font-lock-mode

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66223 in the body.
You can then email your comments to 66223 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: bug-gnu-emacs <at> gnu.org
Subject: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 03:17:57 +0300

X-Debbugs-CC: Yuan Fu <casouri <at> gmail.com>

It doesn't seem necessary (everything seems to work okay without that 
call), and it's not the right thing idiomatically (the user should have 
the ability to disable global-font-lock-mode).

If it does get called, the call to treesit-font-lock-recompute-features 
should happen before that.

The report was triggered by somewhat unusual circumstances (somebody 
trying out mmm-mode together with typescript-ts-mode: 
https://github.com/dgutov/mmm-mode/issues/138), but the fix seems easy 
and natural enough.

To reproduce the bug, though, try this:

(with-current-buffer (generate-new-buffer "foo")
  (let (font-lock-support-mode)
    (typescript-ts-mode)))

It results in

Debugger entered--Lisp error: (treesit-query-error "Node type error at" 
2 "(jsx_opening_element [(nested_identifier (identifier)) (identifier)] 
@typescript-ts-jsx-tag-face) (jsx_closing_element [(nested_identifier 
(identifier)) (identifier)] @typescript-ts-jsx-tag-face) 
(jsx_self_closing_element [(nested_identifier (identifier)) 
(identifier)] @typescript-ts-jsx-tag-face) (jsx_attribute 
(property_identifier) @typescript-ts-jsx-attribute-face)" "Debug the 
query with `treesit-query-validate'")
  treesit-query-capture(#<treesit-node program in 1-1> 
#<treesit-compiled-query> 1 1)
  (let* ((delta-start ...
  treesit--font-lock-fontify-region-1(#<treesit-node program in 1-1> 
#<treesit-compiled-query> 1 1 nil nil)
  (let ((sub-node (car tail)))...
  treesit-font-lock-fontify-region(1 1 nil)
  font-lock-fontify-syntactically-region(1 1 nil)
  font-lock-default-fontify-region(1 1 nil)
  font-lock-fontify-region(1 1 nil)
  font-lock-default-fontify-buffer()
  font-lock-fontify-buffer()
  font-lock-initial-fontify()
  font-lock-mode(1)
  (progn (set (make-lo...
  treesit-major-mode-setup()
  typescript-ts-mode()

because typescript-ts-mode's treesit-font-lock-settings hide the jsx 
rule (which the typescript grammar itself doesn't support, only the tsx 
one does) using the absence of that feature in 
treesit-font-lock-feature-list. But for that to take effect, the call to 
'treesit-font-lock-recompute-features' needs to happen first. The 
jit-lock conceals the problem by inhibiting the first fontification 
until the major mode function has run and the buffer is visible. I ended 
up disabling it in mmm-mode's auxiliary temp buffer because it spams the 
message "Not enabling jit-lock: it does not work in indirect buffer".

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 00:21:16 -0700

> On Sep 26, 2023, at 5:17 PM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> X-Debbugs-CC: Yuan Fu <casouri <at> gmail.com>
> 
> It doesn't seem necessary (everything seems to work okay without that call), and it's not the right thing idiomatically (the user should have the ability to disable global-font-lock-mode).
> 
> If it does get called, the call to treesit-font-lock-recompute-features should happen before that.
> 
> The report was triggered by somewhat unusual circumstances (somebody trying out mmm-mode together with typescript-ts-mode: https://github.com/dgutov/mmm-mode/issues/138), but the fix seems easy and natural enough.
> 
> To reproduce the bug, though, try this:
> 
> (with-current-buffer (generate-new-buffer "foo")
>  (let (font-lock-support-mode)
>    (typescript-ts-mode)))
> 
> It results in
> 
> Debugger entered--Lisp error: (treesit-query-error "Node type error at" 2 "(jsx_opening_element [(nested_identifier (identifier)) (identifier)] @typescript-ts-jsx-tag-face) (jsx_closing_element [(nested_identifier (identifier)) (identifier)] @typescript-ts-jsx-tag-face) (jsx_self_closing_element [(nested_identifier (identifier)) (identifier)] @typescript-ts-jsx-tag-face) (jsx_attribute (property_identifier) @typescript-ts-jsx-attribute-face)" "Debug the query with `treesit-query-validate'")
>  treesit-query-capture(#<treesit-node program in 1-1> #<treesit-compiled-query> 1 1)
>  (let* ((delta-start ...
>  treesit--font-lock-fontify-region-1(#<treesit-node program in 1-1> #<treesit-compiled-query> 1 1 nil nil)
>  (let ((sub-node (car tail)))...
>  treesit-font-lock-fontify-region(1 1 nil)
>  font-lock-fontify-syntactically-region(1 1 nil)
>  font-lock-default-fontify-region(1 1 nil)
>  font-lock-fontify-region(1 1 nil)
>  font-lock-default-fontify-buffer()
>  font-lock-fontify-buffer()
>  font-lock-initial-fontify()
>  font-lock-mode(1)
>  (progn (set (make-lo...
>  treesit-major-mode-setup()
>  typescript-ts-mode()
> 
> because typescript-ts-mode's treesit-font-lock-settings hide the jsx rule (which the typescript grammar itself doesn't support, only the tsx one does) using the absence of that feature in treesit-font-lock-feature-list. But for that to take effect, the call to 'treesit-font-lock-recompute-features' needs to happen first. The jit-lock conceals the problem by inhibiting the first fontification until the major mode function has run and the buffer is visible. I ended up disabling it in mmm-mode's auxiliary temp buffer because it spams the message "Not enabling jit-lock: it does not work in indirect buffer”.

Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.

Yuan

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 11:51:59 +0300

On 27/09/2023 10:21, Yuan Fu wrote:
> Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.

If we have to start on master, so be it. I'd like to see the fix in 
emacs-29, though (at least eventually).

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>, Yuan Fu <casouri <at> gmail.com>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 04:15:54 -0700

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 27/09/2023 10:21, Yuan Fu wrote:
>> Makes sense. We can remove that line in master, and see if anything
>> comes up. I don’t remember why I added it, I don’t think there was
>> any particular reason.
>
> If we have to start on master, so be it. I'd like to see the fix in
> emacs-29, though (at least eventually).

Sounds good to me, if no one has any objections.  The call does seem
superfluous there.

I've noted this bug in my list of things to revisit before the first
Emacs 29.2 pretest.  It doesn't hurt if someone else helps remembering
it too, of course.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Stefan Kangas <stefankangas <at> gmail.com>, Yuan Fu <casouri <at> gmail.com>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 22:42:32 +0300

On 27/09/2023 14:15, Stefan Kangas wrote:
> I've noted this bug in my list of things to revisit before the first
> Emacs 29.2 pretest.  It doesn't hurt if someone else helps remembering
> it too, of course.

Thanks!

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 17:23:41 -0700

> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> On 27/09/2023 10:21, Yuan Fu wrote:
>> Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.
> 
> If we have to start on master, so be it. I'd like to see the fix in emacs-29, though (at least eventually).

Yeah, let’s test this out on master for a while and apply it to emacs-29 if no one comes up and complain. It seems to me that 29.2 is still a while away.

Yuan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Thu, 28 Sep 2023 05:40:32 +0200

On September 28, 2023 2:23:41 AM GMT+02:00, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> > On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> > 
> > On 27/09/2023 10:21, Yuan Fu wrote:
> >> Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.
> > 
> > If we have to start on master, so be it. I'd like to see the fix in emacs-29, though (at least eventually).
> 
> Yeah, let’s test this out on master for a while and apply it to emacs-29 if no one comes up and complain. It seems to me that 29.2 is still a while away.

Actually, it isn't: I was thinking about starting a pretest soon.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Wed, 27 Sep 2023 21:55:24 -0700

> On Sep 27, 2023, at 8:40 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
> On September 28, 2023 2:23:41 AM GMT+02:00, Yuan Fu <casouri <at> gmail.com> wrote:
>> 
>> 
>>> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>>> 
>>> On 27/09/2023 10:21, Yuan Fu wrote:
>>>> Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.
>>> 
>>> If we have to start on master, so be it. I'd like to see the fix in emacs-29, though (at least eventually).
>> 
>> Yeah, let’s test this out on master for a while and apply it to emacs-29 if no one comes up and complain. It seems to me that 29.2 is still a while away.
> 
> Actually, it isn't: I was thinking about starting a pretest soon.

Thanks for the heads up. We’ll need to remember to add that change before the pretest is cut, then.

Yuan

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Sun, 8 Oct 2023 16:03:27 -0700

> On Sep 27, 2023, at 5:23 PM, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> 
>> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
>> 
>> On 27/09/2023 10:21, Yuan Fu wrote:
>>> Makes sense. We can remove that line in master, and see if anything comes up. I don’t remember why I added it, I don’t think there was any particular reason.
>> 
>> If we have to start on master, so be it. I'd like to see the fix in emacs-29, though (at least eventually).
> 
> Yeah, let’s test this out on master for a while and apply it to emacs-29 if no one comes up and complain. It seems to me that 29.2 is still a while away.

I have been running with the fix for awhile and didn’t notice anything. No one seems to have complained either. (Hopefully there are people building master and running tree-sitter modes other than me ;-)

I think we can apply this to emacs-29. Is there anything I should do? Do we just cheery-pick the commit into emac-29, or there are more elaborate operations required?

Yuan

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 66223 <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Mon, 9 Oct 2023 02:05:58 +0300

On 09/10/2023 02:03, Yuan Fu wrote:
> I think we can apply this to emacs-29. Is there anything I should do? Do we just cheery-pick the commit into emac-29, or there are more elaborate operations required?

As per admin/notes/git-workflow (I'm not sure why it's that far away),

...
cd ~/emacs/emacs-29
git cherry-pick -xe <commit hash>

and add "Backport:" to the commit string.  Then
...

Thanks!

Message #37 received at 66223-done <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66223-done <at> debbugs.gnu.org
Subject: Re: bug#66223: treesit-major-mode-setup should not call font-lock-mode
Date: Sun, 8 Oct 2023 22:04:04 -0700

> On Oct 8, 2023, at 4:05 PM, Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
> On 09/10/2023 02:03, Yuan Fu wrote:
>> I think we can apply this to emacs-29. Is there anything I should do? Do we just cheery-pick the commit into emac-29, or there are more elaborate operations required?
> 
> As per admin/notes/git-workflow (I'm not sure why it's that far away),
> 
> ...
> cd ~/emacs/emacs-29
> git cherry-pick -xe <commit hash>
> 
> and add "Backport:" to the commit string.  Then
> ...
> 
> Thanks!

Done, thanks.

Yuan

Previous Next

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