bug#59662: 29.0.50; [PATCH] Add treesit--indent-defun

[Yuan Fu <casouri@gmail.com>]

> On Dec 7, 2022, at 1:53 PM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> 
> 
> On 7 December 2022 22:40:31 CET, Yuan Fu <casouri@gmail.com> wrote:
>> 
>> 
>>> On Dec 7, 2022, at 12:06 PM, Theodor Thornhill <theo@thornhill.no> wrote:
>>> 
>>> Yuan Fu <casouri@gmail.com> writes:
>>> 
>>>> Theodor Thornhill <theo@thornhill.no> writes:
>>>> 
>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>> 
>>>>>>> From: Theodor Thornhill <theo@thornhill.no>
>>>>>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>>>>>> of text editors" <bug-gnu-emacs@gnu.org>, Eli Zaretskii <eliz@gnu.org>,
>>>>>>> casouri@gmail.com, 59662@debbugs.gnu.org
>>>>>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>>>>> 
>>>>>>> Theodor Thornhill <theo@thornhill.no> writes:
>>>>>>> 
>>>>>>>> On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29@yahoo.es> wrote:
>>>>>>>>> Theodor Thornhill <theo@thornhill.no> writes:
>>>>>>>>> 
>>>>>>>>>> Right - thanks.  However, this makes me wonder - should we really be
>>>>>>>>>> setting mark here? I see that c-indent-defun does not, and it feels
>>>>>>>>>> weird that indenting adds to the mark ring.  What do you think?  My
>>>>>>>>>> first patch used
>>>>>>>>>> 
>>>>>>>>>> (indent-region
>>>>>>>>>> (treesit-node-start node)
>>>>>>>>>> (treesit-node-end node))
>>>>>>>>>> 
>>>>>>>>>> Which behaves similarly to c-indent-defun.
>>>>>>>>>> 
>>>>>>>>>> See attached patch.
>>>>>>>>> 
>>>>>>>>> I don't have a strong opinion, but if we can indent without setting the
>>>>>>>>> mark, I think it'd be a cleaner command.  Specially if c-indent-defun
>>>>>>>>> doesn't set the mark either.
>>>>>>>> 
>>>>>>>> Yeah, we don't need to set the mark. Thanks for the feedback :)
>>>>>>>> Theo
>>>>>>> 
>>>>>>> Eli, do you have any opinion on this matter? Should we implement this
>>>>>>> function without using marks?  That will make the function a smidge
>>>>>>> bigger, but would not mess up the mark-ring.  I think that's desirable,
>>>>>>> at least.
>>>>>> 
>>>>>> IMO, it is better not to set the mark, indeed.
>>>>> 
>>>>> Agreed, see attached patch.  What do you think, Yuan and Eli?
>>>>> 
>>>>> Should we expose the internal helper I used, or maybe this function
>>>>> could just as well live in treesit.c?
>>>>> 
>>>>> Theo
>>>>> 
>>>> 
>>>> Very nice, I applied this patch with some changes to the function. I
>>>> think using treesit-beginning/end-of-defun is more robust.
>>>> 
>>>> Yuan
>>> 
>>> Thanks!  Would you mind describing why?
>> 
>> Yes, sorry. I’m planning to change treesit-beginning-of-defun and treesit-end-of-defun so they can behave differently according to user configuration, basically they can decide whether to skip nested defuns or not. If we use these functions rather than re-implementing their logic (to some extent) in c-ts-mode-indent-defun, any improvement to defun navigation would automatically benefit c-ts-mode-indent-defun.
>> 
>> Yuan
> 
> Thanks for explaining your rationale. 
> 
> By the way, there's an issue with the patch you applied:
> 
> void
> foo()
> {
>  ...
> }
> 
> If point is at the v in void, the current implementation will not reinvent. I didn't get time to debug yet, though. Possibly that will be fixed after your rework as well.

Ah, thanks, I’ll fix that. And I’ll ask for your opinion before making changes next time, however innocent I thought the change is :-)

Yuan