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: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter) Date: Tue, 13 Dec 2022 09:11:41 -0700 Message-ID: References: <1B9F9B3A-A757-4A65-9653-CD0112EB8895@gmail.com> <5BB6E79A-B1FA-40F4-B48A-DADF63A30DDC@gmail.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000c7a72905efb7e048" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25994"; 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:12:49 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 1p57tg-0006NZ-20 for ged-emacs-devel@m.gmane-mx.org; Tue, 13 Dec 2022 17:12:48 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p57st-0000MB-Ds; Tue, 13 Dec 2022 11:11:59 -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 1p57sq-0000Ig-Mp for emacs-devel@gnu.org; Tue, 13 Dec 2022 11:11:56 -0500 Original-Received: from mail-ej1-x636.google.com ([2a00:1450:4864:20::636]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p57so-0001L2-D8 for emacs-devel@gnu.org; Tue, 13 Dec 2022 11:11:56 -0500 Original-Received: by mail-ej1-x636.google.com with SMTP id t17so37724195eju.1 for ; Tue, 13 Dec 2022 08:11:54 -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=LghI6UKheMXCSlbf/GlXh7ZcO6samq3K3Aj71+Lv+4g=; b=MIcx9VYdAorc4tSw14rscM8/DWdBW2Dun5WDn8q7pywkJit5P+y/lXU1ZWpNa2nUmr HYp9HeNrU3Hw/iZqVNYTY8aGOaueAoO3IopH0OFZduP5g33CvgR219D3ZFRO35hM60II opsXmSgvtItcNgMQmwEnw80ImXJ3/825VUqcnto0/+Z9ipZ5V+8Zv/kN240Cy2BLD/39 BKI4UbVdx3qHfJhEWxYER9XcznJpdYZqEsNeb/HDj4yloWUXsDZoBPWRfOxiIZ5tMWNK REAigKWzAEJo/7QE4+CHoETJuBw+U6PbCSNHmeGuxUgIsWlycuoEbSscBNPZLl7Mapo6 OQnQ== 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=LghI6UKheMXCSlbf/GlXh7ZcO6samq3K3Aj71+Lv+4g=; b=kqpC9V712d/dcwn8XTPEaVau52Kw5022oAK5a08jbYmzgzYj37mS6jEIh0r9LBjV7J GMy5PWESJtIGaDYDgJ+RtkvBMQ07uXDI30pBie3R2s6dahWgoNtLQAQ2i9wWig/XVsD5 CViYl3RqsWyHdz3rKl8rQ2/IhDjda4XZNsjpcnxCe45dKycXTyJztsxNH/53XCcp0u/5 c5bJBMT2fQI6gvcoCqy2XyO//VbCqn26ECk6nTc4VCpsK0LayT9Nz1HYM42M2aNLvTjk FTw7A0yKOsELiFlNHALglRlyCJuuQ3LMRIbuAv79jcg+a2rL0tz8H1URari3yxwX1t23 6ltQ== X-Gm-Message-State: ANoB5pnV8rqTrspzDt7sE/skC2FxtQttVuQ92qanL8AYjltCctxKxDyC m2p5i6avvvCvw8BZnCgdefKx5MTihC7cLaCdPEw= X-Google-Smtp-Source: AA0mqf7qQLfsCi9bU59aYD2oBqWQ6gCxJpFW5lpcVFi/cklA+pBHXTLqsaCnHvoHUMQor/m7NrbcrT0Zp3/oaE8LpCE= X-Received: by 2002:a17:907:c241:b0:7c1:69a3:19bd with SMTP id tj1-20020a170907c24100b007c169a319bdmr761720ejc.563.1670947912785; Tue, 13 Dec 2022 08:11:52 -0800 (PST) In-Reply-To: <5BB6E79A-B1FA-40F4-B48A-DADF63A30DDC@gmail.com> Received-SPF: pass client-ip=2a00:1450:4864:20::636; envelope-from=jaopaulolc@gmail.com; helo=mail-ej1-x636.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:301326 Archived-At: --000000000000c7a72905efb7e048 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Great job, Yuan. The new `treesit--navigate-defun' does work as documented. However, the behavior does feel a little awkward to me. For instance, if I am within a function I would expect to go to its beginning when pressing C-M-a, and not to the beginning of the first leaf function when searching backward. Although the current behavior might be desirable to some users and should be possible as well. The navigation style that I came up with for bash-ts-mode is to navigate only to functions in the same level or higher of the tree. That seems to be the motions I usually rely on when writing/editing code. But that might just be me. Nevertheless, I do agree with you that large changes would be required to enable proper navigation in the presence of nested functions via the existing beginning/end-of-defun functions. On Mon, Dec 12, 2022 at 10:20 PM Yuan Fu wrote: > Thanks to Alan and Jo=C3=A3o, I have revamped the defun navigation of > tree-sitter, now major modes can get accurate defun navigation by setting > treesit-defun-type-regexp (as before), and switch between top-level-only > mode and nested defun mode by treesit-defun-tactic (new). > > The change I pushed doesn=E2=80=99t replace the old functions, my plan is= to > replace the existing ones if we think this change is ok for the release > branch. After experimenting and implementing this, I don=E2=80=99t think = the > current beginning/end-of-defun can support nested navigation without larg= e > changes. So I=E2=80=99d prefer we use new beg/end-of-defun commands in tr= ee-sitter > major modes. And we later think about merging the two. > > I also added comprehensive tests to the defun navigation[1], which should > give us more peace in mind. Although this is a large change, I hope it ca= n > stay on the release branch because (1) it fixes current bugs (2) it has a > comprehensive test. > > Yuan > > [1] For tests, see > treesit-defun-navigation-nested-1 > treesit-defun-navigation-nested-2 > treesit-defun-navigation-nested-3 > treesit-defun-navigation-top-level > > It tests all possible defun navigations starting from all possible > starting positions (that I can think of), so it should be pretty > comprehensive. I currently have test for python, js, and bash. New tests > can be easily added. > > The test program is like the following, we mark positions with markers > like [100] > > [100] > [101]class Class1(): > [999] prop =3D 0 > [102] > [103]class Class2():[0] > [104] [1]def method1(): > [999] [2]return 0[3] > [105] [4] > [106] [5]def method2(): > [999] [6]return 0[7] > [107] [8] > [999] prop =3D 0[9] > [108] > [109]class Class3(): > [999] prop =3D 0[10] > [110] > > Then we have a list of positions like this, basically saying =E2=80=9Cif = point is > at marker X, and we find the next/previous beginning/end of defun, point > should end up at Y=E2=80=9D, and we have X for all the possible positions= in a > nested defun, and Y for all four operations. So the first line says =E2= =80=9Cif we > start at marker 0, and find prev-beg-of-defun, we should end up at marker > 103=E2=80=9D. > > ;; START PREV-BEG NEXT-END PREV-END NEXT-BEG > '((0 103 105 102 106) ; Between Beg of parent & 1st sibling. > (1 103 105 102 106) ; Beg of 1st sibling. > (2 104 105 102 106) ; Inside 1st sibling. > (3 104 107 102 109) ; End of 1st sibling. > (4 104 107 102 109) ; Between 1st sibling & 2nd sibling. > (5 104 107 102 109) ; Beg of 2nd sibling. > (6 106 107 105 109) ; Inside 2nd sibling. > (7 106 108 105 109) ; End of 2nd sibling. > (8 106 108 105 109) ; Between 2nd sibling & end of parent. > (9 103 110 102 nil) ; End of parent. > > (100 nil 102 nil 103) ; Before 1st parent. > (101 nil 102 nil 103) ; Beg of 1st parent. > (102 101 108 nil 109) ; Between 1st & 2nd parent. > (103 101 108 nil 109) ; Beg of 2nd parent. > (110 109 nil 108 nil) ; After 3rd parent. > ) > > > --=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 --000000000000c7a72905efb7e048 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Great job, Yuan.

The new `treesit--navigate-defun&#= 39; does work as documented. However, the behavior does feel a little awkwa= rd to me.

For instance, if I am within a function I would expect to = go to its beginning when pressing C-M-a, and not to the beginning of the fi= rst leaf function when searching backward. Although the current behavior mi= ght be desirable to some users and should be possible as well.

The n= avigation style that I came up with for bash-ts-mode is to navigate only to= functions=C2=A0in the same level or higher of the tree. That seems to be t= he motions I usually rely on when writing/editing code. But that might just= be me.

Nevertheless, I do agree with you that large changes would b= e required to enable proper navigation=C2=A0in the presence=C2=A0of nested = functions=C2=A0via the existing beginning/end-of-defun functions.
=
On Mon= , Dec 12, 2022 at 10:20 PM Yuan Fu <casouri@gmail.com> wrote:
Thanks to Alan and Jo=C3=A3o, I have revamped the defun n= avigation of tree-sitter, now major modes can get accurate defun navigation= by setting treesit-defun-type-regexp (as before), and switch between top-l= evel-only mode and nested defun mode by treesit-defun-tactic (new).

The change I pushed doesn=E2=80=99t replace the old functions, my plan is t= o replace the existing ones if we think this change is ok for the release b= ranch. After experimenting and implementing this, I don=E2=80=99t think the= current beginning/end-of-defun can support nested navigation without large= changes. So I=E2=80=99d prefer we use new beg/end-of-defun commands in tre= e-sitter major modes. And we later think about merging the two.

I also added comprehensive tests to the defun navigation[1], which should g= ive us more peace in mind. Although this is a large change, I hope it can s= tay on the release branch because (1) it fixes current bugs (2) it has a co= mprehensive test.

Yuan

[1] For tests, see
treesit-defun-navigation-nested-1
treesit-defun-navigation-nested-2
treesit-defun-navigation-nested-3
treesit-defun-navigation-top-level

It tests all possible defun navigations starting from all possible starting= positions (that I can think of), so it should be pretty comprehensive. I c= urrently have test for python, js, and bash. New tests can be easily added.=

The test program is like the following, we mark positions with markers like= [100]

[100]
[101]class Class1():
[999]=C2=A0 =C2=A0 prop =3D 0
[102]
[103]class Class2():[0]
[104]=C2=A0 =C2=A0 [1]def method1():
[999]=C2=A0 =C2=A0 =C2=A0 =C2=A0 [2]return 0[3]
[105]=C2=A0 =C2=A0 [4]
[106]=C2=A0 =C2=A0 [5]def method2():
[999]=C2=A0 =C2=A0 =C2=A0 =C2=A0 [6]return 0[7]
[107]=C2=A0 =C2=A0 [8]
[999]=C2=A0 =C2=A0 prop =3D 0[9]
[108]
[109]class Class3():
[999]=C2=A0 =C2=A0 prop =3D 0[10]
[110]

Then we have a list of positions like this, basically saying =E2=80=9Cif po= int is at marker X, and we find the next/previous beginning/end of defun, p= oint should end up at Y=E2=80=9D, and we have X for all the possible positi= ons in a nested defun, and Y for all four operations. So the first line say= s =E2=80=9Cif we start at marker 0, and find prev-beg-of-defun, we should e= nd up at marker 103=E2=80=9D.

;; START PREV-BEG NEXT-END PREV-END NEXT-BEG
=C2=A0 '((0 103 105 102 106) ; Between Beg of parent & 1st sibling.=
=C2=A0 =C2=A0 (1 103 105 102 106) ; Beg of 1st sibling.
=C2=A0 =C2=A0 (2 104 105 102 106) ; Inside 1st sibling.
=C2=A0 =C2=A0 (3 104 107 102 109) ; End of 1st sibling.
=C2=A0 =C2=A0 (4 104 107 102 109) ; Between 1st sibling & 2nd sibling.<= br> =C2=A0 =C2=A0 (5 104 107 102 109) ; Beg of 2nd sibling.
=C2=A0 =C2=A0 (6 106 107 105 109) ; Inside 2nd sibling.
=C2=A0 =C2=A0 (7 106 108 105 109) ; End of 2nd sibling.
=C2=A0 =C2=A0 (8 106 108 105 109) ; Between 2nd sibling & end of parent= .
=C2=A0 =C2=A0 (9 103 110 102 nil) ; End of parent.

=C2=A0 =C2=A0 (100 nil 102 nil 103) ; Before 1st parent.
=C2=A0 =C2=A0 (101 nil 102 nil 103) ; Beg of 1st parent.
=C2=A0 =C2=A0 (102 101 108 nil 109) ; Between 1st & 2nd parent.
=C2=A0 =C2=A0 (103 101 108 nil 109) ; Beg of 2nd parent.
=C2=A0 =C2=A0 (110 109 nil 108 nil) ; After 3rd parent.
=C2=A0 =C2=A0 )




--
Jo= =C3=A3o Paulo L. de Carvalho
Ph.D Computer Science | =C2=A0IC-UNICAMP | = Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Albe= rta | Edmonton, AB - Canada
--000000000000c7a72905efb7e048--