bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

There's no way to tell if a node belongs to a now-deleted
parser. Checking if it is `missing' or `outdated' returns nil; there
is no way to ascertain this state except by catching an error if you
try to get its node text, type, etc.




In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.20, cairo version 1.16.0) of 2023-01-25 built on mickey-work
Repository revision: 8b87d095acfb23b527f955873a59dd9c13ffc9b4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.3 LTS

Configured using:
 'configure --with-native-compilation --with-json --with-mailutils
 --without-compress-install --with-imagemagick CC=gcc-10'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2
M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP
SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE
XIM XINPUT2 XPM GTK3 ZLIB

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]


Mickey Petersen <mickey@masteringemacs.org> writes:

> There's no way to tell if a node belongs to a now-deleted
> parser. Checking if it is `missing' or `outdated' returns nil; there
> is no way to ascertain this state except by catching an error if you
> try to get its node text, type, etc.

That sounds reasonable. I can add treesit-parser-live-p, and add
a "live" PROPERTY to treesit-node-check.

treesit-parser-live-p only returns t when the parser is not deleted AND
its buffer is live. I assume that’s more useful than just checking
whether the parser is deleted, for whatever usecase you have?

Eli, should we add this in the release branch or Emacs 30? This
functionality is possible to implement in Elisp, albeit a bit clunky. I
attached a patch of how would it look like (without documentation/test).

Yuan


[-- Attachment #2: livep.patch --]
[-- Type: application/octet-stream, Size: 2892 bytes --]

From 9d7651bb7b97faff034186a2667d90e42f6f9e5b Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sun, 5 Feb 2023 20:22:52 -0800
Subject: [PATCH] Demo

---
 src/treesit.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/treesit.c b/src/treesit.c
index 8e772523cc7..94c523950d1 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -1471,6 +1471,20 @@ DEFUN ("treesit-parser-language",
   return XTS_PARSER (parser)->language_symbol;
 }
 
+DEFUN ("treesit-parser-live-p",
+       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
+       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
+  (Lisp_Object parser)
+{
+  treesit_check_parser (parser);
+  if (XTS_PARSER (parser)->deleted)
+    return Qnil;
+  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
+    return Qnil;
+  else
+    return Qt;
+}
+
 /*** Parser API */
 
 DEFUN ("treesit-parser-root-node",
@@ -1904,7 +1918,8 @@ DEFUN ("treesit-node-check",
        Ftreesit_node_check, Streesit_node_check, 2, 2, 0,
        doc: /* Return non-nil if NODE has PROPERTY, nil otherwise.
 
-PROPERTY could be `named', `missing', `extra', `outdated', or `has-error'.
+PROPERTY could be `named', `missing', `extra', `outdated',
+`has-error', or `live'.
 
 Named nodes correspond to named rules in the language definition,
 whereas "anonymous" nodes correspond to string literals in the
@@ -1920,7 +1935,9 @@ DEFUN ("treesit-node-check",
 the node was created.
 
 A node "has error" if itself is a syntax error or contains any syntax
-errors.  */)
+errors.
+
+A node is "live" if its parser is live (i.e., not deleted).  */)
   (Lisp_Object node, Lisp_Object property)
 {
   if (NILP (node)) return Qnil;
@@ -1943,9 +1960,11 @@ DEFUN ("treesit-node-check",
     result = ts_node_is_extra (treesit_node);
   else if (EQ (property, Qhas_error))
     result = ts_node_has_error (treesit_node);
+  else if (EQ (property, Qlive))
+    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);
   else
     signal_error ("Expecting `named', `missing', `extra', "
-		  "`outdated', or `has-error', but got",
+                  "`outdated', `has-error', or `deleted', but got",
 		  property);
   return result ? Qt : Qnil;
 }
@@ -3444,6 +3463,7 @@ syms_of_treesit (void)
   DEFSYM (Qextra, "extra");
   DEFSYM (Qoutdated, "outdated");
   DEFSYM (Qhas_error, "has-error");
+  DEFSYM (Qlive, "live");
 
   DEFSYM (QCanchor, ":anchor");
   DEFSYM (QCequal, ":equal");
@@ -3577,6 +3597,7 @@ syms_of_treesit (void)
   defsubr (&Streesit_parser_list);
   defsubr (&Streesit_parser_buffer);
   defsubr (&Streesit_parser_language);
+  defsubr (&Streesit_parser_live_p);
 
   defsubr (&Streesit_parser_root_node);
   /* defsubr (&Streesit_parse_string); */
-- 
2.33.1

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 5 Feb 2023 20:24:27 -0800
> Cc: 61235@debbugs.gnu.org,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Mickey Petersen <mickey@masteringemacs.org> writes:
> 
> > There's no way to tell if a node belongs to a now-deleted
> > parser. Checking if it is `missing' or `outdated' returns nil; there
> > is no way to ascertain this state except by catching an error if you
> > try to get its node text, type, etc.
> 
> That sounds reasonable. I can add treesit-parser-live-p, and add
> a "live" PROPERTY to treesit-node-check.

I'm not sure I understand the need.  AFAIU, a parser is deleted only
if we call treesit-parser-delete; are we saying that a Lisp program
doesn't know that it deleted a parser?  What exactly is the practical
situation where this problem happens, and why?

Frankly, I don't think we should at this stage add APIs without a very
good reason.  We should instead collect experience, both from users
and from Lisp programs, and analyze them before deciding whether more
APIs are necessary.

> +DEFUN ("treesit-parser-live-p",
> +       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
> +       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
> +  (Lisp_Object parser)
> +{
> +  treesit_check_parser (parser);
> +  if (XTS_PARSER (parser)->deleted)
> +    return Qnil;
> +  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
> +    return Qnil;
> +  else
> +    return Qt;
> +}

Doesn't treesit_check_parser signal an error if the parser was
deleted?  If so, how will the above be useful, if someone wants to
avoid errors?

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sun, 5 Feb 2023 20:24:27 -0800
>> Cc: 61235@debbugs.gnu.org,
>>  Eli Zaretskii <eliz@gnu.org>
>>
>> Mickey Petersen <mickey@masteringemacs.org> writes:
>>
>> > There's no way to tell if a node belongs to a now-deleted
>> > parser. Checking if it is `missing' or `outdated' returns nil; there
>> > is no way to ascertain this state except by catching an error if you
>> > try to get its node text, type, etc.
>>
>> That sounds reasonable. I can add treesit-parser-live-p, and add
>> a "live" PROPERTY to treesit-node-check.
>
> I'm not sure I understand the need.  AFAIU, a parser is deleted only
> if we call treesit-parser-delete; are we saying that a Lisp program
> doesn't know that it deleted a parser?  What exactly is the practical
> situation where this problem happens, and why?
>
> Frankly, I don't think we should at this stage add APIs without a very
> good reason.  We should instead collect experience, both from users
> and from Lisp programs, and analyze them before deciding whether more
> APIs are necessary.
>

Because node references are retained even after a parser is deleted.

Retrieving a node; somehow deleting the parser (maybe you closed the
buffer, or you were doing some off-hand parsing); and then doing
_anything_ with the aforementioned node yields an error for which
there is no way to test for.

This is particularly the case when you mix and match parsers in the
same buffer.

>> +DEFUN ("treesit-parser-live-p",
>> +       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
>> +       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
>> +  (Lisp_Object parser)
>> +{
>> +  treesit_check_parser (parser);
>> +  if (XTS_PARSER (parser)->deleted)
>> +    return Qnil;
>> +  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
>> +    return Qnil;
>> +  else
>> +    return Qt;
>> +}
>
> Doesn't treesit_check_parser signal an error if the parser was
> deleted?  If so, how will the above be useful, if someone wants to
> avoid errors?

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Mickey Petersen <mickey@masteringemacs.org>
> Cc: Yuan Fu <casouri@gmail.com>, 61235@debbugs.gnu.org
> Date: Mon, 06 Feb 2023 12:35:20 +0000
> 
> > I'm not sure I understand the need.  AFAIU, a parser is deleted only
> > if we call treesit-parser-delete; are we saying that a Lisp program
> > doesn't know that it deleted a parser?  What exactly is the practical
> > situation where this problem happens, and why?
> >
> > Frankly, I don't think we should at this stage add APIs without a very
> > good reason.  We should instead collect experience, both from users
> > and from Lisp programs, and analyze them before deciding whether more
> > APIs are necessary.
> >
> 
> Because node references are retained even after a parser is deleted.
> 
> Retrieving a node; somehow deleting the parser (maybe you closed the
> buffer, or you were doing some off-hand parsing); and then doing
> _anything_ with the aforementioned node yields an error for which
> there is no way to test for.
> 
> This is particularly the case when you mix and match parsers in the
> same buffer.

I'm asking why the Lisp program cannot track the parsers its uses and
deletes, and instead expects the core to do the janitor's job for it.

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Mickey Petersen <mickey@masteringemacs.org>
>> Cc: Yuan Fu <casouri@gmail.com>, 61235@debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 12:35:20 +0000
>>
>> > I'm not sure I understand the need.  AFAIU, a parser is deleted only
>> > if we call treesit-parser-delete; are we saying that a Lisp program
>> > doesn't know that it deleted a parser?  What exactly is the practical
>> > situation where this problem happens, and why?
>> >
>> > Frankly, I don't think we should at this stage add APIs without a very
>> > good reason.  We should instead collect experience, both from users
>> > and from Lisp programs, and analyze them before deciding whether more
>> > APIs are necessary.
>> >
>>
>> Because node references are retained even after a parser is deleted.
>>
>> Retrieving a node; somehow deleting the parser (maybe you closed the
>> buffer, or you were doing some off-hand parsing); and then doing
>> _anything_ with the aforementioned node yields an error for which
>> there is no way to test for.
>>
>> This is particularly the case when you mix and match parsers in the
>> same buffer.
>
> I'm asking why the Lisp program cannot track the parsers its uses and
> deletes, and instead expects the core to do the janitor's job for it.

Because I have a proxy-like object of a real node because they're
invalidated if a buffer is edited, even if the parcel of code I hold a
node reference to is untouched. That's just how tree-sitter works, so
I deal with it like this. That part works fine for I can of course use
`treesit-node-check' to determine if it's outdated and thus needs
refreshing (or not.)

The problems begin when the parser is also, for one reason or another,
destroyed.

Now I can no longer determine if it is safe to use the existing (real)
node reference. Any attempt to use a "dangling" node reference
that points to a dead parser throws a nasty error.

Thus, the extension to `treesit-node-check' that can tell if a node
belongs to a dead parser --- something that tree-sitter and the C core
clearly knows about internally.

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Mickey Petersen <mickey@masteringemacs.org>
> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
> Date: Mon, 06 Feb 2023 13:19:57 +0000
> 
> > I'm asking why the Lisp program cannot track the parsers its uses and
> > deletes, and instead expects the core to do the janitor's job for it.
> 
> Because I have a proxy-like object of a real node because they're
> invalidated if a buffer is edited, even if the parcel of code I hold a
> node reference to is untouched. That's just how tree-sitter works, so
> I deal with it like this. That part works fine for I can of course use
> `treesit-node-check' to determine if it's outdated and thus needs
> refreshing (or not.)
> 
> The problems begin when the parser is also, for one reason or another,
> destroyed.

But it is only destroyed if your program calls treesit-parser-delete,
no?

Anyway, I'm okay with exposing treesit_check_parser to Lisp, if you
really insist.  But please be sure you want to insist, because I'm not
really convinced.

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Mickey Petersen <mickey@masteringemacs.org>
>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 13:19:57 +0000
>>
>> > I'm asking why the Lisp program cannot track the parsers its uses and
>> > deletes, and instead expects the core to do the janitor's job for it.
>>
>> Because I have a proxy-like object of a real node because they're
>> invalidated if a buffer is edited, even if the parcel of code I hold a
>> node reference to is untouched. That's just how tree-sitter works, so
>> I deal with it like this. That part works fine for I can of course use
>> `treesit-node-check' to determine if it's outdated and thus needs
>> refreshing (or not.)
>>
>> The problems begin when the parser is also, for one reason or another,
>> destroyed.
>
> But it is only destroyed if your program calls treesit-parser-delete,
> no?
>
> Anyway, I'm okay with exposing treesit_check_parser to Lisp, if you
> really insist.  But please be sure you want to insist, because I'm not
> really convinced.

All I want is a way for treesit-node-check to tell me if the node
belongs to a dead or alive parser. It is already capable of telling me
if a node is outdated, for instance, another rather important feature.
Knowing its status is pertinent if you do any sort of light
refactoring or if you end up destroying a block of code that has its
own nested parser.

Whether I destroy a parser explicitly (which is how I found the issue)
or indirectly (through some other mechanism) is, I think, orthogonal
to the problem of determining liveness (of a node, of a process, or
any of the `xxxxxx-live-p' functions we presently have)

Thanks,

Mickey

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Mickey Petersen <mickey@masteringemacs.org>
> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
> Date: Mon, 06 Feb 2023 14:08:46 +0000
> 
> All I want is a way for treesit-node-check to tell me if the node
> belongs to a dead or alive parser.

That'd be fine by me, but the patch posted by Yuan was a different
one.

Yuan, any reason not to extend treesit-node-check instead?

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Mickey Petersen <mickey@masteringemacs.org>
>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>> 
>> All I want is a way for treesit-node-check to tell me if the node
>> belongs to a dead or alive parser.
> 
> That'd be fine by me, but the patch posted by Yuan was a different
> one.
> 
> Yuan, any reason not to extend treesit-node-check instead?

I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.

Micky, the function I added (and the extension to treesit-node-check) checks that the parser is not deleted AND its buffer is live. That makes the most sense to me, but would it cause any problem for your use case?

Yuan

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 6 Feb 2023 19:00:30 -0800
> Cc: Mickey Petersen <mickey@masteringemacs.org>,
>  61235@debbugs.gnu.org
> 
> > Yuan, any reason not to extend treesit-node-check instead?
> 
> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.

That additional function would signal an error in the case discussed
here, so I'm not sure we should add it in that shape, or at all.  Why
isn't treesit-node-check enough?

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]



> On Feb 6, 2023, at 7:31 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 6 Feb 2023 19:00:30 -0800
>> Cc: Mickey Petersen <mickey@masteringemacs.org>,
>> 61235@debbugs.gnu.org
>> 
>>> Yuan, any reason not to extend treesit-node-check instead?
>> 
>> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
> 
> That additional function would signal an error in the case discussed
> here, so I'm not sure we should add it in that shape, or at all.  Why
> isn't treesit-node-check enough?

Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.

Yuan


[-- Attachment #2: livep.patch --]
[-- Type: application/octet-stream, Size: 2856 bytes --]

From 08765f4f61d63398307e3401313c45a60d65edee Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sun, 5 Feb 2023 20:22:52 -0800
Subject: [PATCH] Demo

---
 src/treesit.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/treesit.c b/src/treesit.c
index 8e772523cc7..f2c3e9845d1 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -1471,6 +1471,19 @@ DEFUN ("treesit-parser-language",
   return XTS_PARSER (parser)->language_symbol;
 }
 
+DEFUN ("treesit-parser-live-p",
+       Ftreesit_parser_live_p, Streesit_parser_live_p, 1, 1, 0,
+       doc: /* Check whether PARSER is not deleted and its buffer is live.  */)
+  (Lisp_Object parser)
+{
+  if (XTS_PARSER (parser)->deleted)
+    return Qnil;
+  else if (NILP (Fbuffer_live_p (XTS_PARSER (parser)->buffer)))
+    return Qnil;
+  else
+    return Qt;
+}
+
 /*** Parser API */
 
 DEFUN ("treesit-parser-root-node",
@@ -1904,7 +1917,8 @@ DEFUN ("treesit-node-check",
        Ftreesit_node_check, Streesit_node_check, 2, 2, 0,
        doc: /* Return non-nil if NODE has PROPERTY, nil otherwise.
 
-PROPERTY could be `named', `missing', `extra', `outdated', or `has-error'.
+PROPERTY could be `named', `missing', `extra', `outdated',
+`has-error', or `live'.
 
 Named nodes correspond to named rules in the language definition,
 whereas "anonymous" nodes correspond to string literals in the
@@ -1920,7 +1934,9 @@ DEFUN ("treesit-node-check",
 the node was created.
 
 A node "has error" if itself is a syntax error or contains any syntax
-errors.  */)
+errors.
+
+A node is "live" if its parser is live (i.e., not deleted).  */)
   (Lisp_Object node, Lisp_Object property)
 {
   if (NILP (node)) return Qnil;
@@ -1943,9 +1959,11 @@ DEFUN ("treesit-node-check",
     result = ts_node_is_extra (treesit_node);
   else if (EQ (property, Qhas_error))
     result = ts_node_has_error (treesit_node);
+  else if (EQ (property, Qlive))
+    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);
   else
     signal_error ("Expecting `named', `missing', `extra', "
-		  "`outdated', or `has-error', but got",
+                  "`outdated', `has-error', or `deleted', but got",
 		  property);
   return result ? Qt : Qnil;
 }
@@ -3444,6 +3462,7 @@ syms_of_treesit (void)
   DEFSYM (Qextra, "extra");
   DEFSYM (Qoutdated, "outdated");
   DEFSYM (Qhas_error, "has-error");
+  DEFSYM (Qlive, "live");
 
   DEFSYM (QCanchor, ":anchor");
   DEFSYM (QCequal, ":equal");
@@ -3577,6 +3596,7 @@ syms_of_treesit (void)
   defsubr (&Streesit_parser_list);
   defsubr (&Streesit_parser_buffer);
   defsubr (&Streesit_parser_language);
+  defsubr (&Streesit_parser_live_p);
 
   defsubr (&Streesit_parser_root_node);
   /* defsubr (&Streesit_parse_string); */
-- 
2.33.1

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 6 Feb 2023 20:55:38 -0800
> Cc: Mickey Petersen <mickey@masteringemacs.org>,
>  61235@debbugs.gnu.org
> 
> >>> Yuan, any reason not to extend treesit-node-check instead?
> >> 
> >> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
> > 
> > That additional function would signal an error in the case discussed
> > here, so I'm not sure we should add it in that shape, or at all.  Why
> > isn't treesit-node-check enough?
> 
> Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.

I think treesit-node-check alone should be enough.

One comment:

> @@ -1943,9 +1959,11 @@ DEFUN ("treesit-node-check",
>      result = ts_node_is_extra (treesit_node);
>    else if (EQ (property, Qhas_error))
>      result = ts_node_has_error (treesit_node);
> +  else if (EQ (property, Qlive))
> +    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);

Ftreesit_parser_live_p returns a Lisp object, whereas 'result' is a C
'bool' type.  This won't compile if you configure with
"--enable-check-lisp-object-type".

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

> On Feb 7, 2023, at 4:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 6 Feb 2023 20:55:38 -0800
>> Cc: Mickey Petersen <mickey@masteringemacs.org>,
>> 61235@debbugs.gnu.org
>> 
>>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>> 
>>>> I did extend treesit-node-check in the patch. But I also added a function treesit-parser-live-p, which makes the same check but directly on a parser. It just made sense to me that if we let treesit-node-check check the nodes’ parser’s status, we’d also add a function to allow directly checking the status of a parser.
>>> 
>>> That additional function would signal an error in the case discussed
>>> here, so I'm not sure we should add it in that shape, or at all.  Why
>>> isn't treesit-node-check enough?
>> 
>> Oops, it shouldn’t have. The updated patch fixes that. Treesit-node-check is enough, it just made more sense implentattion-wise, to implement that function that checks a parser, and let treesit-node-check use that function to check the node’s parser. We can choose to not expose that function, and only expose this feature through treesit-node-check, if you prefer so.
> 
> I think treesit-node-check alone should be enough.

Cool, I’ll only modify treesit-node-check, then.

> 
> One comment:
> 
>> @@ -1943,9 +1959,11 @@ DEFUN ("treesit-node-check",
>>     result = ts_node_is_extra (treesit_node);
>>   else if (EQ (property, Qhas_error))
>>     result = ts_node_has_error (treesit_node);
>> +  else if (EQ (property, Qlive))
>> +    result = Ftreesit_parser_live_p (XTS_NODE (node)->parser);
> 
> Ftreesit_parser_live_p returns a Lisp object, whereas 'result' is a C
> 'bool' type.  This won't compile if you configure with
> "--enable-check-lisp-object-type”.

Right, sorry :-(

Yuan

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

Yuan Fu <casouri@gmail.com> writes:

>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>>> From: Mickey Petersen <mickey@masteringemacs.org>
>>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>
>>> All I want is a way for treesit-node-check to tell me if the node
>>> belongs to a dead or alive parser.
>>
>> That'd be fine by me, but the patch posted by Yuan was a different
>> one.
>>
>> Yuan, any reason not to extend treesit-node-check instead?
>
> I did extend treesit-node-check in the patch. But I also added a
> function treesit-parser-live-p, which makes the same check but
> directly on a parser. It just made sense to me that if we let
> treesit-node-check check the nodes’ parser’s status, we’d also add a
> function to allow directly checking the status of a parser.
>
> Micky, the function I added (and the extension to treesit-node-check)
> checks that the parser is not deleted AND its buffer is live. That
> makes the most sense to me, but would it cause any problem for your
> use case?

Thanks for turning around the features so fast.

I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
that, so perhaps leaving out that check makes sense?

> Yuan

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey@masteringemacs.org> wrote:
> 
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> 
>>>> From: Mickey Petersen <mickey@masteringemacs.org>
>>>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>> 
>>>> All I want is a way for treesit-node-check to tell me if the node
>>>> belongs to a dead or alive parser.
>>> 
>>> That'd be fine by me, but the patch posted by Yuan was a different
>>> one.
>>> 
>>> Yuan, any reason not to extend treesit-node-check instead?
>> 
>> I did extend treesit-node-check in the patch. But I also added a
>> function treesit-parser-live-p, which makes the same check but
>> directly on a parser. It just made sense to me that if we let
>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>> function to allow directly checking the status of a parser.
>> 
>> Micky, the function I added (and the extension to treesit-node-check)
>> checks that the parser is not deleted AND its buffer is live. That
>> makes the most sense to me, but would it cause any problem for your
>> use case?
> 
> Thanks for turning around the features so fast.
> 
> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
> that, so perhaps leaving out that check makes sense?

I’m hoping to write the function as I described, ie, return t only if the parser is not deleted and its buffer is live. So I wonder if this definition of “live” would work for you?

Yuan

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Mickey Petersen]

Yuan Fu <casouri@gmail.com> writes:

>> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey@masteringemacs.org> wrote:
>>
>>
>> Yuan Fu <casouri@gmail.com> writes:
>>
>>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>>
>>>>> From: Mickey Petersen <mickey@masteringemacs.org>
>>>>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>>>
>>>>> All I want is a way for treesit-node-check to tell me if the node
>>>>> belongs to a dead or alive parser.
>>>>
>>>> That'd be fine by me, but the patch posted by Yuan was a different
>>>> one.
>>>>
>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>
>>> I did extend treesit-node-check in the patch. But I also added a
>>> function treesit-parser-live-p, which makes the same check but
>>> directly on a parser. It just made sense to me that if we let
>>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>>> function to allow directly checking the status of a parser.
>>>
>>> Micky, the function I added (and the extension to treesit-node-check)
>>> checks that the parser is not deleted AND its buffer is live. That
>>> makes the most sense to me, but would it cause any problem for your
>>> use case?
>>
>> Thanks for turning around the features so fast.
>>
>> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
>> that, so perhaps leaving out that check makes sense?
>
> I’m hoping to write the function as I described, ie, return t only if
> the parser is not deleted and its buffer is live. So I wonder if this
> definition of “live” would work for you?

Sounds good to me, and I think others will find it useful as well!

bug#61235: 30.0.50; tree-sit: `treesit-node-check' lacks a way to tell if a node belongs to a deleted parser

[Yuan Fu]

Mickey Petersen <mickey@masteringemacs.org> writes:

> Yuan Fu <casouri@gmail.com> writes:
>
>>> On Feb 7, 2023, at 12:03 AM, Mickey Petersen <mickey@masteringemacs.org> wrote:
>>>
>>>
>>> Yuan Fu <casouri@gmail.com> writes:
>>>
>>>>> On Feb 6, 2023, at 7:21 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>>>
>>>>>> From: Mickey Petersen <mickey@masteringemacs.org>
>>>>>> Cc: casouri@gmail.com, 61235@debbugs.gnu.org
>>>>>> Date: Mon, 06 Feb 2023 14:08:46 +0000
>>>>>>
>>>>>> All I want is a way for treesit-node-check to tell me if the node
>>>>>> belongs to a dead or alive parser.
>>>>>
>>>>> That'd be fine by me, but the patch posted by Yuan was a different
>>>>> one.
>>>>>
>>>>> Yuan, any reason not to extend treesit-node-check instead?
>>>>
>>>> I did extend treesit-node-check in the patch. But I also added a
>>>> function treesit-parser-live-p, which makes the same check but
>>>> directly on a parser. It just made sense to me that if we let
>>>> treesit-node-check check the nodes’ parser’s status, we’d also add a
>>>> function to allow directly checking the status of a parser.
>>>>
>>>> Micky, the function I added (and the extension to treesit-node-check)
>>>> checks that the parser is not deleted AND its buffer is live. That
>>>> makes the most sense to me, but would it cause any problem for your
>>>> use case?
>>>
>>> Thanks for turning around the features so fast.
>>>
>>> I can use `treesit-node-buffer' and `buffer-live-p' to accomplish
>>> that, so perhaps leaving out that check makes sense?
>>
>> I’m hoping to write the function as I described, ie, return t only if
>> the parser is not deleted and its buffer is live. So I wonder if this
>> definition of “live” would work for you?
>
> Sounds good to me, and I think others will find it useful as well!

Done.

Yuan