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