Fix treesit--things-around (bug#60355)

Current implementation of treesit--things-around only searches forward
for REGEXP and go up the tree until it finds a valid thing, if nothing
matches it gives up.  This makes it sometimes miss defuns.  The new
implementation tries multiple times (of search forward + go up) until
it exhausts all possible defun nodes.

* lisp/treesit.el (treesit--things-around): New implementation.
(treesit--navigate-defun): Refactor to use treesit-node-top-level to
simplify code, and add some guards in the predicate function.
* test/src/treesit-tests.el:
(treesit--ert-defun-navigation-elixir-program): New variable.
(treesit-defun-navigation-nested-4): New test.
This commit is contained in:
Yuan Fu 2022-12-27 17:02:03 -08:00
parent 7512b9025a
commit ba1ddea9da
No known key found for this signature in database
GPG key ID: 56E19BC57664A442
2 changed files with 88 additions and 61 deletions

View file

@ -1773,78 +1773,67 @@ sound things exists.
REGEXP and PRED are the same as in `treesit-thing-at-point'." REGEXP and PRED are the same as in `treesit-thing-at-point'."
(let* ((node (treesit-node-at pos)) (let* ((node (treesit-node-at pos))
;; NODE-BEFORE/AFTER = NODE when POS is completely in NODE, (result (list nil nil nil)))
;; but if not, that means point could be in between two
;; defun, in that case we want to use a node that's actually
;; before/after point.
(node-before (if (>= (treesit-node-start node) pos)
(save-excursion
(treesit-search-forward-goto node "" t t t))
node))
(node-after (if (<= (treesit-node-end node) pos)
(save-excursion
(treesit-search-forward-goto
node "" nil nil t))
node))
(result (list nil nil nil))
(pred (or pred (lambda (_) t))))
;; 1. Find previous and next sibling defuns. ;; 1. Find previous and next sibling defuns.
(cl-loop (cl-loop
for idx from 0 to 1 for idx from 0 to 1
for node in (list node-before node-after)
for backward in '(t nil) for backward in '(t nil)
;; Make sure we go in the right direction, and the defun we find
;; doesn't cover POS.
for pos-pred in (list (lambda (n) (<= (treesit-node-end n) pos)) for pos-pred in (list (lambda (n) (<= (treesit-node-end n) pos))
(lambda (n) (>= (treesit-node-start n) pos))) (lambda (n) (>= (treesit-node-start n) pos)))
;; If point is inside a defun, our process below will never ;; We repeatedly find next defun candidate with
;; return a next/prev sibling outside of that defun, effectively ;; `treesit-search-forward', and check if it is a valid defun,
;; any prev/next sibling is locked inside the smallest defun ;; until the node we find covers POS, meaning we've gone through
;; covering point, which is the correct behavior. That's because ;; every possible sibling defuns. But there is a catch:
;; when there exists a defun that covers point, ;; `treesit-search-forward' searches bottom-up, so for each
;; `treesit-search-forward' will first reach that defun, after ;; candidate we need to go up the tree and find the top-most
;; that we only go upwards in the tree, so other defuns outside ;; valid sibling, this defun will be at the same level as POS.
;; of the covering defun is never reached. (Don't use ;; Don't use `treesit-search-forward-goto', it skips nodes in
;; `treesit-search-forward-goto' as it breaks when NODE-AFTER is ;; order to enforce progress.
;; the last token of a parent defun: it will skip the parent when node
;; defun because it wants to ensure progress.) do (let ((cursor node)
do (cl-loop for cursor = (when node (iter-pred (lambda (node)
(save-excursion (and (string-match-p
(treesit-search-forward regexp (treesit-node-type node))
node regexp backward backward))) (or (null pred) (funcall pred node))
then (treesit-node-parent cursor) (funcall pos-pred node)))))
while cursor ;; Find the node just before/after POS to start searching.
if (and (string-match-p (save-excursion
regexp (treesit-node-type cursor)) (while (and cursor (not (funcall pos-pred cursor)))
(funcall pred cursor) (setq cursor (treesit-search-forward-goto
(funcall pos-pred cursor)) cursor "" backward backward t))))
do (setf (nth idx result) cursor))) ;; Keep searching until we run out of candidates.
(while (and cursor
(funcall pos-pred cursor)
(null (nth idx result)))
(setf (nth idx result)
(treesit-node-top-level cursor iter-pred t))
(setq cursor (treesit-search-forward
cursor regexp backward backward)))))
;; 2. Find the parent defun. ;; 2. Find the parent defun.
(setf (nth 2 result) (let ((cursor (or (nth 0 result) (nth 1 result) node))
(cl-loop for cursor = (or (nth 0 result) (iter-pred (lambda (node)
(nth 1 result) (and (string-match-p
node) regexp (treesit-node-type node))
then (treesit-node-parent cursor) (or (null pred) (funcall pred node))
while cursor (not (treesit-node-eq node (nth 0 result)))
if (and (string-match-p (not (treesit-node-eq node (nth 1 result)))
regexp (treesit-node-type cursor)) (< (treesit-node-start node)
(funcall pred cursor) pos
(not (member cursor result))) (treesit-node-end node))))))
return cursor)) (setf (nth 2 result)
(treesit-parent-until cursor iter-pred)))
result)) result))
(defun treesit--top-level-thing (node regexp &optional pred) (defun treesit--top-level-thing (node regexp &optional pred)
"Return the top-level parent thing of NODE. "Return the top-level parent thing of NODE.
REGEXP and PRED are the same as in `treesit-thing-at-point'." REGEXP and PRED are the same as in `treesit-thing-at-point'."
(let* ((pred (or pred (lambda (_) t)))) (treesit-node-top-level
;; `treesit-search-forward-goto' will make sure the matched node node (lambda (node)
;; is before POS. (and (string-match-p regexp (treesit-node-type node))
(cl-loop for cursor = node (or (null pred) (funcall pred node))))
then (treesit-node-parent cursor) t))
while cursor
if (and (string-match-p
regexp (treesit-node-type cursor))
(funcall pred cursor))
do (setq node cursor))
node))
;; The basic idea for nested defun navigation is that we first try to ;; The basic idea for nested defun navigation is that we first try to
;; move across sibling defuns in the same level, if no more siblings ;; move across sibling defuns in the same level, if no more siblings

View file

@ -940,7 +940,28 @@ and \"]\"."
[999]} [999]}
[110] [110]
" "
"Javascript source for navigation test.") "Bash source for navigation test.")
(defvar treesit--ert-defun-navigation-elixir-program
"[100]
[101]def bar() do
[999]end
[102]
[103]defmodule Example do[0]
[999] @impl true
[104] [1]def bar() do[2]
[999] end[3]
[105] [4]
[106] [5]def baz() do[6]
[999] end[7]
[107] [8]
[999]end[9]
[108]
[109]def bar() do
[999]end
[110]
"
"Elixir source for navigation test.")
(defvar treesit--ert-defun-navigation-nested-master (defvar treesit--ert-defun-navigation-nested-master
;; START PREV-BEG NEXT-END PREV-END NEXT-BEG ;; START PREV-BEG NEXT-END PREV-END NEXT-BEG
@ -1022,6 +1043,23 @@ the prev-beg, now point should be at marker 103\", etc.")
treesit--ert-defun-navigation-bash-program treesit--ert-defun-navigation-bash-program
treesit--ert-defun-navigation-nested-master))) treesit--ert-defun-navigation-nested-master)))
(ert-deftest treesit-defun-navigation-nested-4 ()
"Test defun navigation using Elixir.
This tests bug#60355."
(skip-unless (treesit-language-available-p 'bash))
;; Nested defun navigation
(let ((treesit-defun-tactic 'nested)
(pred (lambda (node)
(member (treesit-node-text
(treesit-node-child-by-field-name node "target"))
'("def" "defmodule")))))
(treesit--ert-test-defun-navigation
(lambda ()
(treesit-parser-create 'elixir)
(setq-local treesit-defun-type-regexp `("call" . ,pred)))
treesit--ert-defun-navigation-elixir-program
treesit--ert-defun-navigation-nested-master)))
(ert-deftest treesit-defun-navigation-top-level () (ert-deftest treesit-defun-navigation-top-level ()
"Test top-level only defun navigation." "Test top-level only defun navigation."
(skip-unless (treesit-language-available-p 'python)) (skip-unless (treesit-language-available-p 'python))