Subject: Re: bug#78703: beginning-of-defun and friends still wrong in typescript-ts-mode

From: Yuan Fu <[email-masked]>
Date: Mon, 9 Jun 2025 23:02:33 -0700

> On Jun 7, 2025, at 7:03 AM, Troy Brown <[email-masked]> wrote:
> 
> On Sat, Jun 7, 2025 at 2:09 AM Yuan Fu <[email-masked]> 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