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

From: Yuan Fu <[email-masked]>
Date: Tue, 1 Oct 2024 23:14:15 -0700

> On Oct 1, 2024, at 10:49 AM, Juri Linkov <[email-masked]> wrote:
> 
>> 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?
> 
> I tried to do something like this, and everything works nicely with:
> 
>  @@ -2289,11 +2289,10 @@ treesit-forward-sexp
>           (node-at-point
>            (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))
>  +                   (or (treesit-node-match-p node-at-point 'text t)
>  +                       (not (treesit-thing-at
>  +                             (if (> arg 0) (point) (1- (point)))
>  +                             (treesit-thing-definition 'sexp nil)))))
>             (forward-sexp-default-function arg)
>             t)
>           (if (> arg 0)
> 
> The new logic is the following: if there is no sexp thing defined at point,
> then fall back to 'forward-sexp-default-function'.
> 
> Then after (setq js--treesit-sexp-nodes '("binary_expression"))
> 'C-M-f' in e.g.
> 
>  export const add = (a, b) => -!-a + b;
> 
> moves point to
> 
>  export const add = (a, b) => a + b-!-;
> 
> The condition (if (> arg 0) (point) (1- (point))) above
> is necessary to allow 'C-M-b' to move back to:
> 
>  export const add = (a, b) => -!-a + b;
> 
> Also the condition to make sure point is strictly inside node
> was removed to handle the case when point was at the beginning
> of the buffer:
> 
>  -!-
>  export const add = (a, b) => a + b;
> 
> to move after
> 
>  export-!- const add = (a, b) => a + b;
> 
> by 'forward-sexp-default-function'.

Sounds good. Feel free to install on master if you think it works well :-)

> 
>> 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?
> 
> I think we need to support both ways:
> 
> 1. opt-out - where sexp-thing definition is used by default,
>   and only text-thing allows users to override it;
> 
> 2. opt-in - where 'forward-sexp-default-function' is used by default,
>   and user can explicitly define what sexp-things are preferable
>   for navigation by treesit.
> 
> Then in the latter case the users could prefer to use
> treesit sexp navigation only for constructions with
> "invisible parens".  For example, in Ruby there are
> two interchangeable syntaxes for code blocks:
> 
> 1. curly braces {...} that are already handled
>   by 'forward-sexp-default-function';
> 
> 2. do...end that can't be handled by 'forward-sexp-default-function',
>   so treesit is coming to the rescue for the case of such
>   implicit braces.

Sounds good to me, I wonder if there are clever way to implement this. If there isn’t, we’d need to define two sets treesit-sexp functions and add a custom option to control which one to use. Seems a bit clunky to me.

Yuan