GNU bug report logs - #62158
treesit-end-of-defun error

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 62158 in the body.
You can then email your comments to 62158 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: treesit-end-of-defun error
Date: Mon, 13 Mar 2023 09:28:59 +0200

X-Debbugs-Cc: Yuan Fu <casouri <at> gmail.com>, Dmitry Gutov <dgutov <at> yandex.ru>

Since this is a separate problem, I'm closing bug#62086
and opening a new bug report:

>>>> I don't know if the second bug is related to this, but while
>>>> in the same file, also type 'C-M-l' ('reposition-window').
>>>> It raises the error:
>>>>   Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>>>>     treesit-end-of-defun()
>>>>     end-of-defun(-1)
>>>>     reposition-window(nil nil)
>>>>     reposition-window(nil 89)
>>>>     funcall-interactively(reposition-window nil 89)
>>>>     command-execute(reposition-window)
>> I see it only in some files in test/lisp/progmodes/ruby-mode-resources/
>> e.g. ruby-parenless-call-arguments-indent.rb, ruby-method-call-indent.rb,
>> ruby-block-indent.rb.  But not in e.g. ruby-after-operator-indent.rb.
>> Also everywhere in test/lisp/progmodes/js-resources/js-indent-init-dynamic.js,
>> js-indent-init-t.js.  But not in e.g. js-chain.js.
>
> Thanks, I can repro. I might have been trying the wrong binding at the end
> last night (C-l instead of C-M-l).
>
> The fix seems to be easy:
>
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index c118f5d52a4..b271a1f0c4b 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -1882,6 +1882,7 @@ treesit-end-of-defun
>  `treesit-defun-skipper'."
>    (interactive "^p\nd")
>    (let ((orig-point (point)))
> +    (if (or (null arg) (= arg 0)) (setq arg 1))
>      (catch 'done
>        (dotimes (_ 2) ; Not making progress is better than infloop.
>
> But I'm not quite sure if that is what we want to do.
>
> More naturally, I think, would be to remove the argument from
> treesit-end-of-defun altogether (and adjust the code accordingly), because
> end-of-defun-function is documented to take no arguments.
>
> The only other place where treesit-end-of-defun seems to be used is the
> <remap> <end-of-defun> binding set up by treesit-major-mode-setup.
>
> Why not keep the default bindings for these? When
> beginning-of-defun-function and end-of-defun-function are set
> appropriately, they should work fine. Don't they?
>
> Cc'ing Yuan on that subject.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 62158 <at> debbugs.gnu.org, dmitry gutov <dgutov <at> yandex.ru>
Subject: Re: bug#62158: treesit-end-of-defun error
Date: Mon, 13 Mar 2023 15:04:37 -0700

> On Mar 13, 2023, at 12:28 AM, Juri Linkov <juri <at> linkov.net> wrote:
> 
> X-Debbugs-Cc: Yuan Fu <casouri <at> gmail.com>, Dmitry Gutov <dgutov <at> yandex.ru>
> 
> Since this is a separate problem, I'm closing bug#62086
> and opening a new bug report:
> 
>>>>> I don't know if the second bug is related to this, but while
>>>>> in the same file, also type 'C-M-l' ('reposition-window').
>>>>> It raises the error:
>>>>>  Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>>>>>    treesit-end-of-defun()
>>>>>    end-of-defun(-1)
>>>>>    reposition-window(nil nil)
>>>>>    reposition-window(nil 89)
>>>>>    funcall-interactively(reposition-window nil 89)
>>>>>    command-execute(reposition-window)
>>> I see it only in some files in test/lisp/progmodes/ruby-mode-resources/
>>> e.g. ruby-parenless-call-arguments-indent.rb, ruby-method-call-indent.rb,
>>> ruby-block-indent.rb.  But not in e.g. ruby-after-operator-indent.rb.
>>> Also everywhere in test/lisp/progmodes/js-resources/js-indent-init-dynamic.js,
>>> js-indent-init-t.js.  But not in e.g. js-chain.js.
>> 
>> Thanks, I can repro. I might have been trying the wrong binding at the end
>> last night (C-l instead of C-M-l).
>> 
>> The fix seems to be easy:
>> 
>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>> index c118f5d52a4..b271a1f0c4b 100644
>> --- a/lisp/treesit.el
>> +++ b/lisp/treesit.el
>> @@ -1882,6 +1882,7 @@ treesit-end-of-defun
>> `treesit-defun-skipper'."
>>   (interactive "^p\nd")
>>   (let ((orig-point (point)))
>> +    (if (or (null arg) (= arg 0)) (setq arg 1))
>>     (catch 'done
>>       (dotimes (_ 2) ; Not making progress is better than infloop.
>> 
>> But I'm not quite sure if that is what we want to do.

This looks good to me.

>> 
>> More naturally, I think, would be to remove the argument from
>> treesit-end-of-defun altogether (and adjust the code accordingly), because
>> end-of-defun-function is documented to take no arguments.
>> 
>> The only other place where treesit-end-of-defun seems to be used is the
>> <remap> <end-of-defun> binding set up by treesit-major-mode-setup.
>> 
>> Why not keep the default bindings for these? When
>> beginning-of-defun-function and end-of-defun-function are set
>> appropriately, they should work fine. Don't they?

We tried that initially, but end-of-defun doesn’t have the notion of nested defuns, which leads to problems when end-of-defun-function recognizes nested defuns. In the following code

(defun xxx ()
  |
  (defun yyy () ...)

  (defun zzz () ...)
  )

If point is at “|” and you call end-of-defun, you’d expect point to move to the end of yyy, but instead it moves to the end of xxx. That’s because end-of-defun first runs (beginning-of-defun -1) followed by (end-of-defun 1) to check if the starting point is in a defun or between two defuns. This is fine in non-nested defuns, but in this example, the point first goes to the beginning of xxx, then goes to the end of xxx. And end-of-defun thinks that we started in a defun and now is at an end of defun, job’s done, and finishes.

The plan is to improve end-of-defun to support nested defuns in Emacs 30. For now we rebind end-of-defun to treesit-end-of-defun.

Yuan

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

From: Juri Linkov <juri <at> linkov.net>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 62158 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#62158: treesit-end-of-defun error
Date: Mon, 20 Mar 2023 20:21:38 +0200

close 62158 29.0.60
thanks

>>> The fix seems to be easy:
>>> 
>>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>>> index c118f5d52a4..b271a1f0c4b 100644
>>> --- a/lisp/treesit.el
>>> +++ b/lisp/treesit.el
>>> @@ -1882,6 +1882,7 @@ treesit-end-of-defun
>>> `treesit-defun-skipper'."
>>>   (interactive "^p\nd")
>>>   (let ((orig-point (point)))
>>> +    (if (or (null arg) (= arg 0)) (setq arg 1))
>>>     (catch 'done
>>>       (dotimes (_ 2) ; Not making progress is better than infloop.
>>> 
>>> But I'm not quite sure if that is what we want to do.
>
> This looks good to me.

So Dmitry's fix is pushed to emacs-29.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Yuan Fu <casouri <at> gmail.com>, Juri Linkov <juri <at> linkov.net>
Cc: 62158 <at> debbugs.gnu.org
Subject: Re: bug#62158: treesit-end-of-defun error
Date: Mon, 20 Mar 2023 20:43:51 +0200

On 14/03/2023 00:04, Yuan Fu wrote:
> We tried that initially, but end-of-defun doesn’t have the notion of nested defuns, which leads to problems when end-of-defun-function recognizes nested defuns. In the following code
> 
> (defun xxx ()
>    |
>    (defun yyy () ...)
> 
>    (defun zzz () ...)
>    )
> 
> If point is at “|” and you call end-of-defun, you’d expect point to move to the end of yyy, but instead it moves to the end of xxx. That’s because end-of-defun first runs (beginning-of-defun -1) followed by (end-of-defun 1) to check if the starting point is in a defun or between two defuns. This is fine in non-nested defuns, but in this example, the point first goes to the beginning of xxx, then goes to the end of xxx. And end-of-defun thinks that we started in a defun and now is at an end of defun, job’s done, and finishes.
> 
> The plan is to improve end-of-defun to support nested defuns in Emacs 30. For now we rebind end-of-defun to treesit-end-of-defun.

That makes sense, thanks!

I guess one of the things to try is to call end-of-defun-function first, 
followed by beginning-of-defun-function. And see if the resulting 
position is below the original point.

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

From: Yuan Fu <casouri <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 62158 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#62158: treesit-end-of-defun error
Date: Tue, 21 Mar 2023 14:26:26 -0700

> On Mar 20, 2023, at 11:43 AM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> 
> On 14/03/2023 00:04, Yuan Fu wrote:
>> We tried that initially, but end-of-defun doesn’t have the notion of nested defuns, which leads to problems when end-of-defun-function recognizes nested defuns. In the following code
>> (defun xxx ()
>>   |
>>   (defun yyy () ...)
>>   (defun zzz () ...)
>>   )
>> If point is at “|” and you call end-of-defun, you’d expect point to move to the end of yyy, but instead it moves to the end of xxx. That’s because end-of-defun first runs (beginning-of-defun -1) followed by (end-of-defun 1) to check if the starting point is in a defun or between two defuns. This is fine in non-nested defuns, but in this example, the point first goes to the beginning of xxx, then goes to the end of xxx. And end-of-defun thinks that we started in a defun and now is at an end of defun, job’s done, and finishes.
>> The plan is to improve end-of-defun to support nested defuns in Emacs 30. For now we rebind end-of-defun to treesit-end-of-defun.
> 
> That makes sense, thanks!
> 
> I guess one of the things to try is to call end-of-defun-function first, followed by beginning-of-defun-function. And see if the resulting position is below the original point.

Unfortunately, end-of-defun-function must be called from the beginning of a defun, according to its docstring. So that won’t do. We got to upgrade beginning/end-of-defun to support nested defuns.

Yuan

Previous Next

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