From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Yuan Fu Newsgroups: gmane.emacs.devel Subject: Re: Code navigation for sh-mode with Tree-sitter Date: Mon, 12 Dec 2022 20:55:40 -0800 Message-ID: References: <1B9F9B3A-A757-4A65-9653-CD0112EB8895@gmail.com> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Content-Type: multipart/mixed; boundary="Apple-Mail=_949473DB-9426-4FD5-A4A5-355AB852A623" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="11262"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Monnier , Alan Mackenzie , emacs-devel@gnu.org To: =?utf-8?Q?Jo=C3=A3o_Paulo_Labegalini_de_Carvalho?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Dec 13 05:56:38 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p4xLK-0002f1-Av for ged-emacs-devel@m.gmane-mx.org; Tue, 13 Dec 2022 05:56:38 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p4xKX-00047k-UK; Mon, 12 Dec 2022 23:55:49 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4xKW-00047A-DF for emacs-devel@gnu.org; Mon, 12 Dec 2022 23:55:48 -0500 Original-Received: from mail-pl1-x62f.google.com ([2607:f8b0:4864:20::62f]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p4xKU-0006YG-BK for emacs-devel@gnu.org; Mon, 12 Dec 2022 23:55:48 -0500 Original-Received: by mail-pl1-x62f.google.com with SMTP id a9so14393439pld.7 for ; Mon, 12 Dec 2022 20:55:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=Otter2ziL+U8+9RKsoxk0ji2n4RDWXFyaakgABXvAw0=; b=GmrHZJSYTHcYWXJJS4IO9X1STBOIw1YWLUcgmph3aO3cuBZc+IqPoWw5alh8258LFR 9KIbhJyU8+ayaCo1+QAI5vJ/aOVB20lHk8QPC3tQOtQ1vupF+TTYJniO9ii+lm9Z776H 1Ci+8U76abV9dMZwovBISiXQIxaoiwyFkjFjGIn3N1yJuUByLMOTbCXoD6uqc1iPT7it YZ9SzhruDGtbN/f2cx5aPFgRfwucN0/b2VHQy+nrlbZXvWgXDsc13UjNh/aADd/HQ00z VyGnmZRGreVDkM1pI6hyiCurxXqtrLJxElyBuiYItO2f0pTqD1BzD0S3Iqdr7AHatn5K ntAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Otter2ziL+U8+9RKsoxk0ji2n4RDWXFyaakgABXvAw0=; b=tMybp+nGXUELp3Oyufxtq6MuxhMTvnODep6kKyGAIb6Xw5Am4ZNFKviiB10tW56paD 22JSNV7zL0D/Od3eCXBX+hmkYl8wL87QhIS1gU/AY5KZvZQhGKj/eJcxFwEJGKjNHTxY MX6IQGBuhHVOvD7AnSlYty/VvuRJjDTNLHvn0wJZvL722NnLskh1KRBE40ek6LUzROdH PRSgGtnioNdA4+JNTl13AmRCba6E//CEqR20WuQ609VDrXqG0ZgFOyHKyJAHnS0d/P+G BHDjPqHQSL99bvt8/pKFGPF7IJE09oA28mhRurKJOR/6XLbqyypKSw6YfOuAtx+xeRN2 5VVg== X-Gm-Message-State: ANoB5pkH1LX9Cbw+hesHaeiZ8WwR7Fe/wRmWSGv8Mp2vyEEunVAzqTXT PXn9vqdpa6uUqzgT7TeF/28= X-Google-Smtp-Source: AA0mqf4QCe08aN1QgjPBIawdmANj6RAljK/ICw3iDD0osl7WpW+Rvb06g6pS61TpZcSEpznirKiJdQ== X-Received: by 2002:a17:903:3316:b0:189:7819:e5c with SMTP id jk22-20020a170903331600b0018978190e5cmr18840572plb.6.1670907344577; Mon, 12 Dec 2022 20:55:44 -0800 (PST) Original-Received: from smtpclient.apple (cpe-172-117-161-177.socal.res.rr.com. [172.117.161.177]) by smtp.gmail.com with ESMTPSA id m8-20020a1709026bc800b0018853dd8832sm7306314plt.4.2022.12.12.20.55.42 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 12 Dec 2022 20:55:44 -0800 (PST) In-Reply-To: X-Mailer: Apple Mail (2.3696.120.41.1.1) Received-SPF: pass client-ip=2607:f8b0:4864:20::62f; envelope-from=casouri@gmail.com; helo=mail-pl1-x62f.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:301298 Archived-At: --Apple-Mail=_949473DB-9426-4FD5-A4A5-355AB852A623 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Dec 7, 2022, at 9:20 AM, Jo=C3=A3o Paulo Labegalini de Carvalho = wrote: >=20 > I found some minor issues and misspellings on the docstring. >=20 > Here is an updated version of the patch. >=20 > On Tue, Dec 6, 2022 at 6:12 PM Jo=C3=A3o Paulo Labegalini de Carvalho = wrote: >=20 >=20 > On Tue, Dec 6, 2022 at 4:50 PM Stefan Monnier = 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. >=20 > 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`. >=20 > Indeed. I was trying to impose the behavior I desired to achieve = instead of the intended use. I corrected that in my patch.=20 >=20 > Looking forward to comments and suggestions for the patch. Ok, I fianlly finished the defun navigation work, thanks to Alan=E2=80=99s= 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=E2=80=99ll 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) =E2=80=A6)) 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=3D (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 --Apple-Mail=_949473DB-9426-4FD5-A4A5-355AB852A623 Content-Disposition: attachment; filename=bash-defun.diff Content-Type: application/octet-stream; x-unix-mode=0644; name="bash-defun.diff" Content-Transfer-Encoding: 7bit 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 --Apple-Mail=_949473DB-9426-4FD5-A4A5-355AB852A623--