GNU bug report logs - #64647
treesit-query-error due to a recent change to tree-sitter-javascript grammar definition

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 64647 in the body.
You can then email your comments to 64647 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: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: treesit-query-error due to a recent change to tree-sitter-javascript
 grammar definition
Date: Sat, 15 Jul 2023 14:34:29 +0200

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

Hi, 
this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the definition 
of the JSX grammar to tree-sitter-javascript. This causes a node type error:
"
Error while displaying: (jit-lock-function 1) reported (treesit-query-error 
"Node type error at" 24 "(jsx_opening_element [(nested_identifier (identifier)) 
(identifier)] @font-lock-function-call-face) (jsx_closing_element 
[(nested_identifier (identifier)) (identifier)] @font- lock-function-call-face) 
(jsx_self_closing_element [(nested_identifier (identifier)) (identifier)] @font-
lock-function-call-face) (jsx_attribute (property_identifier) @font-lock-
constant-face)" "Debug the query with `treesit-query-validate'")
"
Indentation also has problems due to the deletion of "jsx_fragment" definition.

The patch in attachment fixes both problems.
Thank you
Vincenzo

p.s. nvim-treesitter tries to limit these problems by indicating which commit 
to install. Does it make sense to try a similar approach with emacs as well? 

[0001-Updated-JSX-support-due-to-changes-in-tree-sitter-ja.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>,
 Theodor Thornhill <theo <at> thornhill.no>,
 Jostein Kjonigsen <jostein <at> kjonigsen.net>
Cc: 64647 <at> debbugs.gnu.org
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 15:57:46 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Sat, 15 Jul 2023 14:34:29 +0200
> 
> this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the definition 
> of the JSX grammar to tree-sitter-javascript. This causes a node type error:
> "
> Error while displaying: (jit-lock-function 1) reported (treesit-query-error 
> "Node type error at" 24 "(jsx_opening_element [(nested_identifier (identifier)) 
> (identifier)] @font-lock-function-call-face) (jsx_closing_element 
> [(nested_identifier (identifier)) (identifier)] @font- lock-function-call-face) 
> (jsx_self_closing_element [(nested_identifier (identifier)) (identifier)] @font-
> lock-function-call-face) (jsx_attribute (property_identifier) @font-lock-
> constant-face)" "Debug the query with `treesit-query-validate'")
> "
> Indentation also has problems due to the deletion of "jsx_fragment" definition.
> 
> The patch in attachment fixes both problems.

Will the patch work with the grammar libraries before the recent
change?

> p.s. nvim-treesitter tries to limit these problems by indicating which commit 
> to install. Does it make sense to try a similar approach with emacs as well? 

I think it is better if we make the code work with as many versions as
possible, by checking whether a feature exists before using it.

Theo, Jostein: any comments or ideas?

Thanks.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, Vincenzo Pupillo <v.pupillo <at> gmail.com>,
 Jostein Kjonigsen <jostein <at> kjonigsen.net>
Cc: 64647 <at> debbugs.gnu.org
Subject: Re: bug#64647: treesit-query-error due to a recent change to tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 15:23:17 +0200

On 15 July 2023 14:57:46 CEST, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> Date: Sat, 15 Jul 2023 14:34:29 +0200
>> 
>> this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the definition 
>> of the JSX grammar to tree-sitter-javascript. This causes a node type error:
>> "
>> Error while displaying: (jit-lock-function 1) reported (treesit-query-error 
>> "Node type error at" 24 "(jsx_opening_element [(nested_identifier (identifier)) 
>> (identifier)] @font-lock-function-call-face) (jsx_closing_element 
>> [(nested_identifier (identifier)) (identifier)] @font- lock-function-call-face) 
>> (jsx_self_closing_element [(nested_identifier (identifier)) (identifier)] @font-
>> lock-function-call-face) (jsx_attribute (property_identifier) @font-lock-
>> constant-face)" "Debug the query with `treesit-query-validate'")
>> "
>> Indentation also has problems due to the deletion of "jsx_fragment" definition.
>> 
>> The patch in attachment fixes both problems.
>
>Will the patch work with the grammar libraries before the recent
>change?
>
>> p.s. nvim-treesitter tries to limit these problems by indicating which commit 
>> to install. Does it make sense to try a similar approach with emacs as well? 
>
>I think it is better if we make the code work with as many versions as
>possible, by checking whether a feature exists before using it.
>
>Theo, Jostein: any comments or ideas?
>
>Thanks.

I'll look into it tonight - thanks for the heads up!

Theo

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, Vincenzo Pupillo <v.pupillo <at> gmail.com>,
 Jostein Kjonigsen <jostein <at> kjonigsen.net>
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 19:54:03 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> Date: Sat, 15 Jul 2023 14:34:29 +0200
>> 
>> this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the definition 
>> of the JSX grammar to tree-sitter-javascript. This causes a node type error:
>> "
>> Error while displaying: (jit-lock-function 1) reported (treesit-query-error 
>> "Node type error at" 24 "(jsx_opening_element [(nested_identifier (identifier)) 
>> (identifier)] @font-lock-function-call-face) (jsx_closing_element 
>> [(nested_identifier (identifier)) (identifier)] @font- lock-function-call-face) 
>> (jsx_self_closing_element [(nested_identifier (identifier)) (identifier)] @font-
>> lock-function-call-face) (jsx_attribute (property_identifier) @font-lock-
>> constant-face)" "Debug the query with `treesit-query-validate'")
>> "
>> Indentation also has problems due to the deletion of "jsx_fragment" definition.
>> 
>> The patch in attachment fixes both problems.
>
> Will the patch work with the grammar libraries before the recent
> change?
>

It will introduce regressions, but the patch itself is a change for the
better, both in emacs land and in the grammar itself. 

>> p.s. nvim-treesitter tries to limit these problems by indicating which commit 
>> to install. Does it make sense to try a similar approach with emacs as well? 
>
> I think it is better if we make the code work with as many versions as
> possible, by checking whether a feature exists before using it.
>
> Theo, Jostein: any comments or ideas?
>
> Thanks.


I don't disagree, but I think this is a difficult problem to solve, but
with an easy cop-out solution that most other implementors use - just
refer to the last supported commit. We've had some discussions on this,
but IIRC we never settled on anything. Personally, I think a

;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30

header may be the simplest way to at least signal some awareness
here. That way the auto install mechanism can pull that hash directly
and we can ensure some sort of compatibility checking.

What do you think?

@Vicenzo, seeing as this change only targets the JSX variant in
js-ts-mode, could you possibly also make the according changes to
tsx-ts-mode as well?

Thanks,
Theo

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, v.pupillo <at> gmail.com, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 22:16:42 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: Vincenzo Pupillo <v.pupillo <at> gmail.com>,  Jostein Kjonigsen
>  <jostein <at> kjonigsen.net>,  64647 <at> debbugs.gnu.org
> Date: Sat, 15 Jul 2023 19:54:03 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> The patch in attachment fixes both problems.
> >
> > Will the patch work with the grammar libraries before the recent
> > change?
> >
> 
> It will introduce regressions, but the patch itself is a change for the
> better, both in emacs land and in the grammar itself. 

What kinds of regressions?

> I don't disagree, but I think this is a difficult problem to solve, but
> with an easy cop-out solution that most other implementors use - just
> refer to the last supported commit. We've had some discussions on this,
> but IIRC we never settled on anything. Personally, I think a
> 
> ;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30
> 
> header may be the simplest way to at least signal some awareness
> here. That way the auto install mechanism can pull that hash directly
> and we can ensure some sort of compatibility checking.
> 
> What do you think?

I think what I wrote: that we should try to make our modes work with
reasonably old versions of the grammars, if that is practical.  While
in general it could be a very difficult, if not impossible, to achieve
that, the question is whether this particular issue can be solved in
that manner.  If it can, we should do it.

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, Jostein Kjonigsen <jostein <at> kjonigsen.net>
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 21:17:54 +0200

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

Hi Theo,

In data sabato 15 luglio 2023 19:54:03 CEST, Theodor Thornhill ha scritto:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> >> Date: Sat, 15 Jul 2023 14:34:29 +0200
> >> 
> >> this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the
> >> definition of the JSX grammar to tree-sitter-javascript. This causes a
> >> node type error: "
> >> Error while displaying: (jit-lock-function 1) reported
> >> (treesit-query-error
> >> "Node type error at" 24 "(jsx_opening_element [(nested_identifier
> >> (identifier)) (identifier)] @font-lock-function-call-face)
> >> (jsx_closing_element
> >> [(nested_identifier (identifier)) (identifier)] @font-
> >> lock-function-call-face) (jsx_self_closing_element [(nested_identifier
> >> (identifier)) (identifier)] @font- lock-function-call-face)
> >> (jsx_attribute (property_identifier) @font-lock- constant-face)" "Debug
> >> the query with `treesit-query-validate'")
> >> "
> >> Indentation also has problems due to the deletion of "jsx_fragment"
> >> definition.
> >> 
> >> The patch in attachment fixes both problems.
> > 
> > Will the patch work with the grammar libraries before the recent
> > change?
> 
> It will introduce regressions, but the patch itself is a change for the
> better, both in emacs land and in the grammar itself.
> 
> >> p.s. nvim-treesitter tries to limit these problems by indicating which
> >> commit to install. Does it make sense to try a similar approach with
> >> emacs as well?> 
> > I think it is better if we make the code work with as many versions as
> > possible, by checking whether a feature exists before using it.
> > 
> > Theo, Jostein: any comments or ideas?
> > 
> > Thanks.
> 
> I don't disagree, but I think this is a difficult problem to solve, but
> with an easy cop-out solution that most other implementors use - just
> refer to the last supported commit. We've had some discussions on this,
> but IIRC we never settled on anything. Personally, I think a
> 
> ;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30
> 
> header may be the simplest way to at least signal some awareness
> here. That way the auto install mechanism can pull that hash directly
> and we can ensure some sort of compatibility checking.
> 
> What do you think?
> 
> @Vicenzo, seeing as this change only targets the JSX variant in
> js-ts-mode, could you possibly also make the according changes to
> tsx-ts-mode as well?
Yes, and I also attached the previous one with a corrected commit message ((I 
had written js-ts-mode.el instead of js.el)

Thanks, 
Vincenzo

[0001-Updated-JSX-support-due-to-changes-in-tree-sitter-ja.patch (text/x-patch, attachment)]

[0002-Updated-JSX-support-due-to-changes-in-tree-sitter-ty.patch (text/x-patch, attachment)]

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 21:39:49 +0200

In data sabato 15 luglio 2023 21:16:42 CEST, Eli Zaretskii ha scritto:
> > From: Theodor Thornhill <theo <at> thornhill.no>
> > Cc: Vincenzo Pupillo <v.pupillo <at> gmail.com>,  Jostein Kjonigsen
> > 
> >  <jostein <at> kjonigsen.net>,  64647 <at> debbugs.gnu.org
> > 
> > Date: Sat, 15 Jul 2023 19:54:03 +0200
> > 
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> > >> The patch in attachment fixes both problems.
> > > 
> > > Will the patch work with the grammar libraries before the recent
> > > change?
> > 
> > It will introduce regressions, but the patch itself is a change for the
> > better, both in emacs land and in the grammar itself.
> 
> What kinds of regressions?
> 
> > I don't disagree, but I think this is a difficult problem to solve, but
> > with an easy cop-out solution that most other implementors use - just
> > refer to the last supported commit. We've had some discussions on this,
> > but IIRC we never settled on anything. Personally, I think a
> > 
> > ;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30
> > 
> > header may be the simplest way to at least signal some awareness
> > here. That way the auto install mechanism can pull that hash directly
> > and we can ensure some sort of compatibility checking.
> > 
> > What do you think?
> 
> I think what I wrote: that we should try to make our modes work with
> reasonably old versions of the grammars, if that is practical.  While
> in general it could be a very difficult, if not impossible, to achieve
> that, the question is whether this particular issue can be solved in
> that manner.  If it can, we should do it.
I can rewrite both patches in the same way I had patched java-ts-mode. 
Basically, there are only two changes. I think it would be useful to have a 
function to check whether grammar features are supported or not. For example, 
a specialized version of treesit-query-validate that could also be used in 
interactive mode (to simplify the development).

Do I rewrite the patches?

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 22:45:31 +0200

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> In data sabato 15 luglio 2023 21:16:42 CEST, Eli Zaretskii ha scritto:
>> > From: Theodor Thornhill <theo <at> thornhill.no>
>> > Cc: Vincenzo Pupillo <v.pupillo <at> gmail.com>,  Jostein Kjonigsen
>> > 
>> >  <jostein <at> kjonigsen.net>,  64647 <at> debbugs.gnu.org
>> > 
>> > Date: Sat, 15 Jul 2023 19:54:03 +0200
>> > 
>> > Eli Zaretskii <eliz <at> gnu.org> writes:
>> > >> The patch in attachment fixes both problems.
>> > > 
>> > > Will the patch work with the grammar libraries before the recent
>> > > change?
>> > 
>> > It will introduce regressions, but the patch itself is a change for the
>> > better, both in emacs land and in the grammar itself.
>> 
>> What kinds of regressions?
>> 

Because the nodes seems to have been removed/swapped in for new ones, we
will lose the functionality for people using versions < bb1f97b6.

>> > I don't disagree, but I think this is a difficult problem to solve, but
>> > with an easy cop-out solution that most other implementors use - just
>> > refer to the last supported commit. We've had some discussions on this,
>> > but IIRC we never settled on anything. Personally, I think a
>> > 
>> > ;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30
>> > 
>> > header may be the simplest way to at least signal some awareness
>> > here. That way the auto install mechanism can pull that hash directly
>> > and we can ensure some sort of compatibility checking.
>> > 
>> > What do you think?
>> 
>> I think what I wrote: that we should try to make our modes work with
>> reasonably old versions of the grammars, if that is practical.  While
>> in general it could be a very difficult, if not impossible, to achieve
>> that, the question is whether this particular issue can be solved in
>> that manner.  If it can, we should do it.

Yeah, I think we can do that in this case. I'm just wondering whether
it's worth the effort or not. Should we introduce some notion of
"deprecated" tree sitter functionalities? Otherwise I guess we'll have
to maintain a growing set of compat-code, which over time will incur a
performance cost, and it may be difficult to remember what code is
compat-code and what isn't. 

> I can rewrite both patches in the same way I had patched java-ts-mode. 
> Basically, there are only two changes. I think it would be useful to have a 
> function to check whether grammar features are supported or not. For example, 
> a specialized version of treesit-query-validate that could also be used in 
> interactive mode (to simplify the development).
>
> Do I rewrite the patches?

I seem to have missed the java-ts-mode patch. Where is it? Do you have
such an implementation ready at hand? To me it sounds smart to have such
a function, and it sounds like it should be in treesit.el, but that
would probably be too late for emacs-29, I think?

Do you want to write such a function Vincenzo?

Theo

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Jostein Kjonigsen <jostein <at> kjonigsen.net>
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 15 Jul 2023 22:51:04 +0200

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> Hi Theo,
>
> In data sabato 15 luglio 2023 19:54:03 CEST, Theodor Thornhill ha scritto:
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> >> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> >> Date: Sat, 15 Jul 2023 14:34:29 +0200
>> >> 
>> >> this commit (bb1f97b643b77fc1f082d621bf533b4b14cf0c30) changed the
>> >> definition of the JSX grammar to tree-sitter-javascript. This causes a
>> >> node type error: "
>> >> Error while displaying: (jit-lock-function 1) reported
>> >> (treesit-query-error
>> >> "Node type error at" 24 "(jsx_opening_element [(nested_identifier
>> >> (identifier)) (identifier)] @font-lock-function-call-face)
>> >> (jsx_closing_element
>> >> [(nested_identifier (identifier)) (identifier)] @font-
>> >> lock-function-call-face) (jsx_self_closing_element [(nested_identifier
>> >> (identifier)) (identifier)] @font- lock-function-call-face)
>> >> (jsx_attribute (property_identifier) @font-lock- constant-face)" "Debug
>> >> the query with `treesit-query-validate'")
>> >> "
>> >> Indentation also has problems due to the deletion of "jsx_fragment"
>> >> definition.
>> >> 
>> >> The patch in attachment fixes both problems.
>> > 
>> > Will the patch work with the grammar libraries before the recent
>> > change?
>> 
>> It will introduce regressions, but the patch itself is a change for the
>> better, both in emacs land and in the grammar itself.
>> 
>> >> p.s. nvim-treesitter tries to limit these problems by indicating which
>> >> commit to install. Does it make sense to try a similar approach with
>> >> emacs as well?> 
>> > I think it is better if we make the code work with as many versions as
>> > possible, by checking whether a feature exists before using it.
>> > 
>> > Theo, Jostein: any comments or ideas?
>> > 
>> > Thanks.
>> 
>> I don't disagree, but I think this is a difficult problem to solve, but
>> with an easy cop-out solution that most other implementors use - just
>> refer to the last supported commit. We've had some discussions on this,
>> but IIRC we never settled on anything. Personally, I think a
>> 
>> ;;; Tree-sitter-version: bb1f97b643b77fc1f082d621bf533b4b14cf0c30
>> 
>> header may be the simplest way to at least signal some awareness
>> here. That way the auto install mechanism can pull that hash directly
>> and we can ensure some sort of compatibility checking.
>> 
>> What do you think?
>> 
>> @Vicenzo, seeing as this change only targets the JSX variant in
>> js-ts-mode, could you possibly also make the according changes to
>> tsx-ts-mode as well?
> Yes, and I also attached the previous one with a corrected commit message ((I 
> had written js-ts-mode.el instead of js.el)
>
> Thanks, 
> Vincenzo

Thanks - the patches looks good to me, but let's see what we end up with
wrt backward-compatibility :)

Theo

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 07:48:37 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: jostein <at> kjonigsen.net, 64647 <at> debbugs.gnu.org
> Date: Sat, 15 Jul 2023 21:39:49 +0200
> 
> > I think what I wrote: that we should try to make our modes work with
> > reasonably old versions of the grammars, if that is practical.  While
> > in general it could be a very difficult, if not impossible, to achieve
> > that, the question is whether this particular issue can be solved in
> > that manner.  If it can, we should do it.
> I can rewrite both patches in the same way I had patched java-ts-mode. 

That would be good, thanks.

> Basically, there are only two changes. I think it would be useful to have a 
> function to check whether grammar features are supported or not. For example, 
> a specialized version of treesit-query-validate that could also be used in 
> interactive mode (to simplify the development).

Yes, a good idea.

> Do I rewrite the patches?

Yes, please.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, v.pupillo <at> gmail.com, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 08:13:43 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  64647 <at> debbugs.gnu.org,
>   jostein <at> kjonigsen.net
> Date: Sat, 15 Jul 2023 22:45:31 +0200
> 
> >> > > Will the patch work with the grammar libraries before the recent
> >> > > change?
> >> > 
> >> > It will introduce regressions, but the patch itself is a change for the
> >> > better, both in emacs land and in the grammar itself.
> >> 
> >> What kinds of regressions?
> >> 
> 
> Because the nodes seems to have been removed/swapped in for new ones, we
> will lose the functionality for people using versions < bb1f97b6.

That's what I thought.  I think we should try to avoid such
regressions if it's feasible.

> >> I think what I wrote: that we should try to make our modes work with
> >> reasonably old versions of the grammars, if that is practical.  While
> >> in general it could be a very difficult, if not impossible, to achieve
> >> that, the question is whether this particular issue can be solved in
> >> that manner.  If it can, we should do it.
> 
> Yeah, I think we can do that in this case. I'm just wondering whether
> it's worth the effort or not.

AFAIU, people tend to have old grammar libraries all the time.  For
example, we had a couple of bug reports for C or C++ which turned out
to be due to grammar libraries that are not the latest HEAD of their
respective repositories.  Some people tend to use only official
releases of those grammar libraries (for those which make releases;
many don't).  Also, you cannot upgrade the library while the Emacs
session which uses it is running, so people who have long-running
sessions and don't like to restart Emacs sometimes have no choice but
to stay with an old library for some time.

For these and other reasons, I think being less rigid in requiring the
latest grammar libraries will benefit our users.

> Should we introduce some notion of
> "deprecated" tree sitter functionalities? Otherwise I guess we'll have
> to maintain a growing set of compat-code, which over time will incur a
> performance cost, and it may be difficult to remember what code is
> compat-code and what isn't. 

I'm not afraid of compatibility code, as long as it's manageable.  We
could retire some of that as time passes.  But we are not yet at a
point where this compatibility code presents any significant issue, so
I'd rather we delayed the decision until we come to that bridge.

1> > I can rewrite both patches in the same way I had patched java-ts-mode. 
> > Basically, there are only two changes. I think it would be useful to have a 
> > function to check whether grammar features are supported or not. For example, 
> > a specialized version of treesit-query-validate that could also be used in 
> > interactive mode (to simplify the development).
> >
> > Do I rewrite the patches?
> 
> I seem to have missed the java-ts-mode patch. Where is it? Do you have
> such an implementation ready at hand? To me it sounds smart to have such
> a function, and it sounds like it should be in treesit.el, but that
> would probably be too late for emacs-29, I think?

A function will probably go to master, but an ad-hoc compatibility fix
that doesn't regress for older grammar libraries would be welcome on
emacs-29 (assuming it is not very complicated).

Thanks.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, v.pupillo <at> gmail.com, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 10:38:37 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> 
>> Yeah, I think we can do that in this case. I'm just wondering whether
>> it's worth the effort or not.
>
> AFAIU, people tend to have old grammar libraries all the time.  For
> example, we had a couple of bug reports for C or C++ which turned out
> to be due to grammar libraries that are not the latest HEAD of their
> respective repositories.  Some people tend to use only official
> releases of those grammar libraries (for those which make releases;
> many don't).  Also, you cannot upgrade the library while the Emacs
> session which uses it is running, so people who have long-running
> sessions and don't like to restart Emacs sometimes have no choice but
> to stay with an old library for some time.
>
> For these and other reasons, I think being less rigid in requiring the
> latest grammar libraries will benefit our users.
>

Sure!

>> Should we introduce some notion of
>> "deprecated" tree sitter functionalities? Otherwise I guess we'll have
>> to maintain a growing set of compat-code, which over time will incur a
>> performance cost, and it may be difficult to remember what code is
>> compat-code and what isn't. 
>
> I'm not afraid of compatibility code, as long as it's manageable.  We
> could retire some of that as time passes.  But we are not yet at a
> point where this compatibility code presents any significant issue, so
> I'd rather we delayed the decision until we come to that bridge.
>
> 1> > I can rewrite both patches in the same way I had patched java-ts-mode. 
>> > Basically, there are only two changes. I think it would be useful to have a 
>> > function to check whether grammar features are supported or not. For example, 
>> > a specialized version of treesit-query-validate that could also be used in 
>> > interactive mode (to simplify the development).
>> >
>> > Do I rewrite the patches?
>> 
>> I seem to have missed the java-ts-mode patch. Where is it? Do you have
>> such an implementation ready at hand? To me it sounds smart to have such
>> a function, and it sounds like it should be in treesit.el, but that
>> would probably be too late for emacs-29, I think?
>
> A function will probably go to master, but an ad-hoc compatibility fix
> that doesn't regress for older grammar libraries would be welcome on
> emacs-29 (assuming it is not very complicated).
>
> Thanks.

Sure - unless Vincenzo wants to tackle it, I can look into creating a
function for master to check for available features.

Vincenzo, do you want to add compat-code to emacs-29 and your patches?
I can take care of the in-core function in a separate bug, if that makes
sense :)

Theo

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 20:00:43 +0200

In data domenica 16 luglio 2023 10:38:37 CEST, Theodor Thornhill ha scritto:
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >> Yeah, I think we can do that in this case. I'm just wondering whether
> >> it's worth the effort or not.
> > 
> > AFAIU, people tend to have old grammar libraries all the time.  For
> > example, we had a couple of bug reports for C or C++ which turned out
> > to be due to grammar libraries that are not the latest HEAD of their
> > respective repositories.  Some people tend to use only official
> > releases of those grammar libraries (for those which make releases;
> > many don't).  Also, you cannot upgrade the library while the Emacs
> > session which uses it is running, so people who have long-running
> > sessions and don't like to restart Emacs sometimes have no choice but
> > to stay with an old library for some time.
> > 
> > For these and other reasons, I think being less rigid in requiring the
> > latest grammar libraries will benefit our users.
> 
> Sure!
> 
> >> Should we introduce some notion of
> >> "deprecated" tree sitter functionalities? Otherwise I guess we'll have
> >> to maintain a growing set of compat-code, which over time will incur a
> >> performance cost, and it may be difficult to remember what code is
> >> compat-code and what isn't.
> > 
> > I'm not afraid of compatibility code, as long as it's manageable.  We
> > could retire some of that as time passes.  But we are not yet at a
> > point where this compatibility code presents any significant issue, so
> > I'd rather we delayed the decision until we come to that bridge.
> > 
> > 1> > I can rewrite both patches in the same way I had patched
> > java-ts-mode.
> > 
> >> > Basically, there are only two changes. I think it would be useful to
> >> > have a
> >> > function to check whether grammar features are supported or not. For
> >> > example, a specialized version of treesit-query-validate that could
> >> > also be used in interactive mode (to simplify the development).
> >> > 
> >> > Do I rewrite the patches?
> >> 
> >> I seem to have missed the java-ts-mode patch. Where is it? Do you have
> >> such an implementation ready at hand? To me it sounds smart to have such
> >> a function, and it sounds like it should be in treesit.el, but that
> >> would probably be too late for emacs-29, I think?
> > 
> > A function will probably go to master, but an ad-hoc compatibility fix
> > that doesn't regress for older grammar libraries would be welcome on
> > emacs-29 (assuming it is not very complicated).
> > 
> > Thanks.
> 
> Sure - unless Vincenzo wants to tackle it, I can look into creating a
> function for master to check for available features.

 I am not familiar with the internal emacs treesitter binding. I would need 
some time to study it. However, I have seen that there are functions in the 
treesitter library that are not used by the current binding implementation. 
For example: ts_language_field_id_for_name could be useful to check whether a 
symbol is defined or not. 

In my patch for java-ts-mode I used treesit-query-capture to figure out whether 
a symbol was defined or not. Check out the java-ts-mode--string-highlight-
helper function.

> 
> Vincenzo, do you want to add compat-code to emacs-29 and your patches?

Okay. I think I can do it this week. Do I create a new file, like treesitter-
compat.el ?

> I can take care of the in-core function in a separate bug, if that makes
> sense :)
> 
> Theo

Vincenzo

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 21:19:38 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> Date: Sun, 16 Jul 2023 20:00:43 +0200
> 
> In my patch for java-ts-mode I used treesit-query-capture to figure out whether 
> a symbol was defined or not. Check out the java-ts-mode--string-highlight-
> helper function.

Can you do something similar in this case?  That would be good enough
for Emacs 29.1.

> > Vincenzo, do you want to add compat-code to emacs-29 and your patches?
> 
> Okay. I think I can do it this week. Do I create a new file, like treesitter-
> compat.el ?

This should be for master, I think.  I'd like to release Emacs 29.1
soon, so this more general job should be less urgent than fixing the
typescript modes to work with the latest grammars.

Thanks.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re:  Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 16 Jul 2023 20:56:15 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
>> Date: Sun, 16 Jul 2023 20:00:43 +0200
>> 
>> In my patch for java-ts-mode I used treesit-query-capture to figure out whether 
>> a symbol was defined or not. Check out the java-ts-mode--string-highlight-
>> helper function.
>
> Can you do something similar in this case?  That would be good enough
> for Emacs 29.1.
>
>> > Vincenzo, do you want to add compat-code to emacs-29 and your patches?
>> 
>> Okay. I think I can do it this week. Do I create a new file, like treesitter-
>> compat.el ?
>
> This should be for master, I think.  I'd like to release Emacs 29.1
> soon, so this more general job should be less urgent than fixing the
> typescript modes to work with the latest grammars.
>
> Thanks.

Yeah, I was thinking maybe we could just have a check in the major-mode
init, and appending the appropriate nodes into the
treesit-simple-indent-rules there, rather than in the variable
itself. That way we can avoid performance regressions in the indentation
code. That would mean to extract the nodes you changed in your current
patches, maybe add them to a new variable with a name of your choosing,
and conditionally add either-or when the mode inits. Does that make sense?

Thanks,
Theo

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Mon, 17 Jul 2023 23:24:20 +0200

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

Hi,

In data domenica 16 luglio 2023 20:19:38 CEST, Eli Zaretskii ha scritto:
> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> > Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> > Date: Sun, 16 Jul 2023 20:00:43 +0200
> > 
> > In my patch for java-ts-mode I used treesit-query-capture to figure out
> > whether a symbol was defined or not. Check out the
> > java-ts-mode--string-highlight- helper function.
> 
> Can you do something similar in this case?  That would be good enough
> for Emacs 29.1.

In attachment you can find the new version of the patches (similar to the patch 
that i made for java-ts-mode).
The patches were made on the branch emacs-29.  

Both work with the latest grammar. The javascript version is reliable, the 
typescrypt version seems reliable. In fact, drum roll, with typescrypt both 
tests:
1. (treesit-query-capture 'typescript '((member_expression) @capture)) ;; the 
new node type
2. (treesit-query-capture 'typescript '((nested_identifier) @capture)) ;; the 
old node type
both return nil !!!
If you use #2, then treesitter-ts-mode font lock gives an error!
This happens only for font-lock, while for indentation everything works 
correctly.

No problem for javascript, it works as expected for font-lock. The old node 
type returns an error with treesit-query-capture.

@Theo: I am not sure if the indentation works if I add the new rules to 
treesit-simple-indent-rules in the Init function. I am writing a major-mode 
for php and, unless I am mistaken or due to problems with the various 
treesitter parsers, the order seems to be important.

P.S. I opened two issues (one moth ago) for tree-sitter-php because it flags 
variables with names in non-Western characters as errors. tree-sitter-html, 
after they rewrote the parse from C++ to C, it crashed when used in 
conjunction with other parsers.

Sorry for the length of this email (and for my English)

Vincenzo


 

[0001-Updated-JSX-support-due-to-changes-in-tree-sitter-ja.patch (text/x-patch, attachment)]

[0002-Updated-JSX-support-due-to-changes-in-tree-sitter-ty.patch (text/x-patch, attachment)]

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Wed, 19 Jul 2023 07:11:05 +0200

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> Hi,
>
> In data domenica 16 luglio 2023 20:19:38 CEST, Eli Zaretskii ha scritto:
>> > From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> > Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
>> > Date: Sun, 16 Jul 2023 20:00:43 +0200
>> > 
>> > In my patch for java-ts-mode I used treesit-query-capture to figure out
>> > whether a symbol was defined or not. Check out the
>> > java-ts-mode--string-highlight- helper function.
>> 
>> Can you do something similar in this case?  That would be good enough
>> for Emacs 29.1.
>
> In attachment you can find the new version of the patches (similar to the patch 
> that i made for java-ts-mode).
> The patches were made on the branch emacs-29.  
>
> Both work with the latest grammar. The javascript version is reliable, the 
> typescrypt version seems reliable. In fact, drum roll, with typescrypt both 
> tests:
> 1. (treesit-query-capture 'typescript '((member_expression) @capture)) ;; the 
> new node type
> 2. (treesit-query-capture 'typescript '((nested_identifier) @capture)) ;; the 
> old node type
> both return nil !!!
> If you use #2, then treesitter-ts-mode font lock gives an error!
> This happens only for font-lock, while for indentation everything works 
> correctly.

For Typescript these changes should go into 'tsx-ts-mode, not
'typescript-ts-mode. That may be why you are seeing some strange results?

>
> No problem for javascript, it works as expected for font-lock. The old node 
> type returns an error with treesit-query-capture.
>
> @Theo: I am not sure if the indentation works if I add the new rules to 
> treesit-simple-indent-rules in the Init function. I am writing a major-mode 
> for php and, unless I am mistaken or due to problems with the various 
> treesitter parsers, the order seems to be important.

Thanks! Some minor comments (all of which apply to both patches, even
though the comment is only for one of them):

>  
> From c6a93b510378756f2eff01a11ef4f9127a5e5d17 Mon Sep 17 00:00:00 2001
> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Mon, 17 Jul 2023 22:20:44 +0200
> Subject: [PATCH] Updated JSX support due to changes in tree-sitter-javascript
>
> A recent change in tree-sitter-javascript grammar support for
> JSX (commit bb1f97b), changed two things:
> 1. renamed nested_identifier to member_expression
> 2. removed jsx_fragment, jsx_text is used instead
>
> * lisp/progmodes/js.el: (js--treesit-indent-helper): indent helper
>   function for handle different tree-sitter-javascript version
> * lisp/progmodes/js.el: (js--treesit-indent-rules): use the new
>   function
> * lisp/progmodes/js.el: (js--treesit-font-lock-helper): font lock
>   helper function for handle different tree-sitter-javascript version
> * lisp/progmodes/js.el: (js--treesit-font-lock-settings): use the new
>   function
> ---

"... function to handle ..."

>  lisp/progmodes/js.el | 65 ++++++++++++++++++++++++++++++++------------
>  1 file changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index a05bd758dbc..f5158195500 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -3427,6 +3427,18 @@ This function is intended for use in `after-change-functions'."
>  
>  ;;; Tree sitter integration
>  
> +(defun js--treesit-indent-helper ()
> +    "Indent rules helper, for handle different release of tree-sitter-javascript.
> +Check if a node type is available, then return the right indent rules."

"Indent rules helper, to handle different releases of tree-sitter-javascript."

> +    ;; handle commit bb1f97b
> +    (condition-case nil
> +        (progn (treesit-query-capture 'javascript '((jsx_fragment) @capture))
> +               `(((match "<" "jsx_fragment") parent 0)
> +                 ((parent-is "jsx_fragment") parent js-indent-level)))
> +      (error
> +       `(((match "<" "jsx_text") parent 0)
> +         ((parent-is "jsx_text") parent js-indent-level)))))
> +
>  (defvar js--treesit-indent-rules
>    (let ((switch-case (rx "switch_" (or "case" "default"))))
>      `((javascript
> @@ -3462,8 +3474,9 @@ This function is intended for use in `after-change-functions'."
>         ((parent-is "statement_block") parent-bol js-indent-level)
>  
>         ;; JSX
> -       ((match "<" "jsx_fragment") parent 0)
> -       ((parent-is "jsx_fragment") parent js-indent-level)
> +       ;; ((match "<" "jsx_fragment") parent 0)
> +       ;; ((parent-is "jsx_fragment") parent js-indent-level)
> +       (js--treesit-indent-helper)
>         ((node-is "jsx_closing_element") parent 0)
>         ((match "jsx_element" "statement") parent js-indent-level)
>         ((parent-is "jsx_element") parent js-indent-level)
> @@ -3490,6 +3503,36 @@ This function is intended for use in `after-change-functions'."
>      "&&" "||" "!")
>    "JavaScript operators for tree-sitter font-locking.")
>  
> +(defun js--treesit-font-lock-helper ()
> +  "Font lock rules helper, for handle different release of tree-sitter-javascript.
> +Check if a node type is available, then return the right font lock rules."

Same little comment here.

> +  ;; handle commit bb1f97b
> +  (condition-case nil
> +      (progn (treesit-query-capture 'javascript '((member_expression) @capture))
> +	     '((jsx_opening_element
> +		[(member_expression (identifier)) (identifier)]
> +		@font-lock-function-call-face)
> +
> +	       (jsx_closing_element
> +		[(member_expression (identifier)) (identifier)]
> +		@font-lock-function-call-face)
> +
> +	       (jsx_self_closing_element
> +		[(member_expression (identifier)) (identifier)]
> +		@font-lock-function-call-face)))
> +    (error
> +     '((jsx_opening_element
> +	[(nested_identifier (identifier)) (identifier)]
> +	@font-lock-function-call-face)
> +
> +       (jsx_closing_element
> +	[(nested_identifier (identifier)) (identifier)]
> +	@font-lock-function-call-face)
> +
> +       (jsx_self_closing_element
> +	[(nested_identifier (identifier)) (identifier)]
> +	@font-lock-function-call-face)))))
> +

The indentation here looks off. Can you format this?

>  (defvar js--treesit-font-lock-settings
>    (treesit-font-lock-rules
>  
> @@ -3599,21 +3642,9 @@ This function is intended for use in `after-change-functions'."
>  
>     :language 'javascript
>     :feature 'jsx
> -   '((jsx_opening_element
> -      [(nested_identifier (identifier)) (identifier)]
> -      @font-lock-function-call-face)
> -
> -     (jsx_closing_element
> -      [(nested_identifier (identifier)) (identifier)]
> -      @font-lock-function-call-face)
> -
> -     (jsx_self_closing_element
> -      [(nested_identifier (identifier)) (identifier)]
> -      @font-lock-function-call-face)
> -
> -     (jsx_attribute
> -      (property_identifier)
> -      @font-lock-constant-face))
> +   (append
> +    (js--treesit-font-lock-helper)
> +    '((jsx_attribute (property_identifier) @font-lock-constant-face)))
>  
>     :language 'javascript
>     :feature 'number
> -- 
> 2.41.0
>
> From 263c9f0eca3a7df7cb29306297d32358f0e6537c Mon Sep 17 00:00:00 2001
> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Date: Mon, 17 Jul 2023 22:32:13 +0200
> Subject: [PATCH] Updated JSX support due to changes in tree-sitter-typescript
>
> A recent change in tree-sitter-typescript grammar support for
> JSX (commit b893426), changed two things:
> 1. renamed nested_identifier to member_expression
> 2. removed jsx_fragment, jsx_text is used instead
>
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--indent-helper): indent helper
>   function for handle different tree-sitter-javascript version
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--indent-rules): use the new
>   function
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--font-lock-helper): font lock
>   helper function for handle different tree-sitter-javascript version
> * lisp/progmodes/typescript-ts-mode.el: (typescript-ts-mode--font-lock-settings): use the new
>   function
> ---
>  lisp/progmodes/typescript-ts-mode.el | 64 +++++++++++++++++++++-------
>  1 file changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
> index 5df34de0472..2de7587e43a 100644
> --- a/lisp/progmodes/typescript-ts-mode.el
> +++ b/lisp/progmodes/typescript-ts-mode.el
> @@ -75,6 +75,18 @@
>      table)
>    "Syntax table for `typescript-ts-mode'.")
>  

This seems to not be properly converted to tsx from javascript, both in
the docstring and code.  Also, I think the name is wrong. Maybe it
should describe its intent a little more closely, something like
"tsx-ts-mode--indent-compatibility-b893426"?

> +(defun typescript-ts-mode--indent-helper ()
          ^^^^^^^tsx-ts-mode
> +    "Indent rules helper, for handle different release of tree-sitter-typescript.
> +Check if a node type is available, then return the right indent rules."
> +    ;; handle commit b893426
> +    (condition-case nil
> +        (progn (treesit-query-capture 'javascript '((jsx_fragment) @capture))
                                         'tsx^^^^^^^, right?
> +               `(((match "<" "jsx_fragment") parent 0)
> +                 ((parent-is "jsx_fragment") parent typescript-ts-mode-indent-offset)))
> +      (error
> +       `(((match "<" "jsx_text") parent 0)
> +         ((parent-is "jsx_text") parent typescript-ts-mode-indent-offset)))))
> +
>  (defun typescript-ts-mode--indent-rules (language)
>    "Rules used for indentation.
>  Argument LANGUAGE is either `typescript' or `tsx'."
> @@ -110,8 +122,7 @@ Argument LANGUAGE is either `typescript' or `tsx'."
>       ((parent-is "binary_expression") parent-bol typescript-ts-mode-indent-offset)
>  
>       ,@(when (eq language 'tsx)
> -         `(((match "<" "jsx_fragment") parent 0)
> -           ((parent-is "jsx_fragment") parent typescript-ts-mode-indent-offset)
> +         `((typescript-ts-mode--indent-helper)
>             ((node-is "jsx_closing_element") parent 0)
>             ((match "jsx_element" "statement") parent typescript-ts-mode-indent-offset)
>             ((parent-is "jsx_element") parent typescript-ts-mode-indent-offset)
> @@ -142,6 +153,39 @@ Argument LANGUAGE is either `typescript' or `tsx'."
>      "&&" "||" "!" "?.")
>    "TypeScript operators for tree-sitter font-locking.")
>  

Same naming comment here, and the language is wrong. It should be 'tsx,
not 'typescript. 

> +(defun typescript-ts-mode--font-lock-helper ()
> +  "Font lock rules helper, for handle different release of tree-sitter-typescript.
> +Check if a node type is available, then return the right font lock rules."
> +  ;; handle commit bb1f97b
> +  ;; Warning: treesitter-query-capture says both node types are valid,
> +  ;; but then raises an error if the wrong node type is used. So it is
> +  ;; important to check with the new node type (member_expression)
> +  (condition-case nil
> +      (progn (treesit-query-capture 'typescript '((member_expression) @capture))
> +	     '((jsx_opening_element
> +		[(member_expression (identifier)) (identifier)]
> +		@typescript-ts-jsx-tag-face)
> +
> +	       (jsx_closing_element
> +		[(member_expression (identifier)) (identifier)]
> +		@typescript-ts-jsx-tag-face)
> +
> +	       (jsx_self_closing_element
> +		[(member_expression (identifier)) (identifier)]
> +		@typescript-ts-jsx-tag-face)))
> +    (error
> +     '((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)))))
> +
>  (defun typescript-ts-mode--font-lock-settings (language)
>    "Tree-sitter font-lock settings.
>  Argument LANGUAGE is either `typescript' or `tsx'."
> @@ -293,19 +337,9 @@ Argument LANGUAGE is either `typescript' or `tsx'."
>  
>     :language language
>     :feature 'jsx
> -   `((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))
> +   (append
> +    (typescript-ts-mode--font-lock-helper)
> +    `((jsx_attribute (property_identifier) @typescript-ts-jsx-attribute-face)))
>  
>     :language language
>     :feature 'number
> -- 
> 2.41.0

Thanks for the patch :)

Theo

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Thu, 20 Jul 2023 12:14:07 +0200

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

In data mercoledì 19 luglio 2023 07:11:05 CEST, Theodor Thornhill ha scritto:

> For Typescript these changes should go into 'tsx-ts-mode, not
> 'typescript-ts-mode. That may be why you are seeing some strange results?
No, exactly the same problem occurs, unfortunately. After all, the sources of 
libtree-sitter-tsx and libtree-sitter-typescript come from the same repository 
and the content of  tree-sitter-typescript/tsx/grammar.js is just that:
const defineGrammar = require('../common/define-grammar');
module.exports = defineGrammar('tsx');


> > ---
> 
> "... function to handle ..."

fixed


> "Indent rules helper, to handle different releases of
> tree-sitter-javascript."

fixed


> 
> The indentation here looks off. Can you format this?
> 

fixed

> 
> This seems to not be properly converted to tsx from javascript, both in
> the docstring and code.  Also, I think the name is wrong. Maybe it
> should describe its intent a little more closely, something like
> "tsx-ts-mode--indent-compatibility-b893426"?


Sorry for the error. I fixed them, tested and fixed the function names according to your instructions (also in js.el)
Hope the patches are better now.
Thanks.
Vincenzo

[0002-Updated-TSX-support-due-to-changes-in-tree-sitter-ty.patch (text/x-patch, attachment)]

[0001-Updated-JSX-support-due-to-changes-in-tree-sitter-ja.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net, theo <at> thornhill.no
Subject: Re: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 09:41:04 +0300

> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> Date: Thu, 20 Jul 2023 12:14:07 +0200
> 
> Sorry for the error. I fixed them, tested and fixed the function names according to your instructions (also in js.el)
> Hope the patches are better now.

Thanks.

Theo, Yuan, Jostein: any further comments?  If not, I'd like to
install this on the release branch soon.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, Vincenzo Pupillo <v.pupillo <at> gmail.com>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re:  Re: bug#64647: treesit-query-error due to a recent change to tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 09:29:26 +0200

On 22 July 2023 08:41:04 CEST, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
>> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
>> Date: Thu, 20 Jul 2023 12:14:07 +0200
>> 
>> Sorry for the error. I fixed them, tested and fixed the function names according to your instructions (also in js.el)
>> Hope the patches are better now.
>
>Thanks.
>
>Theo, Yuan, Jostein: any further comments?  If not, I'd like to
>install this on the release branch soon.

I'll look at it in a few hours, then install. Is that ok?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, v.pupillo <at> gmail.com, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 11:52:13 +0300

> Date: Sat, 22 Jul 2023 09:29:26 +0200
> From: Theodor Thornhill <theo <at> thornhill.no>
> CC: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> 
> 
> 
> On 22 July 2023 08:41:04 CEST, Eli Zaretskii <eliz <at> gnu.org> wrote:
> >> From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
> >> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> >> Date: Thu, 20 Jul 2023 12:14:07 +0200
> >> 
> >> Sorry for the error. I fixed them, tested and fixed the function names according to your instructions (also in js.el)
> >> Hope the patches are better now.
> >
> >Thanks.
> >
> >Theo, Yuan, Jostein: any further comments?  If not, I'd like to
> >install this on the release branch soon.
> 
> I'll look at it in a few hours, then install. Is that ok?

Yes, installing this on the release branch is OK.

Thanks.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 13:56:04 +0200

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> In data mercoledì 19 luglio 2023 07:11:05 CEST, Theodor Thornhill ha scritto:
>
>> For Typescript these changes should go into 'tsx-ts-mode, not
>> 'typescript-ts-mode. That may be why you are seeing some strange results?
> No, exactly the same problem occurs, unfortunately. After all, the sources of 
> libtree-sitter-tsx and libtree-sitter-typescript come from the same repository 
> and the content of  tree-sitter-typescript/tsx/grammar.js is just that:
> const defineGrammar = require('../common/define-grammar');
> module.exports = defineGrammar('tsx');
>
>
>> > ---
>> 
>> "... function to handle ..."
>
> fixed

Thanks for the fixes :)

>
>
> Sorry for the error. I fixed them, tested and fixed the function names according to your instructions (also in js.el)
> Hope the patches are better now.
> Thanks.
> Vincenzo
>

No worries! Am I correct in that this patch should be applied as well?
If you agree, I'll just apply it myself, no need to make a new patch.


Theo

diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index 173ec52f209..39fcd1de30e 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -75,10 +75,10 @@ typescript-ts-mode--syntax-table
     table)
   "Syntax table for `typescript-ts-mode'.")
 
-(defun tsx-ts-mode--indent-compatibility-b893426 ()
+(defun tsx-ts-mode--indent-compatibility-bb1f97b ()
   "Indent rules helper, to handle different releases of tree-sitter-tsx.
 Check if a node type is available, then return the right indent rules."
-  ;; handle commit b893426
+  ;; handle commit bb1f97b
   (condition-case nil
       (progn (treesit-query-capture 'tsx '((jsx_fragment) @capture))
              `(((match "<" "jsx_fragment") parent 0)
@@ -122,7 +122,7 @@ typescript-ts-mode--indent-rules
      ((parent-is "binary_expression") parent-bol typescript-ts-mode-indent-offset)
 
      ,@(when (eq language 'tsx)
-	 (append (tsx-ts-mode--indent-compatibility-b893426)
+	 (append (tsx-ts-mode--indent-compatibility-bb1f97b)
 		 `(((node-is "jsx_closing_element") parent 0)
 		   ((match "jsx_element" "statement") parent typescript-ts-mode-indent-offset)
 		   ((parent-is "jsx_element") parent typescript-ts-mode-indent-offset)

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

From: Vincenzo Pupillo <v.pupillo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 16:10:24 +0200

Hi Theo,
In data sabato 22 luglio 2023 13:56:04 CEST, Theodor Thornhill ha scritto:
> 
> Thanks for the fixes :)
>
Thanks you for the help.
 
> > Sorry for the error. I fixed them, tested and fixed the function names
> > according to your instructions (also in js.el) Hope the patches are
> > better now.
> > Thanks.
> > Vincenzo
> 
> No worries! Am I correct in that this patch should be applied as well?
 
The changes were made with two different patches. The one that modifies the 
indentation is this one: 
https://github.com/tree-sitter/tree-sitter-typescript/commit/
b893426b82492e59388a326b824a346d829487e8

Thanks.
Ciao
V.

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Vincenzo Pupillo <v.pupillo <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 23:22:44 +0200

Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:

> Hi Theo,
> In data sabato 22 luglio 2023 13:56:04 CEST, Theodor Thornhill ha scritto:
>> 
>> Thanks for the fixes :)
>>
> Thanks you for the help.
>  
>> > Sorry for the error. I fixed them, tested and fixed the function names
>> > according to your instructions (also in js.el) Hope the patches are
>> > better now.
>> > Thanks.
>> > Vincenzo
>> 
>> No worries! Am I correct in that this patch should be applied as well?
>  
> The changes were made with two different patches. The one that modifies the 
> indentation is this one: 
> https://github.com/tree-sitter/tree-sitter-typescript/commit/
> b893426b82492e59388a326b824a346d829487e8
>
> Thanks.
> Ciao
> V.

Great! Installed and pushed the patches to emacs-29. Thanks, Vincenzo!

Theo

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

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Vincenzo Pupillo <v.pupillo <at> gmail.com>, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sat, 22 Jul 2023 15:59:27 -0700

> On Jul 22, 2023, at 2:22 PM, Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs <at> gnu.org> wrote:
> 
> Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:
> 
>> Hi Theo,
>> In data sabato 22 luglio 2023 13:56:04 CEST, Theodor Thornhill ha scritto:
>>> 
>>> Thanks for the fixes :)
>>> 
>> Thanks you for the help.
>> 
>>>> Sorry for the error. I fixed them, tested and fixed the function names
>>>> according to your instructions (also in js.el) Hope the patches are
>>>> better now.
>>>> Thanks.
>>>> Vincenzo
>>> 
>>> No worries! Am I correct in that this patch should be applied as well?
>> 
>> The changes were made with two different patches. The one that modifies the 
>> indentation is this one: 
>> https://github.com/tree-sitter/tree-sitter-typescript/commit/
>> b893426b82492e59388a326b824a346d829487e8
>> 
>> Thanks.
>> Ciao
>> V.
> 
> Great! Installed and pushed the patches to emacs-29. Thanks, Vincenzo!
> 
> Theo

Thanks Theo and Vincenzo :-)

Yuan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64647-done <at> debbugs.gnu.org, v.pupillo <at> gmail.com, jostein <at> kjonigsen.net
Subject: Re: bug#64647: treesit-query-error due to a recent change to
 tree-sitter-javascript grammar definition
Date: Sun, 23 Jul 2023 08:17:56 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 64647 <at> debbugs.gnu.org, jostein <at> kjonigsen.net
> Date: Sat, 22 Jul 2023 23:22:44 +0200
> 
> Vincenzo Pupillo <v.pupillo <at> gmail.com> writes:
> 
> > The changes were made with two different patches. The one that modifies the 
> > indentation is this one: 
> > https://github.com/tree-sitter/tree-sitter-typescript/commit/
> > b893426b82492e59388a326b824a346d829487e8
> >
> > Thanks.
> > Ciao
> > V.
> 
> Great! Installed and pushed the patches to emacs-29. Thanks, Vincenzo!

Thank you all for a fast and efficient solution.

(The commit needed a small fixup, though.)

Closing the bug.

Previous Next

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