From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_Paulo_Labegalini_de_Carvalho?= Newsgroups: gmane.emacs.devel Subject: Re: Code navigation for sh-mode with Tree-sitter Date: Tue, 13 Dec 2022 09:00:44 -0700 Message-ID: References: <1B9F9B3A-A757-4A65-9653-CD0112EB8895@gmail.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000a3154e05efb7b943" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8654"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Monnier , Alan Mackenzie , emacs-devel@gnu.org To: Yuan Fu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Dec 13 17:02:08 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 1p57jM-00020F-Dx for ged-emacs-devel@m.gmane-mx.org; Tue, 13 Dec 2022 17:02:08 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p57iV-0002CC-Ce; Tue, 13 Dec 2022 11:01:16 -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 1p57iJ-00024b-Ab for emacs-devel@gnu.org; Tue, 13 Dec 2022 11:01:05 -0500 Original-Received: from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p57iD-0004D3-NO for emacs-devel@gnu.org; Tue, 13 Dec 2022 11:00:59 -0500 Original-Received: by mail-ej1-x62b.google.com with SMTP id x22so37591425ejs.11 for ; Tue, 13 Dec 2022 08:00:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=90iE3U59P2vvIa/H1AOs1lKaFw/wrdUHu+d0c5xsC6s=; b=lyf+K5la4pHFiziaULkWs1yoCrtT2SU5xB1RaIZsOTZzJO54tTG2dbyvXIjGJ5gMma c1dSYoB68kmHjLMbWz/bLC58UHqi6M4wkj4Tjldfw8agfOSkWSx0tiZjS3DgM00j8kRc /exBm3qMb4CVLAAHjteBiXP7H07zZb0RQ5EgKScpp3+/Xxbghbd/UU1mdArLVG/ZbeyT Al7g3IL/E7IRWmVjLyKCxHTAgkCeia4bZm7iftEOTxFZm+5AnnfkMPhk4gYmbe+x3Wym EJycOliqSaddCzqW7ZuHzSF0cE5D8v1l0XzqiBcal3vDIE4uAHpkkVzylKbYPBJPHRYQ PEMw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=90iE3U59P2vvIa/H1AOs1lKaFw/wrdUHu+d0c5xsC6s=; b=u8GwlRfv2l8LrsaHLEZl9guio7nfhVRBdljotjalFH5UA+aoMQyrOErw8IUY/O3lYq Le2kBztDiKFv5DENZWX5hvYe6bplKjbXOGIEnxevNuL6HpckLgddvizutavkBnmQVfi6 28ai2QgHzcQizD522v3uKIjqwlfKou3ngvk6JEQf4VT13Uek2z5VN6HEzf/9S/lp79GA 7QAwpQPupi8awuItXaXP/qa5rMohMfNhVQx+SKXYn+avzeW2DygJMlOeQ9mZsjwZR81G yA+CanAt4HefhLCNulLDsiNRndam6ls8rQDO9Y3JpP/i4Poo53kL42fvRea+pYaYojDw xU4A== X-Gm-Message-State: ANoB5pnB1o4AMHDwMJunfXASbp/+/komD9Po0iQOsjQ96q9QoFPaLB+a 7AiqhPku934wXNajUrQ10V7QnrLQYi8XYiG8XdI= X-Google-Smtp-Source: AA0mqf7FoROjSKzi5DVZ9wS6VtKsHko8otpmZeXV+5KoJQrdl+0CwTvJlS42n+0/CO0NbWhUYp8d0/rTs+X/N9J+v30= X-Received: by 2002:a17:906:2810:b0:7b2:7b45:2bf6 with SMTP id r16-20020a170906281000b007b27b452bf6mr65004797ejc.467.1670947256078; Tue, 13 Dec 2022 08:00:56 -0800 (PST) In-Reply-To: Received-SPF: pass client-ip=2a00:1450:4864:20::62b; envelope-from=jaopaulolc@gmail.com; helo=mail-ej1-x62b.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, HTML_MESSAGE=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:301325 Archived-At: --000000000000a3154e05efb7b943 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > > Ok, I fianlly finished the defun navigation work, thanks to Alan=E2=80=99= 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=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 sw= itch > between top-level/nested with treesit-defun-tactic. > Cool. Thanks for all the work. I will test it and report my experience in the other thread. > > 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? > I agree. Those are generic enough to be useful to other use cases. +(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?). Yes, I was thinking about performance but out of intuition and not evidence= . > Diff artifact? > Yes, my bad. 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 gener= al > version of defun navigation! > No problem at all. Trying things out has been a great opportunity for me to learn. Thank you for taking the time to review my patch. Regarding the new navigation functions the do work as documented for bash-ts-mode. However, the overall behavior feels awkward to me. I will elaborate on the new thread. --=20 Jo=C3=A3o Paulo L. de Carvalho Ph.D Computer Science | IC-UNICAMP | Campinas , SP - Brazil Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canad= a joao.carvalho@ic.unicamp.br joao.carvalho@ualberta.ca --000000000000a3154e05efb7b943 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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-l= evel defun navigation for free, provided that the major mode sets treesit-d= efun-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 betwee= n top-level/nested with treesit-defun-tactic.

Cool= . Thanks for all the work. I will test it and report my experience in the o= ther thread.
=C2=A0

Anyway, here is some minor points of the patch:

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

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

I agree. Those are generic enough to be useful to other use cas= es.=C2=A0

+(defmacro sh-mode--treesit-parent-defun (node)
+=C2=A0 "Return nearest function-node that surrounds NODE, if any, or = nil.
+
+This macro can be used to determine if NODE is within a function.=C2=A0 If=
+so, the macro evaluates to the nearest function-node and parent of NODE. +Otherwise it evaluates to NIL."
+=C2=A0 `(treesit-parent-until ,node 'sh-mode--treesit-defun-p))
+
+(defmacro sh-mode--treesit-oldest-parent-in-defun (node)
+=C2=A0 "Return oldest parent of NODE in common function, if any, or N= IL.
+
+This function returns the oldest parent of NODE such that the common
+parent is the nearest function-node."
+=C2=A0 `(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?).

Y= es, I was thinking about performance but out of intuition and not evidence.=
=C2=A0
Di= ff artifact?

Yes, my bad.=C2=A0=C2=A0
Looks good to me, but I didn't scrutinize it line-by-line. If the new s= ystem 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 ha= rd work, but your work definitely helped me to implement the more general v= ersion of defun navigation!

No problem at all. Try= ing things out has been a great opportunity for me to learn. Thank you for = taking the time to review my patch.

Regarding the new navigation fun= ctions the do work as documented for bash-ts-mode. However, the overall beh= avior feels awkward to me. I will elaborate on the new thread.
<= div>
--
Jo=C3=A3o Paulo L. de Carvalho
Ph.D C= omputer Science | =C2=A0IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral= Research Fellow | University of Alberta | Edmonton, AB - Canada
--000000000000a3154e05efb7b943--