GNU bug report logs - #60197
30.0.50; beginning-of-defun broken after new treesit impl

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 60197 in the body.
You can then email your comments to 60197 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: Theodor Thornhill <theo <at> thornhill.no>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; beginning-of-defun broken after new treesit impl
Date: Mon, 19 Dec 2022 11:13:15 +0100

Hi, Yuan!

It seems 'prog-fill-reindent-defun' is broken after the latest changes
to treesit-beginning-of-defun.  The culprit is that we now use remap
instead of setting the beginning-of-defun-function.  What is the
reasoning behind that change?  Can't we just rely on the variable
beginning-of-defun-function?

I see you mentioned it is inteded to be used as a command, but surely
both should be possible?

Theo

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: 60197 <at> debbugs.gnu.org
Cc: casouri <at> gmail.com
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit
 impl
Date: Tue, 20 Dec 2022 09:32:23 +0100

Theodor Thornhill <theo <at> thornhill.no> writes:

> Hi, Yuan!
>
> It seems 'prog-fill-reindent-defun' is broken after the latest changes
> to treesit-beginning-of-defun.  The culprit is that we now use remap
> instead of setting the beginning-of-defun-function.  What is the
> reasoning behind that change?  Can't we just rely on the variable
> beginning-of-defun-function?
>
> I see you mentioned it is inteded to be used as a command, but surely
> both should be possible?
>

This will also affect things like mark-defun and friends.  Should I
prepare a patch to fix this?

Theo

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

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit 
 impl
Date: Tue, 20 Dec 2022 20:08:09 -0800

Theodor Thornhill <theo <at> thornhill.no> writes:

> Hi, Yuan!
>
> It seems 'prog-fill-reindent-defun' is broken after the latest changes
> to treesit-beginning-of-defun.  The culprit is that we now use remap
> instead of setting the beginning-of-defun-function.  What is the
> reasoning behind that change?  Can't we just rely on the variable
> beginning-of-defun-function?

Not really, end-of-defun uses beginning/end-of-defun-function in a way
that’s incompatible with nested defuns[1]. So if we want to support
navigation nested defuns reliably we need to remap the commands instead.
In the future (ie emacs 30), we can extend the current
beginning/end-of-defun to support nested defuns, then we don’t need to
remap the commands anymore.

> I see you mentioned it is inteded to be used as a command, but surely
> both should be possible?

Could you remind me where is this function defined? I should have
updated it when I changed the defun navigation implementation. (It was
broken by my change before the defun nav change which you noticed, I
thought I’m going to fix it with the new defun nav functions, but I
forgot...)

Yuan


[1] For example, a nested defun like this:

def parent:
    (1)
    def child:
        return 0
(2) return 1
(3)

When point is at (1), end-of-defun calls beginning-of-defun-function
followed by end-of-defun-function to check if point is in a defun: if
point ends up after the starting point, then starting point is inside a
defun, and we can stop there. In this case, point ends up at
(3), because b-o-d-f goes to previous b-o-d, which is the beg of parent,
then e-o-d-f goes to (3), which is the end of that parent, and
end-of-defun stops at (3).

However, we should have gone to (2), which is the immediately following
end-of-defun.

Thanks,
Yuan

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit  impl
Date: Wed, 21 Dec 2022 06:58:53 +0100

On 21 December 2022 05:08:09 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>
>Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Hi, Yuan!
>>
>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>> instead of setting the beginning-of-defun-function.  What is the
>> reasoning behind that change?  Can't we just rely on the variable
>> beginning-of-defun-function?
>
>Not really, end-of-defun uses beginning/end-of-defun-function in a way
>that’s incompatible with nested defuns[1]. So if we want to support
>navigation nested defuns reliably we need to remap the commands instead.
>In the future (ie emacs 30), we can extend the current
>beginning/end-of-defun to support nested defuns, then we don’t need to
>remap the commands anymore.
>
>> I see you mentioned it is inteded to be used as a command, but surely
>> both should be possible?
>
>Could you remind me where is this function defined? I should have
>updated it when I changed the defun navigation implementation. (It was
>broken by my change before the defun nav change which you noticed, I
>thought I’m going to fix it with the new defun nav functions, but I
>forgot...)
>
>Yuan
>

It is in prog-mode.el, in the master branch. But the biggest issue now is that every function or command that relies on beginning-of-defun and  end-of-defun is broken. 


>
>[1] For example, a nested defun like this:
>
>def parent:
>    (1)
>    def child:
>        return 0
>(2) return 1
>(3)
>
>When point is at (1), end-of-defun calls beginning-of-defun-function
>followed by end-of-defun-function to check if point is in a defun: if
>point ends up after the starting point, then starting point is inside a
>defun, and we can stop there. In this case, point ends up at
>(3), because b-o-d-f goes to previous b-o-d, which is the beg of parent,
>then e-o-d-f goes to (3), which is the end of that parent, and
>end-of-defun stops at (3).
>
>However, we should have gone to (2), which is the immediately following
>end-of-defun.
>

That depends on the tactic chosen, right?

>Thanks,
>Yuan

Are you sure this isn't compatible?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit 
 impl
Date: Tue, 20 Dec 2022 22:50:04 -0800

Yuan Fu <casouri <at> gmail.com> writes:

> Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Hi, Yuan!
>>
>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>> instead of setting the beginning-of-defun-function.  What is the
>> reasoning behind that change?  Can't we just rely on the variable
>> beginning-of-defun-function?
>
> Not really, end-of-defun uses beginning/end-of-defun-function in a way
> that’s incompatible with nested defuns[1]. So if we want to support
> navigation nested defuns reliably we need to remap the commands instead.
> In the future (ie emacs 30), we can extend the current
> beginning/end-of-defun to support nested defuns, then we don’t need to
> remap the commands anymore.

I see the problem now... Many other functions uses
beginning/end-of-defun. I didn’t thought about that initially :-(

But I don’t want to make big changes to beg/end-of-deun, hmmm.

Yuan

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit  impl
Date: Wed, 21 Dec 2022 08:42:38 +0100

On 21 December 2022 07:50:04 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>
>Yuan Fu <casouri <at> gmail.com> writes:
>
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>>> Hi, Yuan!
>>>
>>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>>> instead of setting the beginning-of-defun-function.  What is the
>>> reasoning behind that change?  Can't we just rely on the variable
>>> beginning-of-defun-function?
>>
>> Not really, end-of-defun uses beginning/end-of-defun-function in a way
>> that’s incompatible with nested defuns[1]. So if we want to support
>> navigation nested defuns reliably we need to remap the commands instead.
>> In the future (ie emacs 30), we can extend the current
>> beginning/end-of-defun to support nested defuns, then we don’t need to
>> remap the commands anymore.
>
>I see the problem now... Many other functions uses
>beginning/end-of-defun. I didn’t thought about that initially :-(
>
>But I don’t want to make big changes to beg/end-of-deun, hmmm.
>
>Yuan


I think you can set the functions and remap, right? Maybe you can force the beginning-of-defun-function variant to choose the smallest scope as a default? Or just follow the same tactic the user set?
Theo

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

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit 
 impl
Date: Wed, 21 Dec 2022 21:00:43 -0800

Theodor Thornhill <theo <at> thornhill.no> writes:

> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>
>>Yuan Fu <casouri <at> gmail.com> writes:
>>
>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>
>>>> Hi, Yuan!
>>>>
>>>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>>>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>>>> instead of setting the beginning-of-defun-function.  What is the
>>>> reasoning behind that change?  Can't we just rely on the variable
>>>> beginning-of-defun-function?
>>>
>>> Not really, end-of-defun uses beginning/end-of-defun-function in a way
>>> that’s incompatible with nested defuns[1]. So if we want to support
>>> navigation nested defuns reliably we need to remap the commands instead.
>>> In the future (ie emacs 30), we can extend the current
>>> beginning/end-of-defun to support nested defuns, then we don’t need to
>>> remap the commands anymore.
>>
>>I see the problem now... Many other functions uses
>>beginning/end-of-defun. I didn’t thought about that initially :-(
>>
>>But I don’t want to make big changes to beg/end-of-deun, hmmm.
>>
>>Yuan
>
>
> I think you can set the functions and remap, right? Maybe you can
> force the beginning-of-defun-function variant to choose the smallest
> scope as a default? Or just follow the same tactic the user set?

Maybe, we can have beg-of-defun-function respect treesit-defun-tactic,
and end-of-defun-function simply jump to the end of the defun at point,
and remap the commands as we do now. I’ll experiment with that and see
if it works well.

Yuan

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit
 impl
Date: Thu, 22 Dec 2022 08:48:25 +0100

Yuan Fu <casouri <at> gmail.com> writes:

> Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>>
>>>Yuan Fu <casouri <at> gmail.com> writes:
>>>
>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>
>>>>> Hi, Yuan!
>>>>>
>>>>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>>>>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>>>>> instead of setting the beginning-of-defun-function.  What is the
>>>>> reasoning behind that change?  Can't we just rely on the variable
>>>>> beginning-of-defun-function?
>>>>
>>>> Not really, end-of-defun uses beginning/end-of-defun-function in a way
>>>> that’s incompatible with nested defuns[1]. So if we want to support
>>>> navigation nested defuns reliably we need to remap the commands instead.
>>>> In the future (ie emacs 30), we can extend the current
>>>> beginning/end-of-defun to support nested defuns, then we don’t need to
>>>> remap the commands anymore.
>>>
>>>I see the problem now... Many other functions uses
>>>beginning/end-of-defun. I didn’t thought about that initially :-(
>>>
>>>But I don’t want to make big changes to beg/end-of-deun, hmmm.
>>>
>>>Yuan
>>
>>
>> I think you can set the functions and remap, right? Maybe you can
>> force the beginning-of-defun-function variant to choose the smallest
>> scope as a default? Or just follow the same tactic the user set?
>
> Maybe, we can have beg-of-defun-function respect treesit-defun-tactic,
> and end-of-defun-function simply jump to the end of the defun at point,
> and remap the commands as we do now. I’ll experiment with that and see
> if it works well.

Sure!  Do you mean some treesit-specific code inside of
beginning-of-defun, or just making treesit-beginning-of-defun callable
non-interactively?

Theo

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

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit 
 impl
Date: Thu, 22 Dec 2022 00:51:33 -0800

Theodor Thornhill <theo <at> thornhill.no> writes:

> Yuan Fu <casouri <at> gmail.com> writes:
>
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>>> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>>>
>>>>Yuan Fu <casouri <at> gmail.com> writes:
>>>>
>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>
>>>>>> Hi, Yuan!
>>>>>>
>>>>>> It seems 'prog-fill-reindent-defun' is broken after the latest changes
>>>>>> to treesit-beginning-of-defun.  The culprit is that we now use remap
>>>>>> instead of setting the beginning-of-defun-function.  What is the
>>>>>> reasoning behind that change?  Can't we just rely on the variable
>>>>>> beginning-of-defun-function?
>>>>>
>>>>> Not really, end-of-defun uses beginning/end-of-defun-function in a way
>>>>> that’s incompatible with nested defuns[1]. So if we want to support
>>>>> navigation nested defuns reliably we need to remap the commands instead.
>>>>> In the future (ie emacs 30), we can extend the current
>>>>> beginning/end-of-defun to support nested defuns, then we don’t need to
>>>>> remap the commands anymore.
>>>>
>>>>I see the problem now... Many other functions uses
>>>>beginning/end-of-defun. I didn’t thought about that initially :-(
>>>>
>>>>But I don’t want to make big changes to beg/end-of-deun, hmmm.
>>>>
>>>>Yuan
>>>
>>>
>>> I think you can set the functions and remap, right? Maybe you can
>>> force the beginning-of-defun-function variant to choose the smallest
>>> scope as a default? Or just follow the same tactic the user set?
>>
>> Maybe, we can have beg-of-defun-function respect treesit-defun-tactic,
>> and end-of-defun-function simply jump to the end of the defun at point,
>> and remap the commands as we do now. I’ll experiment with that and see
>> if it works well.
>
> Sure!  Do you mean some treesit-specific code inside of
> beginning-of-defun, or just making treesit-beginning-of-defun callable
> non-interactively?

I played around, and it seems the best we can do is to simply setting
the b/e-of-defun-function. That’s also what cc-mode does. If cc-mode
does that for 20 years and no one complained, I think we are pretty
safe. Though we should still improve end-of-defun in Emacs 30.

So I did that :-)

Yuan

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 60197 <at> debbugs.gnu.org
Subject: Re: bug#60197: 30.0.50; beginning-of-defun broken after new treesit
 impl
Date: Thu, 22 Dec 2022 10:28:59 +0100

Yuan Fu <casouri <at> gmail.com> writes:

> Theodor Thornhill <theo <at> thornhill.no> writes:
>
> I played around, and it seems the best we can do is to simply setting
> the b/e-of-defun-function.

Yep!

> That’s also what cc-mode does. If cc-mode does that for 20 years and
> no one complained, I think we are pretty safe. Though we should still
> improve end-of-defun in Emacs 30.
>

Great news!

Thanks!

Theo

Previous Next

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