bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Yuan Fu]

Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 21 December 2022 05:08:09 CET, Yuan Fu <casouri@gmail.com> wrote:
>
>Theodor Thornhill <theo@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?

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Yuan Fu]

Yuan Fu <casouri@gmail.com> writes:

> Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 21 December 2022 07:50:04 CET, Yuan Fu <casouri@gmail.com> wrote:
>
>Yuan Fu <casouri@gmail.com> writes:
>
>> Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Yuan Fu]

Theodor Thornhill <theo@thornhill.no> writes:

> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri@gmail.com> wrote:
>>
>>Yuan Fu <casouri@gmail.com> writes:
>>
>>> Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Yuan Fu <casouri@gmail.com> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri@gmail.com> wrote:
>>>
>>>Yuan Fu <casouri@gmail.com> writes:
>>>
>>>> Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Yuan Fu]

Theodor Thornhill <theo@thornhill.no> writes:

> Yuan Fu <casouri@gmail.com> writes:
>
>> Theodor Thornhill <theo@thornhill.no> writes:
>>
>>> On 21 December 2022 07:50:04 CET, Yuan Fu <casouri@gmail.com> wrote:
>>>>
>>>>Yuan Fu <casouri@gmail.com> writes:
>>>>
>>>>> Theodor Thornhill <theo@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

bug#60197: 30.0.50; beginning-of-defun broken after new treesit impl

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Yuan Fu <casouri@gmail.com> writes:

> Theodor Thornhill <theo@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