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

Previous Next

Full log

Message #52 received at 73404 <at> debbugs.gnu.org (full text, mbox):

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

[Message part 1 (text/plain, inline)]

>>     export const add = (a, b) -!- => a + b;
>>
>> typing 'C-M-f' should jump to the end of the next sexp
>> (to the end of whole "binary_expression"):
>>
>>     export const add = (a, b) => a + b-!-;
>>
>> since only tree-sitter knows about "binary_expression",
>> so 'forward-sexp-default-function' can't be used here.
>
> Actually, I have one idea of possible heuristics:
>
> 1. first try 'forward-sexp-default-function'
> 2. if it crosses the boundary of sexp defined by 'treesit-thing-settings'
>    then use 'treesit-end-of-thing' instead
>
> This should work.  Ok, will try.

This is implemented now in the attached patch, and it works nicely.

The main rule is the following: 'forward-sexp-default-function'
should not go out of the current thing, neither go inside a sibling.
So we use 'treesit-end-of-thing' in such cases.  But when inside
a thing or outside a thing, use the default function.

This supposes that such things as "identifier" in js should be
removed from 'treesit-thing-settings' since identifiers should be
navigated the same way as such keywords as "export" and "const"
using 'forward-sexp-default-function'.

What should remain in 'treesit-thing-settings' are only grouping
constructs such as "parenthesized_expression" and "statement_block".

Removing "identifier" from 'treesit-thing-settings' exposed a problem
in 'treesit-navigate-thing'.  This line

  ((and (null next) (null prev)) parent)

tries to go out of the current thing to its parent,
thus breaking the main principle that 'forward-sexp'
should move forward across siblings only.  But removing
this line fixed the problem:

[treesit-forward-sexp.patch (text/x-diff, inline)]

diff --git a/lisp/treesit.el b/lisp/treesit.el
index db8f7a7595d..4fcdbe7fc56 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -2373,21 +2373,41 @@ treesit-forward-sexp
 What constitutes as text and source code sexp is determined
 by `text' and `sexp' in `treesit-thing-settings'."
   (interactive "^p")
-  (let ((arg (or arg 1))
-        (pred (or treesit-sexp-type-regexp '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))
-          (forward-sexp-default-function arg)
-          t)
-        (if (> arg 0)
-            (treesit-end-of-thing pred (abs arg) 'restricted)
-          (treesit-beginning-of-thing pred (abs arg) 'restricted))
+  (let* ((arg (or arg 1))
+         (pred (or treesit-sexp-type-regexp 'sexp))
+         (current-thing (treesit-thing-at (point) pred t))
+         (default-pos
+          (condition-case _
+              (save-excursion
+                (forward-sexp-default-function arg)
+                (point))
+            (scan-error nil)))
+         (default-pos (unless (eq (point) default-pos) default-pos))
+         (sibling-pos
+          (save-excursion
+            (and (if (> arg 0)
+                     (treesit-end-of-thing pred (abs arg) 'restricted)
+                   (treesit-beginning-of-thing pred (abs arg) 'restricted))
+                 (point))))
+         (sibling (when sibling-pos
+                    (if (> arg 0)
+                        (treesit-thing-prev sibling-pos pred)
+                      (treesit-thing-next sibling-pos pred)))))
+
+    ;; 'forward-sexp-default-function' should not go out of the current thing,
+    ;; neither go inside the next thing, neither go over the next thing
+    (or (when (and default-pos
+                   (or (null current-thing)
+                       (if (> arg 0)
+                           (< default-pos (treesit-node-end current-thing))
+                         (> default-pos (treesit-node-start current-thing))))
+                   (or (null sibling)
+                       (if (> arg 0)
+                           (< default-pos (treesit-node-start sibling))
+                         (> default-pos (treesit-node-end sibling)))))
+          (goto-char default-pos))
+        (when sibling-pos
+          (goto-char sibling-pos))
         ;; If we couldn't move, we should signal an error and report
         ;; the obstacle, like `forward-sexp' does.  If we couldn't
         ;; find a parent, we simply return nil without moving point,
@@ -2849,8 +2869,7 @@ treesit-navigate-thing
           (if (eq tactic 'restricted)
               (setq pos (funcall
                          advance
-                         (cond ((and (null next) (null prev)) parent)
-                               ((> arg 0) next)
+                         (cond ((> arg 0) next)
                                (t prev))))
             ;; For `nested', it's a bit more work:
             ;; Move...

Previous Next

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