Package: emacs;
Reported by: Daniel Colascione <dancol <at> dancol.org>
Date: Thu, 5 Jun 2025 23:41:02 UTC
Severity: normal
Message #29 received at 78703 <at> debbugs.gnu.org (full text, mbox):
From: Yuan Fu <casouri <at> gmail.com> To: Troy Brown <brownts <at> troybrown.dev> Cc: Eli Zaretskii <eliz <at> gnu.org>, Daniel Colascione <dancol <at> dancol.org>, 78703 <at> debbugs.gnu.org Subject: Re: bug#78703: beginning-of-defun and friends still wrong in typescript-ts-mode Date: Mon, 9 Jun 2025 23:02:33 -0700
> On Jun 7, 2025, at 7:03 AM, Troy Brown <brownts <at> troybrown.dev> wrote:
>
> On Sat, Jun 7, 2025 at 2:09 AM Yuan Fu <casouri <at> gmail.com> wrote:
>>
>> So for this tactic, point should move out of the enclosing defun if it is inside a defun; and if point isn’t inside any defun it should move to the previous defund-beginning?
>
> Care must be taken when talking about being "inside" or "not inside" a
> defun. Unless point is before, after, or between top-level defuns, it
> will always be inside some defun. I previously suggested
> differentiating behavior based on whether point was at a defun
> boundary. By boundary, I mean point is either immediately before or
> immediately after a defun.
>
> It's also important to point out that classes, namespaces, packages,
> etc. could all be defuns, not just functions. Therefore the nesting
> as being described is not a pathological exercise, but a much more
> common occurrence than one might initially consider.
>
> As far as behavior, I think the way I've observed it working in
> non-Tree-sitter modes was more intuitive than the current Tree-sitter
> behavior (but I haven't exhaustively checked). Additionally, I think
> it should work as described in the documentation. Much of what I'm
> saying was already expressed in Bug#68664. There, I tried to codify
> how I believed the non-Tree-sitter modes were behaving and how
> intuitively I thought it should work for Tree-sitter modes.
>
> Regardless of what tactic is configured, `treesit-beginning-of-defun`
> must go to the beginning of the enclosing defun when point is not at a
> defun boundary. I think this is fundamental or the assumptions made
> when using `beginning-of-defun` in the general case no longer hold
> (such as its use in `prog-fill-reindent-defun`). I think the tactic
> should only come into play when point is already at a defun boundary.
Consider that existing code in Emacs isn’t set in stone, it’s not really that fundamental that beginning-of-defun must work in a way that makes prog-fill-reindent-defun behave desirably. prog-fill-reindent-defun uses beginning-of-defun because we don’t have better choices before tree-sitter. In tree-sitter major modes, what we’ve been doing is to make the existing commands customizable so tree-sitter can provide a tree-sitter version of it. We’ve done this for forward-sexp: we added forward-sexp-function. Some commands already have customization points long ago, like beginning-of-defun, which has beginning-of-defun-function.
So I added prog-fill-reindent-defun-function and a tree-sitter version treesit-fill-reindent-defun. The tree-sitter implementation uses treesit-defun-at-point, so it doesn’t even need to concern with tactics.
Now in tree-sitter major modes, prog-fill-reindent-defun should always indent the enclosing defun.
> After that, I'm less concerned about its behavior, but think the
> current `nested` behavior of visiting the previous sibling defun until
> there are no more, then visiting the parent defun makes sense. The
> key difference is that you only visit the previous sibling if point is
> at a defun boundary...otherwise visit the defun containing point.
Hmmm, it doesn’t feel very convenient, you’d need to first adjust your point to be precisely at the boundary, then press C-M-a/e? IMO that adds too much overhead. I added a tactic `parent-first` that always move to the beginning/end of the enclosing defun. People that prefers this kind of defun movement can use this tactic.
>
> Furthermore, consider the following `top-level` tactic example using
> `c++-ts-mode`. Here, we have a C++ namespace (which is considered a
> defun for C++) as well as a "doSomething" function defun within the
> namespace. If we place point on the "printf" and press `M-q`,
> triggering `prog-fill-reindent-defun`, we'll see that the entire
> namespace has been re-indented (including "int i;"). I'd argue that
> was not intuitively what I would have expected to happen. Instead, I
> would have expected only "doSomething" to have been re-indented.
>
> ```cpp
> // -*- mode: c++-ts
> #include <cstdio>
>
> namespace Hello
> {
> int i;
>
> void doSomething(void)
> {
> printf("doSomething\n");
> }
> };
>
> // Local Variables:
> // treesit-defun-tactic: top-level
> // End:
> ```
>
> If the behavior of `treesit-beginning-of-defun` was to first move to
> the beginning of the enclosing defun when point is not at a defun
> boundary (as previously described), the expected behavior of only
> reindenting "doSomething" would have occurred. If
> `treesit-beginning-of-defun` worked the way I propose, and you really
> wanted to re-indent the entire namespace, it would have been as simple
> as moving to the beginning of the enclosing defun and triggering
> `prog-fill-reindent-defun` there (i.e., `C-M-a` `M-q`).
>
>> Any suggestions for a good name for this tactic? Right now we have `nested` and `top-level`.
>
> See above. I don't think a new tactic is necessary, just a change in
> how `treesit-beginning-of-defun` works when point is not at a defun
> boundary, regardless of the configured tactic.
Yuan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.