bug#68899: Treesitter's forward-sexp-function

[Yuan Fu <casouri@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]



> On Feb 2, 2024, at 4:57 PM, João Távora <joaotavora@gmail.com> wrote:
> 
> On Sat, Feb 3, 2024 at 12:42 AM João Távora <joaotavora@gmail.com> wrote:
> 
> > > This doesn't seem to break tests, assuming it's not in these 3 there
> > > were skipped because I don't have the grammar installed.
> >
> > Despite that, I think it's still wrong :-/ Now it moves too much,
> > i.e. it never stops moving.
> 
> This looks more promising.  Works well in my tests.
> 
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index c6b9d8ff4bc..cad7497fb74 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -2579,9 +2579,12 @@ treesit--navigate-thing
>              (setq parent (treesit-node-top-level parent thing t)
>                    prev nil
>                    next nil))
> -          ;; If TACTIC is `restricted', the implementation is very simple.
> +          ;; If TACTIC is `restricted', the implementation is reasonably simple.
>            (if (eq tactic 'restricted)
> -              (setq pos (funcall advance (if (> arg 0) next prev)))
> +              (setq pos (funcall advance (cond ((and (null next) (null prev))
> +                                                parent)
> +                                               ((> arg 0) next)
> +                                               (t prev))))
>              ;; For `nested', it's a bit more work:
>              ;; Move...
>              (if (> arg 0)

Thanks for looking into this, Joao. IME a very useful characteristic of forward-sexp is that it stays in the same “level” and doesn’t go up automatically when there’s no siblings (when there’s a closing delimiter). Eg, in an Elisp buffer, forward-sexp stops at the closing parenthesis, in C, it should stop at the closing bracket.

Also you don’t want to check for prev when moving forward, and vice versa, ie, we don’t want to check (null next) and (null prev) together.

So, how do you like this patch:

Yuan


[-- Attachment #2: forward-sexp.patch --]
[-- Type: application/octet-stream, Size: 2539 bytes --]

From 0afe667244caba04f061270cd2fc052ff6021130 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 3 Feb 2024 21:24:29 -0800
Subject: [PATCH] Improve treesit-forward-sexp behavior for leaf nodes
 (bug#68899)

treesit-forward-sexp uses treesit--navigate-thing with 'restricted'
tactic.  In this tactic we don't move over the parent thing.  However,
this makes forward-sexp useless for symbols when point is in the
symbol rather than at the beginning of it: in that case, the symbol is
considered parent and treesit-forward-sexp won't move to the end of
it.

To solve that, we allow to move across the parent even in 'restricted'
mode if the parent is a leaf node.

Here, "leaf node" is defined as "doesn't have named child node", i.e.,
if the node has only anonymous child nodes, it's still considered a
leaf node.  I don't know how useful this heuristic is, but it feels
like a good idea.

* lisp/treesit.el (treesit--navigate-thing): Move over parent in
'restricted' tactic if the parent is a leaf node.
---
 lisp/treesit.el | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/lisp/treesit.el b/lisp/treesit.el
index fab2ddd88e6..bf6b8dffde4 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -2662,9 +2662,21 @@ treesit--navigate-thing
             (setq parent (treesit-node-top-level parent thing t)
                   prev nil
                   next nil))
-          ;; If TACTIC is `restricted', the implementation is very simple.
+          ;; If TACTIC is `restricted', the implementation is simple.
+          ;; In principle we don't go to parent's beg/end for
+          ;; `restricted' tactic, but if the parent is a leaf node
+          ;; (e.g., a symbol), we do, lest you can't go to the end of
+          ;; a symbol with `forward-sexp' when point is in the symbol
+          ;; rather than at the beg of it.
           (if (eq tactic 'restricted)
-              (setq pos (funcall advance (if (> arg 0) next prev)))
+              (let ((restricted-parent
+                     (and (not (treesit-node-child parent 0 t))
+                          parent)))
+                (setq pos (funcall
+                           advance
+                           (if (> arg 0)
+                               (or next restricted-parent)
+                             (or prev restricted-parent)))))
             ;; For `nested', it's a bit more work:
             ;; Move...
             (if (> arg 0)
-- 
2.41.0


[-- Attachment #3: Type: text/plain, Size: 2 bytes --]