Re: Code navigation for sh-mode with Tree-sitter

[Yuan Fu <casouri@gmail.com>]

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



> On Dec 7, 2022, at 9:20 AM, João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> I found some minor issues and misspellings on the docstring.
> 
> Here is an updated version of the patch.
> 
> On Tue, Dec 6, 2022 at 6:12 PM João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> 
> On Tue, Dec 6, 2022 at 4:50 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > However, with negative arguments that does not happen, as
> > `sh-mode--treesit-beginning-of-defun' moves point to (beginning of) the
> > closest sibling function (after point) and
> > `sh-mode--treesit-end-of-defun' moves
> > point to (end of) the closest sibling function (before point).  In this
> > case, the selected functions to which point move to are not the same.
> 
> Please read the docstring of `end-of-defun-function`, because I suspect
> that you are confused about what it's supposed to do.  E.g. it's not
> supposed to "move point to (end of) the closest sibling function", so
> I think you'll need to set it to a different function than
> `sh-mode--treesit-end-of-defun`.
> 
> Indeed. I was trying to impose the behavior I desired to achieve instead of the intended use. I corrected that in my patch. 
> 
> Looking forward to comments and suggestions for the patch.

Ok, I fianlly finished the defun navigation work, thanks to Alan’s suggestion, your experiment, and honorable sacrifices of my many brain cells. Now tree-sitter should give you both nested and top-level defun navigation for free, provided that the major mode sets treesit-defun-type-regexp. I’ll start a new thread about it.

I hope you can try and see if it works for bash-ts-mode, when you have time. You should only need to apply the attached patch and (goto-char (treesit--navigate-defun (point) …)) should just work. You can switch between top-level/nested with treesit-defun-tactic.

Anyway, here is some minor points of the patch:

+;;; Tree-sitter navigation
+
+(defun sh-mode--treesit-defun-p (node)
+  "Return t if NODE is a function and nil otherwise."
+  (string-match treesit-defun-type-regexp
+                (treesit-node-type node)))
+
+(defun sh-mode-treesit-not-cs-p (node)
+  "Return t if NODE is *not* a compound-statement and nil otherwise."
+  (lambda (p)
+    (not (string-match "compound_statement"
+                       (treesit-node-type p)))))

We probably want this to be an internal function, too, right?

+(defmacro sh-mode--treesit-parent-defun (node)
+  "Return nearest function-node that surrounds NODE, if any, or nil.
+
+This macro can be used to determine if NODE is within a function.  If
+so, the macro evaluates to the nearest function-node and parent of NODE.
+Otherwise it evaluates to NIL."
+  `(treesit-parent-until ,node 'sh-mode--treesit-defun-p))
+
+(defmacro sh-mode--treesit-oldest-parent-in-defun (node)
+  "Return oldest parent of NODE in common function, if any, or NIL.
+
+This function returns the oldest parent of NODE such that the common
+parent is the nearest function-node."
+  `(treesit-parent-while ,node 'sh-mode--treesit-not-cp-p))

I'd prefer we use functions when functions will do, unless you have
particular reasons to use macros (for performance?).

+
+(defun sh-mode-treesit-beginning-of-defun (&optional arg)
+  "Tree-sitter `beginning-of-defun' function.
+ARG is the same as in `beginning-of-defun'.
+
+This function can be used either to set `beginning-of-defun-function'
+or as a direct replacement to `beginning-of-defun'.
+
+This function works the same way the non-tree-sitter
+`beginning-of-defun' when point is not within a function.  It diverges
+from `beginning-of-defun' when inside a function by moving point to
+the beginning of the closest enclosing function when ARG is positive.
+When ARG is negative and inside a function, point is moved to the
+beginning of closest sibling function, if any.  Otherwise the search
+continues from the function enclosing the current function."
+  (interactive "P")
+  (let ((arg (or arg 1))
+        (target nil)
+        (curr (treesit-node-at (point)))
+        (function treesit-defun-type-regexp))
+    (if (> arg 0)
+        ;; Go backward.
+        (while (and (> arg 0) curr)
+          (if (string= (treesit-node-type curr) "function")
+              (setq curr (treesit-node-parent curr)))
+          (setq target (sh-mode--treesit-parent-defun curr))
+          (unless target +            (let ((maybe-target nil))
+              (setq maybe-target (treesit-search-forward curr
+                                                         function
+                                                         t))

Diff artifact?

+              (setq target (or (treesit-node-top-level maybe-target)
+                               maybe-target))))
+          (when target
+            (setq curr target))
+          (setq arg (1- arg)))
+      ;; Go forward.
+      (while (and (< arg 0) curr)
+        (setq target nil)
+        (if (sh-mode--treesit-defun-p curr)
+            (setq curr (treesit-node-at
+                        (treesit-node-end
+                         (treesit-node-parent curr)))))
+        (let ((parent-defun (sh-mode--treesit-parent-defun curr)))
+          (while (and (not target)
+                      parent-defun)
+            (setq target (sh-mode--treesit-next-sibling-defun curr))
+            (unless target
+              (setq curr (treesit-node-next-sibling parent-defun))
+              (setq parent-defun
+                    (sh-mode--treesit-parent-defun curr))))
+          (unless target
+            (let ((maybe-target nil))
+              (setq maybe-target (treesit-search-forward curr
+                                                         function))
+              (setq target (or (treesit-node-top-level maybe-target)
+                               maybe-target))))
+          (when target
+            (setq curr target)))
+        (setq arg (1+ arg))))
+    (when target
+      (goto-char (treesit-node-start target)))))
+
+(defun sh-mode--treesit-end-of-defun-function ()
+  "Tree-sitter `end-of-defun-function' function."
+  (let ((curr (treesit-node-at (point))))
+    (if curr
+        (let ((curr-defun (sh-mode--treesit-parent-defun curr)))
+          (if curr-defun
+              (goto-char (treesit-node-end curr-defun)))))))
+
+(defun sh-mode-treesit-end-of-defun (&optional arg)
+  "Tree-sitter `end-of-defun' function.
+
+This function is a more opinionated version of `end-of-defun' and can
+be used as its direct replacement.
+
+This function works the same way the non-tree-sitter `end-of-defun'
+when point is not within a function.  It diverges from `end-of-defun'
+when inside a function by moving point to the end of the closest
+enclosing function when ARG is positive.  When ARG is negative and
+inside a function, point is moved to the end of closest sibling
+function, if any.  Otherwise the search continues from the function
+enclosing the current function."
+  (interactive "P")
+  (let ((arg (or arg 1))
+        (curr (treesit-node-at (point)))
+        (target nil)
+        (function treesit-defun-type-regexp))
+    (if (> arg 0)
+        ;; Go forward.
+        (while (and (> arg 0) curr)
+          (setq target (sh-mode--treesit-parent-defun curr))
+          (unless target
+            (setq target (treesit-search-forward curr
+                                                 function))
+            (when (and target
+                       (sh-mode--treesit-parent-defun target))
+              (setq target (treesit-node-top-level target))))
+          (when target
+            (setq curr target))
+          (setq arg (1- arg)))
+      ;; Go backward.
+      (while (and (< arg 0) curr)
+        (setq target nil)
+        (if (sh-mode--treesit-parent-defun curr)
+            (setq curr
+                  (or (sh-mode--treesit-oldest-parent-in-defun curr)
+                      curr)))
+        (let* ((prev-defun (sh-mode--treesit-prev-sibling-defun curr))
+               (prev-defun-end (treesit-node-at
+                                (treesit-node-end prev-defun))))
+          (if (and prev-defun (treesit-node-eq curr prev-defun-end))
+              (setq curr prev-defun)))
+        (let ((parent-defun (sh-mode--treesit-parent-defun curr)))
+          (while (and (not target)
+                      parent-defun)
+            (setq target (sh-mode--treesit-prev-sibling-defun curr))
+            (unless target
+              (setq curr (treesit-node-prev-sibling parent-defun))
+              (setq parent-defun
+                    (sh-mode--treesit-parent-defun curr))))
+          (unless target
+            (let ((maybe-target nil))
+              (setq maybe-target (treesit-search-forward curr
+                                                         function
+                                                         t))
+              (setq target (or (treesit-node-top-level maybe-target)
+                               maybe-target))))
+          (when target
+            (setq curr target)))
+        (setq arg (1+ arg))))
+    (when target
+      (goto-char (treesit-node-end target)))))
+

Looks good to me, but I didn't scrutinize it line-by-line. If the new system works well, bash-ts-mode (and other major modes) wouldn't need to implemente its own navigation function. Sorry for not end up using your hard work, but your work definitely helped me to implement the more general version of defun navigation!


Yuan


[-- Attachment #2: bash-defun.diff --]
[-- Type: application/octet-stream, Size: 883 bytes --]

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index e170d18afeb..271f693dc14 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1612,6 +1612,7 @@ bash-ts-mode
 This mode automatically falls back to `sh-mode' if the buffer is
 not written in Bash or sh."
   (when (treesit-ready-p 'bash)
+    (treesit-parser-create 'bash)
     (setq-local treesit-font-lock-feature-list
                 '(( comment function)
                   ( command declaration-command keyword string)
@@ -1619,6 +1620,7 @@ bash-ts-mode
                   ( bracket delimiter misc-punctuation operator)))
     (setq-local treesit-font-lock-settings
                 sh-mode--treesit-settings)
+    (setq-local treesit-defun-type-regexp "function_definition")
     (treesit-major-mode-setup)))
 
 (advice-add 'bash-ts-mode :around #'sh--redirect-bash-ts-mode