unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#66223: treesit-major-mode-setup should not call font-lock-mode
@ 2023-09-27  0:17 Dmitry Gutov
  2023-09-27  7:21 ` Yuan Fu
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-09-27  0:17 UTC (permalink / raw)
  To: 66223; +Cc: yuan fu

X-Debbugs-CC: Yuan Fu <casouri@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".





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-27  0:17 bug#66223: treesit-major-mode-setup should not call font-lock-mode Dmitry Gutov
@ 2023-09-27  7:21 ` Yuan Fu
  2023-09-27  8:51   ` Dmitry Gutov
  0 siblings, 1 reply; 11+ messages in thread
From: Yuan Fu @ 2023-09-27  7:21 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 66223



> On Sep 26, 2023, at 5:17 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> X-Debbugs-CC: Yuan Fu <casouri@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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-27  7:21 ` Yuan Fu
@ 2023-09-27  8:51   ` Dmitry Gutov
  2023-09-27 11:15     ` Stefan Kangas
  2023-09-28  0:23     ` Yuan Fu
  0 siblings, 2 replies; 11+ messages in thread
From: Dmitry Gutov @ 2023-09-27  8:51 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 66223

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).





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-27  8:51   ` Dmitry Gutov
@ 2023-09-27 11:15     ` Stefan Kangas
  2023-09-27 19:42       ` Dmitry Gutov
  2023-09-28  0:23     ` Yuan Fu
  1 sibling, 1 reply; 11+ messages in thread
From: Stefan Kangas @ 2023-09-27 11:15 UTC (permalink / raw)
  To: Dmitry Gutov, Yuan Fu; +Cc: 66223

Dmitry Gutov <dmitry@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.





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-27 11:15     ` Stefan Kangas
@ 2023-09-27 19:42       ` Dmitry Gutov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Gutov @ 2023-09-27 19:42 UTC (permalink / raw)
  To: Stefan Kangas, Yuan Fu; +Cc: 66223

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!





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-27  8:51   ` Dmitry Gutov
  2023-09-27 11:15     ` Stefan Kangas
@ 2023-09-28  0:23     ` Yuan Fu
  2023-09-28  3:40       ` Eli Zaretskii
  2023-10-08 23:03       ` Yuan Fu
  1 sibling, 2 replies; 11+ messages in thread
From: Yuan Fu @ 2023-09-28  0:23 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 66223



> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry@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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-28  0:23     ` Yuan Fu
@ 2023-09-28  3:40       ` Eli Zaretskii
  2023-09-28  4:55         ` Yuan Fu
  2023-10-08 23:03       ` Yuan Fu
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2023-09-28  3:40 UTC (permalink / raw)
  To: 66223

On September 28, 2023 2:23:41 AM GMT+02:00, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> > On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry@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.







^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-28  3:40       ` Eli Zaretskii
@ 2023-09-28  4:55         ` Yuan Fu
  0 siblings, 0 replies; 11+ messages in thread
From: Yuan Fu @ 2023-09-28  4:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66223



> On Sep 27, 2023, at 8:40 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> On September 28, 2023 2:23:41 AM GMT+02:00, Yuan Fu <casouri@gmail.com> wrote:
>> 
>> 
>>> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry@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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-09-28  0:23     ` Yuan Fu
  2023-09-28  3:40       ` Eli Zaretskii
@ 2023-10-08 23:03       ` Yuan Fu
  2023-10-08 23:05         ` Dmitry Gutov
  1 sibling, 1 reply; 11+ messages in thread
From: Yuan Fu @ 2023-10-08 23:03 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 66223



> On Sep 27, 2023, at 5:23 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Sep 27, 2023, at 1:51 AM, Dmitry Gutov <dmitry@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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-10-08 23:03       ` Yuan Fu
@ 2023-10-08 23:05         ` Dmitry Gutov
  2023-10-09  5:04           ` Yuan Fu
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Gutov @ 2023-10-08 23:05 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 66223

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!





^ permalink raw reply	[flat|nested] 11+ messages in thread

* bug#66223: treesit-major-mode-setup should not call font-lock-mode
  2023-10-08 23:05         ` Dmitry Gutov
@ 2023-10-09  5:04           ` Yuan Fu
  0 siblings, 0 replies; 11+ messages in thread
From: Yuan Fu @ 2023-10-09  5:04 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 66223-done



> On Oct 8, 2023, at 4:05 PM, Dmitry Gutov <dmitry@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




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-10-09  5:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-27  0:17 bug#66223: treesit-major-mode-setup should not call font-lock-mode Dmitry Gutov
2023-09-27  7:21 ` Yuan Fu
2023-09-27  8:51   ` Dmitry Gutov
2023-09-27 11:15     ` Stefan Kangas
2023-09-27 19:42       ` Dmitry Gutov
2023-09-28  0:23     ` Yuan Fu
2023-09-28  3:40       ` Eli Zaretskii
2023-09-28  4:55         ` Yuan Fu
2023-10-08 23:03       ` Yuan Fu
2023-10-08 23:05         ` Dmitry Gutov
2023-10-09  5:04           ` Yuan Fu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).