Package: emacs;
Reported by: Daniel Colascione <dancol <at> dancol.org>
Date: Thu, 5 Jun 2025 23:41:02 UTC
Severity: normal
View this message in rfc822 format
From: Daniel Colascione <dancol <at> dancol.org> To: Yuan Fu <casouri <at> gmail.com> Cc: Eli Zaretskii <eliz <at> gnu.org>, Troy Brown <brownts <at> troybrown.dev>, 78703 <at> debbugs.gnu.org Subject: bug#78703: beginning-of-defun and friends still wrong in typescript-ts-mode Date: Tue, 10 Jun 2025 00:24:54 -0700
Yuan Fu <casouri <at> gmail.com> writes: >> 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. Experimenting with a different UI paradigm doesn't justify introducing an inconsistency to behavior that's worked well for literally decades. If I'm using, say, c++-ts-mode, my navigation commands should do the same thing they do in c++-mode. Plenty of code as well as muscle memories rely on this behavior. The "tactic" concept is an unnecessary layer of indirection. If operation A takes you to place X and operation B takes you to different place Y, the way to express the difference between operations A and B is to make them _different commands_, not by twiddling some global switch. When you change tree-sitter "strategies" right now, you're silently turning one command into another command, that's confusing for everyone. Please give these "strategies" individual command names. We have beginning-of-defun and beginning-of-defun-comments, not a knob that alters beginning-of-defun. > 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. Why does there need to be a tree-sitter version of prog-fill-reindent-defun? Isn't it enough that tree-sitter provide the low-level syntactic analysis for prog-fill-reindent-defun to do its job? Why the high level hook? > 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. Modes use generally these "customization points" to _implement_ the familiar behavior, not to give them random different user-visible semantics. > 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. Which now means prog-fill-reindent-defun can indent something other than what mark-defun highlights? That seems odd to me. Tree sitter's job is syntactic analysis, not UI differentiation. >> 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. The default should be to match behavior that's been stable for decades. Use of tree sitter should be an implementation detail for users. If we want to provide UI to better handle nested defuns, this UI should go in prog-mode.el and rely on mode-provided syntactic analysis, not just delegate to a mode function that does different random stuff in each mode. >> 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++) Namespaces aren't defuns and c++-ts-mode shouldn't be indenting their contents by a level either. c++-ts-mode is unusable without hacks like pragmatically editing the indentation rules in user configuration. >> 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. Yes, because a namespace isn't a defun. >> 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.