GNU bug report logs - #77077
Markers in treesit_check_node

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 77077 in the body.
You can then email your comments to 77077 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: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Markers in treesit_check_node
Date: Mon, 17 Mar 2025 19:52:27 +0200

In mhtml-ts-mode activate the region, type 'C-c C-t'
and an element name like div, type RET RET, then:

Debugger entered--Lisp error: (wrong-type-argument integerp #<marker (moves after insertion) at 10 in test.html>)
  treesit-query-capture(#<treesit-parser for html> #<treesit-compiled-query> 7 #<marker (moves after insertion) at 10 in test.html>)
  treesit-query-range(#<treesit-parser for html> #<treesit-compiled-query> 7 #<marker (moves after insertion) at 10 in test.html> nil nil)
  treesit--update-ranges-non-local(#<treesit-parser for html> #<treesit-compiled-query> javascript 32 1 7 #<marker (moves after insertion) at 10 in test.html> nil nil)
  treesit--update-range-1(7 #<marker (moves after insertion) at 10 in test.html> #<treesit-parser for html> ...)
  treesit-update-ranges(7 #<marker (moves after insertion) at 10 in test.html>)
  treesit-indent-region(7 #<marker (moves after insertion) at 10 in test.html>)
  indent-region(7 #<marker (moves after insertion) at 10 in test.html> nil)
  skeleton-internal-1(n t nil)
  skeleton-insert(... -1 nil)
  skeleton-proxy-new(... nil nil)
  sgml-tag(nil nil)
  funcall-interactively(sgml-tag nil nil)
  command-execute(sgml-tag)

Here's the patch to fix this and it seems no more changes necessary
since it fails only on checking, but on using it converts markers
to numbers automatically.

diff --git a/src/treesit.c b/src/treesit.c
index b0979397d35..7df9bb4d0d3 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -2190,6 +2190,9 @@ treesit_check_node (Lisp_Object obj)
 static void
 treesit_check_position (Lisp_Object obj, struct buffer *buf)
 {
+  if (MARKERP (obj))
+    obj = make_fixnum (marker_position (obj));
+
   treesit_check_positive_integer (obj);
   ptrdiff_t pos = XFIXNUM (obj);
   if (pos < BUF_BEGV (buf) || pos > BUF_ZV (buf))

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

From: Yuan Fu <casouri <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 77077 <at> debbugs.gnu.org
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Tue, 18 Mar 2025 17:58:27 -0700

> On Mar 17, 2025, at 10:52 AM, Juri Linkov <juri <at> linkov.net> wrote:
> 
> In mhtml-ts-mode activate the region, type 'C-c C-t'
> and an element name like div, type RET RET, then:
> 
> Debugger entered--Lisp error: (wrong-type-argument integerp #<marker (moves after insertion) at 10 in test.html>)
>  treesit-query-capture(#<treesit-parser for html> #<treesit-compiled-query> 7 #<marker (moves after insertion) at 10 in test.html>)
>  treesit-query-range(#<treesit-parser for html> #<treesit-compiled-query> 7 #<marker (moves after insertion) at 10 in test.html> nil nil)
>  treesit--update-ranges-non-local(#<treesit-parser for html> #<treesit-compiled-query> javascript 32 1 7 #<marker (moves after insertion) at 10 in test.html> nil nil)
>  treesit--update-range-1(7 #<marker (moves after insertion) at 10 in test.html> #<treesit-parser for html> ...)
>  treesit-update-ranges(7 #<marker (moves after insertion) at 10 in test.html>)
>  treesit-indent-region(7 #<marker (moves after insertion) at 10 in test.html>)
>  indent-region(7 #<marker (moves after insertion) at 10 in test.html> nil)
>  skeleton-internal-1(n t nil)
>  skeleton-insert(... -1 nil)
>  skeleton-proxy-new(... nil nil)
>  sgml-tag(nil nil)
>  funcall-interactively(sgml-tag nil nil)
>  command-execute(sgml-tag)
> 
> Here's the patch to fix this and it seems no more changes necessary
> since it fails only on checking, but on using it converts markers
> to numbers automatically.
> 
> diff --git a/src/treesit.c b/src/treesit.c
> index b0979397d35..7df9bb4d0d3 100644
> --- a/src/treesit.c
> +++ b/src/treesit.c
> @@ -2190,6 +2190,9 @@ treesit_check_node (Lisp_Object obj)
> static void
> treesit_check_position (Lisp_Object obj, struct buffer *buf)
> {
> +  if (MARKERP (obj))
> +    obj = make_fixnum (marker_position (obj));
> +
>   treesit_check_positive_integer (obj);
>   ptrdiff_t pos = XFIXNUM (obj);
>   if (pos < BUF_BEGV (buf) || pos > BUF_ZV (buf))

We call XFIXNUM (obj) after treesit_check_position in a lot of places. Your patch didn’t cause any crash? Anyway, I think we should keep treesit_check_position as-is. And change treesit-indent-region to convert markers to integers. Integer and marker might be interchangeable in Lisp, but I don’t think it’s a good idea to mingle them in C.

Yuan

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

From: Juri Linkov <juri <at> linkov.net>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 77077 <at> debbugs.gnu.org
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Wed, 19 Mar 2025 09:23:09 +0200

>> treesit_check_position (Lisp_Object obj, struct buffer *buf)
>> {
>> +  if (MARKERP (obj))
>> +    obj = make_fixnum (marker_position (obj));
>> +
>
> We call XFIXNUM (obj) after treesit_check_position in a lot of places. Your
> patch didn’t cause any crash?

No crashes at all.  I guess this is because of automatic conversion of
markers to integers.

> Anyway, I think we should keep treesit_check_position as-is. And
> change treesit-indent-region to convert markers to integers. Integer
> and marker might be interchangeable in Lisp, but I don’t think it’s
> a good idea to mingle them in C.

OK, then let's fix only treesit-indent-region:

diff --git a/lisp/treesit.el b/lisp/treesit.el
index 46332cb1e4b..7410c0f32a8 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -2602,6 +2602,8 @@ treesit--indent-region-batch-size
 (defun treesit-indent-region (beg end)
   "Indent the region between BEG and END.
 Similar to `treesit-indent', but indent a region instead."
+  (when (markerp beg) (setq beg (marker-position beg)))
+  (when (markerp end) (setq end (marker-position end)))
   (treesit-update-ranges beg end)
   ;; We indent `treesit--indent-region-batch-size' lines at a time, to
   ;; reduce the number of times the parser needs to re-parse.  In each

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

From: Juri Linkov <juri <at> linkov.net>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 77077 <at> debbugs.gnu.org
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Fri, 21 Mar 2025 09:57:40 +0200

>> Anyway, I think we should keep treesit_check_position as-is. And
>> change treesit-indent-region to convert markers to integers. Integer
>> and marker might be interchangeable in Lisp, but I don’t think it’s
>> a good idea to mingle them in C.
>
> OK, then let's fix only treesit-indent-region:
>
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index 46332cb1e4b..7410c0f32a8 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -2602,6 +2602,8 @@ treesit--indent-region-batch-size
>  (defun treesit-indent-region (beg end)
>    "Indent the region between BEG and END.
>  Similar to `treesit-indent', but indent a region instead."
> +  (when (markerp beg) (setq beg (marker-position beg)))
> +  (when (markerp end) (setq end (marker-position end)))
>    (treesit-update-ranges beg end)
>    ;; We indent `treesit--indent-region-batch-size' lines at a time, to
>    ;; reduce the number of times the parser needs to re-parse.  In each

Eli, is it ok to install this patch in emacs-30?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: casouri <at> gmail.com, 77077 <at> debbugs.gnu.org
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Fri, 21 Mar 2025 10:46:49 +0200

> Cc: 77077 <at> debbugs.gnu.org
> From: Juri Linkov <juri <at> linkov.net>
> Date: Fri, 21 Mar 2025 09:57:40 +0200
> 
> >> Anyway, I think we should keep treesit_check_position as-is. And
> >> change treesit-indent-region to convert markers to integers. Integer
> >> and marker might be interchangeable in Lisp, but I don’t think it’s
> >> a good idea to mingle them in C.
> >
> > OK, then let's fix only treesit-indent-region:
> >
> > diff --git a/lisp/treesit.el b/lisp/treesit.el
> > index 46332cb1e4b..7410c0f32a8 100644
> > --- a/lisp/treesit.el
> > +++ b/lisp/treesit.el
> > @@ -2602,6 +2602,8 @@ treesit--indent-region-batch-size
> >  (defun treesit-indent-region (beg end)
> >    "Indent the region between BEG and END.
> >  Similar to `treesit-indent', but indent a region instead."
> > +  (when (markerp beg) (setq beg (marker-position beg)))
> > +  (when (markerp end) (setq end (marker-position end)))
> >    (treesit-update-ranges beg end)
> >    ;; We indent `treesit--indent-region-batch-size' lines at a time, to
> >    ;; reduce the number of times the parser needs to re-parse.  In each
> 
> Eli, is it ok to install this patch in emacs-30?

Yes, thanks.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77077 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Fri, 21 Mar 2025 21:02:19 -0700

> On Mar 21, 2025, at 1:46 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> Cc: 77077 <at> debbugs.gnu.org
>> From: Juri Linkov <juri <at> linkov.net>
>> Date: Fri, 21 Mar 2025 09:57:40 +0200
>> 
>>>> Anyway, I think we should keep treesit_check_position as-is. And
>>>> change treesit-indent-region to convert markers to integers. Integer
>>>> and marker might be interchangeable in Lisp, but I don’t think it’s
>>>> a good idea to mingle them in C.
>>> 
>>> OK, then let's fix only treesit-indent-region:
>>> 
>>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>>> index 46332cb1e4b..7410c0f32a8 100644
>>> --- a/lisp/treesit.el
>>> +++ b/lisp/treesit.el
>>> @@ -2602,6 +2602,8 @@ treesit--indent-region-batch-size
>>> (defun treesit-indent-region (beg end)
>>>   "Indent the region between BEG and END.
>>> Similar to `treesit-indent', but indent a region instead."
>>> +  (when (markerp beg) (setq beg (marker-position beg)))
>>> +  (when (markerp end) (setq end (marker-position end)))
>>>   (treesit-update-ranges beg end)
>>>   ;; We indent `treesit--indent-region-batch-size' lines at a time, to
>>>   ;; reduce the number of times the parser needs to re-parse.  In each
>> 
>> Eli, is it ok to install this patch in emacs-30?
> 
> Yes, thanks.

Thank you Juri.

Yuan

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

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 77077 <at> debbugs.gnu.org
Subject: Re: bug#77077: Markers in treesit_check_node
Date: Sat, 22 Mar 2025 20:37:49 +0200

close 77077 31.0.50
thanks

>> >  (defun treesit-indent-region (beg end)
>> >    "Indent the region between BEG and END.
>> >  Similar to `treesit-indent', but indent a region instead."
>> > +  (when (markerp beg) (setq beg (marker-position beg)))
>> > +  (when (markerp end) (setq end (marker-position end)))
>> >    (treesit-update-ranges beg end)
>> >    ;; We indent `treesit--indent-region-batch-size' lines at a time, to
>> >    ;; reduce the number of times the parser needs to re-parse.  In each
>> 
>> Eli, is it ok to install this patch in emacs-30?
>
> Yes, thanks.

So now pushed to emacs-30 and closed.

Previous Next

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