bug#73404: 30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes

[Yuan Fu <casouri@gmail.com>]

> On Sep 29, 2024, at 9:56 AM, Juri Linkov <juri@linkov.net> wrote:
> 
>> Note that you can affect the behavior of tree-sitter sexp movement by
>> defining the sexp “thing” in treesit-thing-settings. Js-ts-mode defines one
>> (js--treesit-sexp-nodes) and it only consider some nodes as sexp. You might
>> be able to tweak the sexp movement to your liking by changing it, or
>> directly modifying the definition for `sexp’ in treesit-thing-settings.
>> 
>>>> Do you think the examples I listed in the original bug report match
>>>> your expectations? If so, then it is probably OK to close the bug report.
>>> 
>>> Yes, I do, but let's wait for others to chime in if they have opinions
>>> on this.
> 
> Here are some ideas how to cover more use cases.
> 
> Suppose that a user wants to disable tree-sitter sexp movement
> completely to use the default forward-sexp-default-function.
> The natural way to do this would be set the list of nodes to nil:
> 
>  (setq js--treesit-sexp-nodes nil)
> 
> However, this currently doesn't work, and requires a change like this:
> 
>  @@ -2290,10 +2290,12 @@ treesit-forward-sexp
>            (treesit-node-at (point) (treesit-language-at (point)))))
>       (or (when (and node-at-point
>                      ;; Make sure point is strictly inside node.
>  -                   (< (treesit-node-start node-at-point)
>  -                      (point)
>  -                      (treesit-node-end node-at-point))
>  -                   (treesit-node-match-p node-at-point 'text t))
>  +                   (<= (treesit-node-start node-at-point)
>  +                       (point)
>  +                       (treesit-node-end node-at-point))
>  +                   (or (treesit-node-match-p node-at-point 'text t)
>  +                       (not (treesit-node-match-p node-at-point 'sexp t))
>  +                       ))
>             (forward-sexp-default-function arg)
>             t)
>           (if (> arg 0)
> 
> Now, the next case: what if the user wants to use the default
> forward-sexp-default-function except for the 'binary_expression'
> like "a + b" where `C-M-f' should move from "a" to the end of "b":
> 
>  export const add = (a, b) => -!-a + b;
> 
> should move to
> 
>  export const add = (a, b) => a + b;
> 
>                                    ^1
> 
> The best way for the user would be to customize:
> 
>  (setq js--treesit-sexp-nodes '("binary_expression"))
> 
> But this is not yet handled by the condition above:
> 
>  (not (treesit-node-match-p node-at-point 'sexp t))
> 
> because 'node-at-point' is "identifier".
> So we need to use 'treesit-parent-until'
> to check if all parent nodes match
> 'js--treesit-sexp-nodes'.  Then it will find
> the parent "binary_expression".
> 
> I believe something like this will make
> treesit-forward-sexp more customizable.

The user can modify treesit-thing-settings to alter the behavior of sexp navigation, they don’t necessarily need to use js--treesit-sexp-nodes. Maybe we should add a test for (treesit-thing-defined-p 'sexp nil) in treesit-forward-sexp? 

Your second example sounds useful, but right now the premise of tree-sitter sexp movement is to use the parse tree primarily, and only use the default sexp movement for comments and strings. What you envisioned seems to be the other way around: use default sexp movement by default, and only use tree-sitter movement under certain conditions. Is that few lines of change able to make such big difference in the logic?

Yuan