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

Previous Next

Package: emacs;

Reported by: Mickey Petersen <mickey <at> masteringemacs.org>

Date: Sat, 21 Sep 2024 05:13:01 UTC

Severity: normal

Merged with 74366

Found in version 30.0.50

Fixed in version 31.0.50

Done: Juri Linkov <juri <at> linkov.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Yuan Fu <casouri <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mickey Petersen <mickey <at> masteringemacs.org>, 73404 <at> debbugs.gnu.org
Subject: bug#73404: 30.0.50; [forward/kill/etc]-sexp commands do not behave as expected in tree-sitter modes
Date: Tue, 1 Oct 2024 23:14:15 -0700

> On Oct 1, 2024, at 10:49 AM, Juri Linkov <juri <at> linkov.net> 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



This bug report was last modified 131 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.