unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Code navigation for sh-mode with Tree-sitter
@ 2022-12-03 20:23 João Paulo Labegalini de Carvalho
  2022-12-03 21:46 ` Alan Mackenzie
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-03 20:23 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1930 bytes --]

Hi all,

I am working on basic navigation for sh-mode. My idea is that in sh-mode
invoking C-M-a or C-M-e moves point to the beginning/end of the
surrounding function if the point was inside of said function or to the
beginning/end of the next/previous top-level function otherwise.

I got the functions to do so in the attached patch (I did not include the
C-M-a with a negative argument yet).

If the functions are defined and used in isolation they work as intended.
However, because `end-of-defun' calls `beginning-of-defun-raw' (which,
AFAIK, uses `beginning-of-defun-function') it causes the C-M-e to not work
when the point is not inside of a function.

What I think is happening is that, when `end-of-defun' calls
`beginning-of-defun-raw' it brings point to the beginning of a top-level
function, so the subsequent call to `end-of-defun' moves point to the start
location, does it make it seems as the point did not move.

I have also noticed that calling the private function I defined directly
via M-: brings the point to a slightly different location. For example:

<---- point before C-M-e
A {
} <--- point after M-: (sh-mode--treesit-end-of-defun)
<---- pint after C-M-e

Calling sh-mode--treesit-end-of-defun brings the point right after the
closing curly brace and with C-M-e the point is positioned at the next line.

That might be due to additional processing done inside `end-of-defun'.

So, would it be ok to override `beginning-of-defun' and `end-of-defun' and
thus have bash-ts-mode use the functions I wrote directly? If so, how could
I do that? If not, how can I fix the "miss-behavior"?

I would appreciate all feedback and suggestions.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #1.2: Type: text/html, Size: 2423 bytes --]

[-- Attachment #2: 0001-Basic-navigation-for-sh-mode.patch --]
[-- Type: text/x-patch, Size: 3759 bytes --]

From ec7e99cc93fb8f11d94e4c02c558cbfd756237a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20P=2E=20L=2E=20de=20Carvalho?=
 <jaopaulolc@gmail.com>
Date: Sat, 3 Dec 2022 12:55:27 -0700
Subject: [PATCH] Basic navigation for sh-mode

---
 lisp/progmodes/sh-script.el | 63 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 408ebfc045..5885261c01 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,7 +1619,13 @@ bash-ts-mode
                   ( bracket delimiter misc-punctuation operator)))
     (setq-local treesit-font-lock-settings
                 sh-mode--treesit-settings)
-    (treesit-major-mode-setup)))
+    (setq-local treesit-defun-prefer-top-level t)
+    (setq-local treesit-defun-type-regexp "function_definition")
+    (treesit-major-mode-setup)
+    (setq-local beginning-of-defun-function
+                #'sh-mode--treesit-beginning-of-defun)
+    (setq-local end-of-defun-function
+                #'sh-mode--treesit-end-of-defun)))
 
 (advice-add 'bash-ts-mode :around #'sh--redirect-bash-ts-mode
             ;; Give it lower precedence than normal advice, so other
@@ -3364,5 +3370,60 @@ sh-mode--treesit-settings
    '((["$"]) @font-lock-misc-punctuation-face))
   "Tree-sitter font-lock settings for `sh-mode'.")
 
+\f
+;;; Tree-sitter navigation
+
+(defmacro get-fun-or-nil (node)
+  `((treesit-parent-until ,node
+                        (lambda (p)
+                          (string= (treesit-node-type p)
+                                   "function_definition"))))
+
+(defun sh-mode--treesit-beginning-of-defun (&optional arg)
+  "Tree-sitter `beginning-of-defun' function.
+ARG is the same as in `beginning-of-defun'."
+  (let ((arg (or arg 1))
+        (target-node nil)
+        (node-at-point nil)
+        (function treesit-defun-type-regexp))
+    (if (> arg 0)
+        ;; Go backward.
+        (while (> arg 0)
+          (setq node-at-point (treesit-node-at (point)))
+          (if (string= (treesit-node-type node-at-point) "function")
+              (setq node-at-point (treesit-node-parent node-at-point)))
+          (setq target-node (get-fun-or-nil node-at-point))
+          (unless target-node
+            (setq maybe-target-node (treesit-search-forward node-at-point
+                                                            function
+                                                            t))
+            (setq target-node (or (treesit-node-top-level maybe-target-node)
+                                  maybe-target-node)))
+          (when target-node
+            (goto-char (treesit-node-start target-node)))
+          (setq arg (1- arg)))
+      ;; Go forward.
+      (while (< arg 0)
+        (setq arg (1+ arg))))))
+
+(defun sh-mode--treesit-end-of-defun ()
+  "Tree-sitter `end-of-defun' function."
+  (let ((node-at-point nil)
+        (target-node nil)
+        (function treesit-defun-type-regexp))
+    (setq node-at-point (treesit-node-at (point)))
+    (setq target-node (get-fun-or-nil node-at-point))
+    (unless target-node
+      (setq target-node (treesit-search-forward node-at-point
+                                                function))
+      (when (and target-node
+                 (treesit-parent-until target-node
+                                       (lambda (p)
+                                         (string= (treesit-node-type p)
+                                                  function))))
+        (setq target-node (treesit-node-top-level target-node))))
+    (when target-node
+      (goto-char (treesit-node-end target-node)))))
+
 (provide 'sh-script)
 ;;; sh-script.el ends here
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-03 20:23 Code navigation for sh-mode with Tree-sitter João Paulo Labegalini de Carvalho
@ 2022-12-03 21:46 ` Alan Mackenzie
  2022-12-05 15:24   ` João Paulo Labegalini de Carvalho
  0 siblings, 1 reply; 35+ messages in thread
From: Alan Mackenzie @ 2022-12-03 21:46 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho; +Cc: emacs-devel

Hello, João.

On Sat, Dec 03, 2022 at 13:23:25 -0700, João Paulo Labegalini de Carvalho wrote:
> Hi all,

> I am working on basic navigation for sh-mode. My idea is that in sh-mode
> invoking C-M-a or C-M-e moves point to the beginning/end of the
> surrounding function if the point was inside of said function or to the
> beginning/end of the next/previous top-level function otherwise.

> I got the functions to do so in the attached patch (I did not include the
> C-M-a with a negative argument yet).

> If the functions are defined and used in isolation they work as intended.
> However, because `end-of-defun' calls `beginning-of-defun-raw' (which,
> AFAIK, uses `beginning-of-defun-function') it causes the C-M-e to not work
> when the point is not inside of a function.

Yes.  beginning-of-defun and end-of-defun are broken, and have been for
a long time.  They cannot, in general, work, for the reason you've just
noted.

The problem has been raised on emacs-devel before, without the problem
getting solved.  CC Mode, for example, works around the problem by
binding c-beginning-of-defun and c-end-of-defun directly to C-M-a and
C-M-e, bypassing the offending code.

The tactic of using beginning-of-defun-raw is only valid in certain
circumstances, and is an inappropriate inefficiency in modes such as
the one you're writing, where it is just as easy to go directly to an
end of defun as a beginning of defun.

> What I think is happening is that, when `end-of-defun' calls
> `beginning-of-defun-raw' it brings point to the beginning of a top-level
> function, so the subsequent call to `end-of-defun' moves point to the start
> location, does it make it seems as the point did not move.

Yes.  I suggest you submit a bug report for this bug.

To work properly, beginning/end-of-defun need to know whether the
starting point is inside a defun or between defuns.  Your patch includes
a macro which determines this (as CC Mode includes a function which
determines this).  Possibly, a proper bug fix might include a function
supplied by the major mode for this analysis.

> I have also noticed that calling the private function I defined directly
> via M-: brings the point to a slightly different location. For example:

> <---- point before C-M-e
> A {
> } <--- point after M-: (sh-mode--treesit-end-of-defun)
> <---- pint after C-M-e

> Calling sh-mode--treesit-end-of-defun brings the point right after the
> closing curly brace and with C-M-e the point is positioned at the next line.

This is C-M-e deciding that the end of defun should be defined as
somewhere else other than the end of the function.  It seems as though
there ought to be an end-of-defun-raw function (which would be usable by
programs) and an end-of-defun which puts point where interactive use
supposedly wants it to be.

> That might be due to additional processing done inside `end-of-defun'.

Indeed.

> So, would it be ok to override `beginning-of-defun' and `end-of-defun' and
> thus have bash-ts-mode use the functions I wrote directly? If so, how could
> I do that? If not, how can I fix the "miss-behavior"?

As already mentioned, you could bind C-M-a and C-M-e directly to your
new functions in the major mode map.  Other than that, I'm convinced the
misbehaviour cannot be fixed without fixing the basic functions in
emacs-lisp/lisp.el.  Hence the suggestion of a bug report.

> I would appreciate all feedback and suggestions.



> -- 
> João Paulo L. de Carvalho
> Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
> Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
> joao.carvalho@ic.unicamp.br
> joao.carvalho@ualberta.ca

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-03 21:46 ` Alan Mackenzie
@ 2022-12-05 15:24   ` João Paulo Labegalini de Carvalho
  2022-12-05 20:12     ` Stefan Monnier
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-05 15:24 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

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

On Sat, Dec 3, 2022 at 2:46 PM Alan Mackenzie <acm@muc.de> wrote:

> Yes.  beginning-of-defun and end-of-defun are broken, and have been for
> a long time.  They cannot, in general, work, for the reason you've just
> noted.
>

Thanks for confirming this, Alan. For a moment I thought that I was making
one of those elisp newbie mistakes.


> The problem has been raised on emacs-devel before, without the problem
> getting solved.  CC Mode, for example, works around the problem by
> binding c-beginning-of-defun and c-end-of-defun directly to C-M-a and
> C-M-e, bypassing the offending code.
>

I think for now I will use the same workaround.


> The tactic of using beginning-of-defun-raw is only valid in certain
> circumstances, and is an inappropriate inefficiency in modes such as
> the one you're writing, where it is just as easy to go directly to an
> end of defun as a beginning of defun.
>
> > What I think is happening is that, when `end-of-defun' calls
> > `beginning-of-defun-raw' it brings point to the beginning of a top-level
> > function, so the subsequent call to `end-of-defun' moves point to the
> start
> > location, does it make it seems as the point did not move.
>
> Yes.  I suggest you submit a bug report for this bug.
>

I will put some time into this and see if I can come up with a patch before
flagging it as a bug.


> To work properly, beginning/end-of-defun need to know whether the
> starting point is inside a defun or between defuns.  Your patch includes
> a macro which determines this (as CC Mode includes a function which
> determines this).  Possibly, a proper bug fix might include a function
> supplied by the major mode for this analysis.
>

Indeed. I am not sure if that would be as easy as without tree-sitter, but
such a function is definitely handy and easy to implement thanks to Yuan's
efforts to bring tree-sitter to core.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 3291 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-05 15:24   ` João Paulo Labegalini de Carvalho
@ 2022-12-05 20:12     ` Stefan Monnier
  2022-12-05 21:29       ` Alan Mackenzie
  2022-12-06 15:51       ` João Paulo Labegalini de Carvalho
  0 siblings, 2 replies; 35+ messages in thread
From: Stefan Monnier @ 2022-12-05 20:12 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho; +Cc: Alan Mackenzie, emacs-devel

>> Yes.  beginning-of-defun and end-of-defun are broken, and have been for
>> a long time.  They cannot, in general, work, for the reason you've just
>> noted.
> Thanks for confirming this, Alan. For a moment I thought that I was making
> one of those elisp newbie mistakes.

Well, "broken" is an opinion, not a fact.  AFAIK they can work.  Maybe
not as efficiently as some other API, but that's a different question.

> I think for now I will use the same workaround.

I'd urge you to try and find a way to make it work without such
a workaround.

>> Yes.  I suggest you submit a bug report for this bug.
> I will put some time into this and see if I can come up with a patch
> before flagging it as a bug.

Once you get it to work without the above workaround, you'll be able to
write a much better bug report.

>> To work properly, beginning/end-of-defun need to know whether the
>> starting point is inside a defun or between defuns.

Calling beginning-of-defun-function followed by end-of-defun-function
(and comparing the resulting position to the start position) should be
sufficient to let you know whether or not you're inside the function
whose definition starts most closely before point.


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-05 20:12     ` Stefan Monnier
@ 2022-12-05 21:29       ` Alan Mackenzie
  2022-12-05 21:56         ` Stefan Monnier
  2022-12-06 15:51       ` João Paulo Labegalini de Carvalho
  1 sibling, 1 reply; 35+ messages in thread
From: Alan Mackenzie @ 2022-12-05 21:29 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: João Paulo Labegalini de Carvalho, emacs-devel

Hello, Stefan.

On Mon, Dec 05, 2022 at 15:12:22 -0500, Stefan Monnier wrote:
> >> Yes.  beginning-of-defun and end-of-defun are broken, and have been for
> >> a long time.  They cannot, in general, work, for the reason you've just
> >> noted.
> > Thanks for confirming this, Alan. For a moment I thought that I was making
> > one of those elisp newbie mistakes.

> Well, "broken" is an opinion, not a fact.  AFAIK they can work.  Maybe
> not as efficiently as some other API, but that's a different question.

In which case, why not amend emacs-lisp-mode so that it works for .el
buffers?  It currently doesn't do so in the general case.  I don't
dispute it does so for commonly encountered special cases.

For example, insert the defuns foo and bar into lisp/emacs-lisp/lisp.el
just before end-of-defun-moves-to-eol, the pertinent bit of the result
looking like:

1. (defun foo ())    (defun bar ())

3. (defvar end-of-defun-moves-to-eol t
4.   "Whether `end-of-defun' moves to eol before doing anything else.
5.   Set this to nil if this movement adversely affects the buffer's
6.   major mode's decisions about context.")

Put point at the beginning of L1.  C-M-e goes not to the end of foo, but
the beginning of bar.  A small point, but, I believe, an important one
to the OP João.

Now with point at this position, the start of bar, type C-M-e again.
Point jumps to BOL 7, having moved forward over both bar and
end-of-defun-moves-to-eol.

Are you maintaining that it is just my opinion that this behaviour is
broken?

We had a long discussion about this sort of thing in 2009.  It was
broken then, and it's still broken now.

Even the existence of the variable end-of-defun-moves-to-eol is so crass
it can't be other than broken.  João's patch contains a
beginning-of-defun-function and and end-of-defun-function which
intrinsically work.  Emacs's end-of-defun messes this up with it's
ridiculous call to beginning-of-defun-raw, followed by a later call to
(beginning-of-defun-raw -1).

> > I think for now I will use the same workaround.

> I'd urge you to try and find a way to make it work without such
> a workaround.

I'm convinced this is not possible.  Filipp Gunbin didn't manage it when
he re-reported the problem some months ago.  I think it's likely other
people may have reported the problem in the years since 2009.

> >> Yes.  I suggest you submit a bug report for this bug.
> > I will put some time into this and see if I can come up with a patch
> > before flagging it as a bug.

> Once you get it to work without the above workaround, you'll be able to
> write a much better bug report.

> >> To work properly, beginning/end-of-defun need to know whether the
> >> starting point is inside a defun or between defuns.

> Calling beginning-of-defun-function followed by end-of-defun-function
> (and comparing the resulting position to the start position) should be
> sufficient to let you know whether or not you're inside the function
> whose definition starts most closely before point.

It's not.  With starting point between two defuns, that sequence could
leave point at any of before, at, or after starting point.  That's
entirely disregarding nested functions, and how to handle them.

CC Mode's C-M-e works.  João's new functions look like they will work.
Both of these analyse, as a distinct operation, the starting position to
determine whether or not it is inside or outside a defun.  Why not fix
the standard end-of-defun (and beginning-of-defun) in lisp.el so that it
will likewise work?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-05 21:29       ` Alan Mackenzie
@ 2022-12-05 21:56         ` Stefan Monnier
  0 siblings, 0 replies; 35+ messages in thread
From: Stefan Monnier @ 2022-12-05 21:56 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: João Paulo Labegalini de Carvalho, emacs-devel

> Even the existence of the variable end-of-defun-moves-to-eol is so crass
> it can't be other than broken.  João's patch contains a
> beginning-of-defun-function and and end-of-defun-function which
> intrinsically work.  Emacs's end-of-defun messes this up with it's
> ridiculous call to beginning-of-defun-raw, followed by a later call to
> (beginning-of-defun-raw -1).

Maybe I misunderstood.  I thought your claim of brokenness was relating to
`beginning/end-of-defun-function` rather than to `beginning/end-of-defun`.

>> Calling beginning-of-defun-function followed by end-of-defun-function
>> (and comparing the resulting position to the start position) should be
>> sufficient to let you know whether or not you're inside the function
>> whose definition starts most closely before point.
>
> It's not.  With starting point between two defuns, that sequence could
> leave point at any of before, at, or after starting point.

Exactly, and this tells you if you're after the "immediately preceding"
function or inside of it.

> That's entirely disregarding nested functions, and how to handle them.

If you need to handle nested functions, then after the begin+end which
told you you're after the previous function, you need to go back to the
beginning of the previous function (i.e. where you were after the call
to BODF) and call BODF again (and EODF again to check whether you
started from within that outer function or not, and maybe iterate if
needed).

> CC Mode's C-M-e works.  João's new functions look like they will work.
> Both of these analyse, as a distinct operation, the starting position to
> determine whether or not it is inside or outside a defun.  Why not fix
> the standard end-of-defun (and beginning-of-defun) in lisp.el so that it
> will likewise work?

As soon as we can find an API that works *both* for the case of
tree-sitter (i.e. where we have more or less direct access to a global
information telling us where we are relative to the surrounding code
constructs) as well as for the case where we don't have much more info
than what BODF/EODF work with (i.e. a regexp to match a defun-start and
a function to jump over a defun).


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-05 20:12     ` Stefan Monnier
  2022-12-05 21:29       ` Alan Mackenzie
@ 2022-12-06 15:51       ` João Paulo Labegalini de Carvalho
  2022-12-06 16:48         ` Stefan Monnier
  1 sibling, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-06 15:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Alan Mackenzie, emacs-devel

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

>
> > I think for now I will use the same workaround.
>
> I'd urge you to try and find a way to make it work without such
> a workaround.
>

I definitely will. I am just using the work around for testing.

>> Yes.  I suggest you submit a bug report for this bug.
> > I will put some time into this and see if I can come up with a patch
> > before flagging it as a bug.
>
> Once you get it to work without the above workaround, you'll be able to
> write a much better bug report.
>

Yes, and hopefully I can come up with a patch to improve the behavior as
well.


> >> To work properly, beginning/end-of-defun need to know whether the
> >> starting point is inside a defun or between defuns.
>
> Calling beginning-of-defun-function followed by end-of-defun-function
> (and comparing the resulting position to the start position) should be
> sufficient to let you know whether or not you're inside the function
> whose definition starts most closely before point.
>

Hmm. In sh-mode `beginning-of-defun-function' is nil and in the example
below, calling `end-of-defun-function' with M-: (funcall
end-of-defun-function) brings point to fi and not the end of the function.

function foo  {
  if [[ $1 > 0 ]]; then
    return 24
  else
    return 42
  echo "Hello from foo"
}

In the example above, C-M-a and C-M-e do the right thing. However, in the
presence of nested functions, C-M-a and C-M-e only navigate over top-level
functions.  For example:

function foo  {
  function bar {
    echo "Hello from bar"
  }
  if [[ $1 > 0 ]]; then
    return 24
  else
    return 42
  echo "Hello from foo"
}

C-M-a/e brings point to beginning/end of foo even when point was inside bar
and M-: (funcall end-of-defun-function) brings point to after o in echo.

If I define beginning/end-of-defun-function with the functions I wrote, the
behavior I intended happens (bringing point to bar in the example above if
point was inside bar).

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 3866 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 15:51       ` João Paulo Labegalini de Carvalho
@ 2022-12-06 16:48         ` Stefan Monnier
  2022-12-06 21:04           ` Yuan Fu
  0 siblings, 1 reply; 35+ messages in thread
From: Stefan Monnier @ 2022-12-06 16:48 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho; +Cc: Alan Mackenzie, emacs-devel

>> Calling beginning-of-defun-function followed by end-of-defun-function
>> (and comparing the resulting position to the start position) should be
>> sufficient to let you know whether or not you're inside the function
>> whose definition starts most closely before point.
>>
>
> Hmm. In sh-mode `beginning-of-defun-function' is nil and in the example
> below, calling `end-of-defun-function' with M-: (funcall
> end-of-defun-function) brings point to fi and not the end of the function.

Many major modes do not implement those two functions in a fully
reliable way, indeed.

`bash-ts-mode` should be able to implement them reliably, OTOH.

> In the example above, C-M-a and C-M-e do the right thing. However, in the
> presence of nested functions, C-M-a and C-M-e only navigate over top-level
> functions.  For example:

Yes, it's a common limitation when the major mode is unable to do
proper parsing of the code.


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 16:48         ` Stefan Monnier
@ 2022-12-06 21:04           ` Yuan Fu
  2022-12-06 21:08             ` Yuan Fu
  0 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-06 21:04 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: João Paulo Labegalini de Carvalho, Alan Mackenzie,
	emacs-devel



> On Dec 6, 2022, at 8:48 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
>>> Calling beginning-of-defun-function followed by end-of-defun-function
>>> (and comparing the resulting position to the start position) should be
>>> sufficient to let you know whether or not you're inside the function
>>> whose definition starts most closely before point.
>>> 
>> 
>> Hmm. In sh-mode `beginning-of-defun-function' is nil and in the example
>> below, calling `end-of-defun-function' with M-: (funcall
>> end-of-defun-function) brings point to fi and not the end of the function.
> 
> Many major modes do not implement those two functions in a fully
> reliable way, indeed.
> 
> `bash-ts-mode` should be able to implement them reliably, OTOH.
> 
>> In the example above, C-M-a and C-M-e do the right thing. However, in the
>> presence of nested functions, C-M-a and C-M-e only navigate over top-level
>> functions.  For example:
> 
> Yes, it's a common limitation when the major mode is unable to do
> proper parsing of the code.

It seems there are not convention on whether defun movements should move across top-level defun’s or both top-level and nested ones. I’ve seen bug report on python-ts-mode complaining about both sides.

Should we make it configurable, then? A variable that makes tree-sitter defun navigation switch between two modes: top-level only and not top-level only. 

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:04           ` Yuan Fu
@ 2022-12-06 21:08             ` Yuan Fu
  2022-12-06 21:40               ` Alan Mackenzie
  0 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-06 21:08 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: João Paulo Labegalini de Carvalho, Alan Mackenzie,
	emacs-devel



> On Dec 6, 2022, at 1:04 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Dec 6, 2022, at 8:48 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> 
>>>> Calling beginning-of-defun-function followed by end-of-defun-function
>>>> (and comparing the resulting position to the start position) should be
>>>> sufficient to let you know whether or not you're inside the function
>>>> whose definition starts most closely before point.
>>>> 
>>> 
>>> Hmm. In sh-mode `beginning-of-defun-function' is nil and in the example
>>> below, calling `end-of-defun-function' with M-: (funcall
>>> end-of-defun-function) brings point to fi and not the end of the function.
>> 
>> Many major modes do not implement those two functions in a fully
>> reliable way, indeed.
>> 
>> `bash-ts-mode` should be able to implement them reliably, OTOH.
>> 
>>> In the example above, C-M-a and C-M-e do the right thing. However, in the
>>> presence of nested functions, C-M-a and C-M-e only navigate over top-level
>>> functions.  For example:
>> 
>> Yes, it's a common limitation when the major mode is unable to do
>> proper parsing of the code.
> 
> It seems there are not convention on whether defun movements should move across top-level defun’s or both top-level and nested ones. I’ve seen bug report on python-ts-mode complaining about both sides.
> 
> Should we make it configurable, then? A variable that makes tree-sitter defun navigation switch between two modes: top-level only and not top-level only. 

And for functions nested in a class: if you type C-M-e at the beginning of a class, should it go to the end of the first function in that class, or should it go to the end of the class? Right now because of how end-of-defun works, it will jump to the end of the class if point is at the beginning of the class (1), and jump to the first function if point is before the beginning of the class (2).

(2)
(1)class class1():
    prop = 0
    def method1(self):
        pass

    def method2(self):
        pass


Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:08             ` Yuan Fu
@ 2022-12-06 21:40               ` Alan Mackenzie
  2022-12-06 21:46                 ` João Paulo Labegalini de Carvalho
  2022-12-07  0:41                 ` Code navigation for sh-mode with Tree-sitter Yuan Fu
  0 siblings, 2 replies; 35+ messages in thread
From: Alan Mackenzie @ 2022-12-06 21:40 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, João Paulo Labegalini de Carvalho,
	emacs-devel

Hello, Yuan.

On Tue, Dec 06, 2022 at 13:08:19 -0800, Yuan Fu wrote:


> > On Dec 6, 2022, at 1:04 PM, Yuan Fu <casouri@gmail.com> wrote:



> >> On Dec 6, 2022, at 8:48 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> >>>> Calling beginning-of-defun-function followed by
> >>>> end-of-defun-function (and comparing the resulting position to the
> >>>> start position) should be sufficient to let you know whether or
> >>>> not you're inside the function whose definition starts most
> >>>> closely before point.


> >>> Hmm. In sh-mode `beginning-of-defun-function' is nil and in the
> >>> example below, calling `end-of-defun-function' with M-: (funcall
> >>> end-of-defun-function) brings point to fi and not the end of the
> >>> function.

> >> Many major modes do not implement those two functions in a fully
> >> reliable way, indeed.

> >> `bash-ts-mode` should be able to implement them reliably, OTOH.

> >>> In the example above, C-M-a and C-M-e do the right thing. However,
> >>> in the presence of nested functions, C-M-a and C-M-e only navigate
> >>> over top-level functions.  For example:

> >> Yes, it's a common limitation when the major mode is unable to do
> >> proper parsing of the code.

> > It seems there are not convention on whether defun movements should
> > move across top-level defun’s or both top-level and nested ones. I’ve
> > seen bug report on python-ts-mode complaining about both sides.

> > Should we make it configurable, then? A variable that makes
> > tree-sitter defun navigation switch between two modes: top-level only
> > and not top-level only. 

In CC Mode, it has been configurable via the user option c-defun-tactic
for somewhere between ten and fifteen years.  When c-defun-tactic is t,
C-M-a/e go to the start/end of the top level defuns.  When it is the
symbol go-outward, C-M-a/e move to the next start/end of defun, if any,
at the current level of class/namespace nesting, and move outwards to
the next level of class/namespace nesting when a class/namespace boundary
is reached.

I don't remember any complaints about this mechanism.

> And for functions nested in a class: if you type C-M-e at the beginning
> of a class, should it go to the end of the first function in that
> class, or should it go to the end of the class? Right now because of
> how end-of-defun works, it will jump to the end of the class if point
> is at the beginning of the class (1), and jump to the first function if
> point is before the beginning of the class (2).

This doesn't seem sensible.

> (2)
> (1)class class1():
>     prop = 0
>     def method1(self):
>         pass

>     def method2(self):
>         pass



> Yuan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:40               ` Alan Mackenzie
@ 2022-12-06 21:46                 ` João Paulo Labegalini de Carvalho
  2022-12-06 21:55                   ` João Paulo Labegalini de Carvalho
  2022-12-07  0:41                 ` Code navigation for sh-mode with Tree-sitter Yuan Fu
  1 sibling, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-06 21:46 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Yuan Fu, Stefan Monnier, emacs-devel

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

>
> > > Should we make it configurable, then? A variable that makes
> > > tree-sitter defun navigation switch between two modes: top-level only
> > > and not top-level only.
>
> In CC Mode, it has been configurable via the user option c-defun-tactic
> for somewhere between ten and fifteen years.  When c-defun-tactic is t,
> C-M-a/e go to the start/end of the top level defuns.  When it is the
> symbol go-outward, C-M-a/e move to the next start/end of defun, if any,
> at the current level of class/namespace nesting, and move outwards to
> the next level of class/namespace nesting when a class/namespace boundary
> is reached.
>

The 'go-outward behavior is the one I have implemented.

I don't remember any complaints about this mechanism.
>
> > And for functions nested in a class: if you type C-M-e at the beginning
> > of a class, should it go to the end of the first function in that
> > class, or should it go to the end of the class? Right now because of
> > how end-of-defun works, it will jump to the end of the class if point
> > is at the beginning of the class (1), and jump to the first function if
> > point is before the beginning of the class (2).
>
> This doesn't seem sensible.
>

I tend to agree. I would expect it to go to the nearest node (either class
or function) and not bypass a class node in favor of function nodes.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 2351 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:46                 ` João Paulo Labegalini de Carvalho
@ 2022-12-06 21:55                   ` João Paulo Labegalini de Carvalho
  2022-12-06 22:35                     ` Stefan Monnier
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-06 21:55 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Yuan Fu, Stefan Monnier, emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 2762 bytes --]

I completed my patch to add basic navigation to bash-ts-mode.

It works consistently when the new navigation
functions (`sh-mode--treesit-beginning-of-defun' and
`sh-mode--treesit-end-of-defun') are bound directly to C-M-a/e. However, if
those functions are bound to `beginning-of-defun-function' and
`end-of-defun-function', C-M-e does not work with negative arguments inside
of functions.

In the example below C-u - 1 C-M-e goes to the beginning of foo and not to
end of bar when point is inside foo, and anywhere after bar:

function foo {
    function bar {
        echo "Hello from bar"
    }
    if [[ $1 > 0 ]]; then
        return 42
    else
        return 24
    fi
    echo bar
}

Any comments and feedback on the patch are welcomed.

On Tue, Dec 6, 2022 at 2:46 PM João Paulo Labegalini de Carvalho <
jaopaulolc@gmail.com> wrote:

> > > Should we make it configurable, then? A variable that makes
>> > > tree-sitter defun navigation switch between two modes: top-level only
>> > > and not top-level only.
>>
>> In CC Mode, it has been configurable via the user option c-defun-tactic
>> for somewhere between ten and fifteen years.  When c-defun-tactic is t,
>> C-M-a/e go to the start/end of the top level defuns.  When it is the
>> symbol go-outward, C-M-a/e move to the next start/end of defun, if any,
>> at the current level of class/namespace nesting, and move outwards to
>> the next level of class/namespace nesting when a class/namespace boundary
>> is reached.
>>
>
> The 'go-outward behavior is the one I have implemented.
>
> I don't remember any complaints about this mechanism.
>>
>> > And for functions nested in a class: if you type C-M-e at the beginning
>> > of a class, should it go to the end of the first function in that
>> > class, or should it go to the end of the class? Right now because of
>> > how end-of-defun works, it will jump to the end of the class if point
>> > is at the beginning of the class (1), and jump to the first function if
>> > point is before the beginning of the class (2).
>>
>> This doesn't seem sensible.
>>
>
> I tend to agree. I would expect it to go to the nearest node (either class
> or function) and not bypass a class node in favor of function nodes.
>
> --
> João Paulo L. de Carvalho
> Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
> Postdoctoral Research Fellow | University of Alberta | Edmonton, AB -
> Canada
> joao.carvalho@ic.unicamp.br
> joao.carvalho@ualberta.ca
>


-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #1.2: Type: text/html, Size: 4132 bytes --]

[-- Attachment #2: 0001-Basic-navigation-for-bash-ts-mode.patch --]
[-- Type: text/x-patch, Size: 8933 bytes --]

From e3691d9261233241d349dd4d39b72ddb144f6f1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20P=2E=20L=2E=20de=20Carvalho?=
 <jaopaulolc@gmail.com>
Date: Sat, 3 Dec 2022 12:55:27 -0700
Subject: [PATCH] Basic navigation for bash-ts-mode

Enables navigation to beginning/end of function in bash-ts-mode.
  * lisp/progmodes/sh-script.el (bash-ts-mode): enable navegation via
  new internal navigation functions.
  (sh-mode--treesit-beginning-of-defun)
  (sh-mode--treesit-end-of-defun)
  (sh-mode--treesit-defun-p)
  (sh-mode--treesit-not-sc-p)
  (sh-mode--treesit-next-sibling-defun)
  (sh-mode--treesit-prev-sibling-defun): New functions.
  (sh-mode--treesit-parent-defun): New macro.
---
 lisp/progmodes/sh-script.el | 171 +++++++++++++++++++++++++++++++++++-
 1 file changed, 170 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 408ebfc045..751e5d6993 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,7 +1619,12 @@ bash-ts-mode
                   ( bracket delimiter misc-punctuation operator)))
     (setq-local treesit-font-lock-settings
                 sh-mode--treesit-settings)
-    (treesit-major-mode-setup)))
+    (setq-local treesit-defun-type-regexp "function_definition")
+    (treesit-major-mode-setup)
+    ;; (keymap-set bash-ts-mode-map "C-M-a" #'sh-mode--treesit-beginning-of-defun)
+    ;; (keymap-set bash-ts-mode-map "C-M-e" #'sh-mode--treesit-end-of-defun)
+    (setq-local beginning-of-defun-function #'sh-mode--treesit-beginning-of-defun)
+    (setq-local end-of-defun-function #'sh-mode--treesit-end-of-defun)))
 
 (advice-add 'bash-ts-mode :around #'sh--redirect-bash-ts-mode
             ;; Give it lower precedence than normal advice, so other
@@ -3364,5 +3369,169 @@ sh-mode--treesit-settings
    '((["$"]) @font-lock-misc-punctuation-face))
   "Tree-sitter font-lock settings for `sh-mode'.")
 
+\f
+;;; 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)
+  "Returns t if NODE is *not* a compound-statement
+and nil otherwise."
+  (lambda (p)
+    (not (string-match "compound_statement"
+                       (treesit-node-type p)))))
+
+(defun sh-mode--treesit-next-sibling-defun (node)
+  "Returns the next sibling function of NODE, if any, or nil."
+  (let ((sibling node))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-next-sibling sibling)))
+    sibling))
+
+(defun sh-mode--treesit-prev-sibling-defun (node)
+  "Returns the previous sibling function of NODE, if any, or nil."
+  (let ((sibling (treesit-node-prev-sibling node)))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-prev-sibling sibling)))
+    sibling))
+
+(defmacro sh-mode--treesit-parent-defun (node)
+  "Returns 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)
+  "Returns 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))
+
+(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 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
+beggining of closest sibling function, recursively."
+  (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))
+              (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 (&optional arg)
+  "Tree-sitter `end-of-defun' function.
+
+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, recursively."
+  (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)))))
+
 (provide 'sh-script)
 ;;; sh-script.el ends here
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:55                   ` João Paulo Labegalini de Carvalho
@ 2022-12-06 22:35                     ` Stefan Monnier
  2022-12-06 22:41                       ` João Paulo Labegalini de Carvalho
  2022-12-06 22:57                       ` Stefan Monnier
  0 siblings, 2 replies; 35+ messages in thread
From: Stefan Monnier @ 2022-12-06 22:35 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Alan Mackenzie, Yuan Fu, emacs-devel

> However, if those functions are bound to `beginning-of-defun-function'
> and `end-of-defun-function',

Could you clarify?
`beginning-of-defun-function' is a variable holding a function.
Keys can be bound neither to that variable nor to the function it holds
(because it's not a command).
And functions usually aren't "bound" either.


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 22:35                     ` Stefan Monnier
@ 2022-12-06 22:41                       ` João Paulo Labegalini de Carvalho
  2022-12-06 22:57                       ` Stefan Monnier
  1 sibling, 0 replies; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-06 22:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Alan Mackenzie, Yuan Fu, emacs-devel

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

On Tue, Dec 6, 2022 at 3:36 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > However, if those functions are bound to `beginning-of-defun-function'
> > and `end-of-defun-function',
>
> Could you clarify?
> `beginning-of-defun-function' is a variable holding a function.
> Keys can be bound neither to that variable nor to the function it holds
> (because it's not a command).
> And functions usually aren't "bound" either.
>

 Please ignore my wrong use of terminology. This is what I mean:

(keymap-set bash-ts-mode-map "C-M-a" #'sh-mode--treesit-beginning-of-defun)
(keymap-set bash-ts-mode-map "C-M-e" #'sh-mode--treesit-end-of-defun)

If the above code is evaluated as part of bash-ts-mode startup,
invoking C-M-a/e does as intended by the new functions.

However, if the variables `beginning-of-defun-function' &
`end-of-defun-function' are set as in:

(setq-local beginning-of-defun-function
#'sh-mode--treesit-beginning-of-defun)
(setq-local end-of-defun-function #'sh-mode--treesit-end-of-defun)

C-M-a works as intended, but C-M-e with negative arguments does not.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 2022 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 22:35                     ` Stefan Monnier
  2022-12-06 22:41                       ` João Paulo Labegalini de Carvalho
@ 2022-12-06 22:57                       ` Stefan Monnier
  2022-12-06 23:43                         ` João Paulo Labegalini de Carvalho
  1 sibling, 1 reply; 35+ messages in thread
From: Stefan Monnier @ 2022-12-06 22:57 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Alan Mackenzie, Yuan Fu, emacs-devel

>> However, if those functions are bound to `beginning-of-defun-function'
>> and `end-of-defun-function',
>
> Could you clarify?
> `beginning-of-defun-function' is a variable holding a function.
> Keys can be bound neither to that variable nor to the function it holds
> (because it's not a command).
> And functions usually aren't "bound" either.

Ah, I think I get it.  You mean your set the `[EB]OD-function` variables
to your two functions/commands, right?

To figure out whether the problem is inside `end-of-defun` or in the
way `[EB]OD-function` are expected to behave, you'll have to single-step
through `end-of-defun`, I think.


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 22:57                       ` Stefan Monnier
@ 2022-12-06 23:43                         ` João Paulo Labegalini de Carvalho
  2022-12-06 23:50                           ` Stefan Monnier
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-06 23:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Alan Mackenzie, Yuan Fu, emacs-devel

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

On Tue, Dec 6, 2022 at 3:57 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> Ah, I think I get it.  You mean your set the `[EB]OD-function` variables
> to your two functions/commands, right?
>

Exactly.


> To figure out whether the problem is inside `end-of-defun` or in the
> way `[EB]OD-function` are expected to behave, you'll have to single-step
> through `end-of-defun`, I think.
>

It seems that the problem originates from the function which `EOD-function'
is set to be called after via the `BOD-raw'. With a positive argument all
is good, since the navigation functions for bash-ts-mode have symmetric
behavior -- both bring point to beginning/end of the closest function that
encloses point.

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.

A second look revealed that the function set to `end-of-defun-function' is
called without arguments, thus when used can only produce motions that are
symmetric to `beginning-of-defun-function'.

Since that is not the case for the functions I am proposing for
bash-ts-mode, I believe that binding them as commands to C-M-a/e is the
only option. Unless I am missing something.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 2720 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 23:43                         ` João Paulo Labegalini de Carvalho
@ 2022-12-06 23:50                           ` Stefan Monnier
  2022-12-07  1:12                             ` João Paulo Labegalini de Carvalho
  0 siblings, 1 reply; 35+ messages in thread
From: Stefan Monnier @ 2022-12-06 23:50 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Alan Mackenzie, Yuan Fu, emacs-devel

> 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`.

> A second look revealed that the function set to `end-of-defun-function' is
> called without arguments,

As documented in its docstring, yes.

> thus when used can only produce motions that are symmetric to
> `beginning-of-defun-function'.

Not sure what you mean by that.


        Stefan




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 21:40               ` Alan Mackenzie
  2022-12-06 21:46                 ` João Paulo Labegalini de Carvalho
@ 2022-12-07  0:41                 ` Yuan Fu
  1 sibling, 0 replies; 35+ messages in thread
From: Yuan Fu @ 2022-12-07  0:41 UTC (permalink / raw)
  To: Alan Mackenzie
  Cc: Stefan Monnier, João Paulo Labegalini de Carvalho,
	emacs-devel

> 
>>> Should we make it configurable, then? A variable that makes
>>> tree-sitter defun navigation switch between two modes: top-level only
>>> and not top-level only. 
> 
> In CC Mode, it has been configurable via the user option c-defun-tactic
> for somewhere between ten and fifteen years.  When c-defun-tactic is t,
> C-M-a/e go to the start/end of the top level defuns.  When it is the
> symbol go-outward, C-M-a/e move to the next start/end of defun, if any,
> at the current level of class/namespace nesting, and move outwards to
> the next level of class/namespace nesting when a class/namespace boundary
> is reached.
> 
> I don't remember any complaints about this mechanism.

Ah, very good. I’ll try to make tree-sitter to do the same.

> 
>> And for functions nested in a class: if you type C-M-e at the beginning
>> of a class, should it go to the end of the first function in that
>> class, or should it go to the end of the class? Right now because of
>> how end-of-defun works, it will jump to the end of the class if point
>> is at the beginning of the class (1), and jump to the first function if
>> point is before the beginning of the class (2).
> 
> This doesn't seem sensible.

I agree.

>> (2)
>> (1)class class1():
>>    prop = 0
>>    def method1(self):
>>        pass
> 
>>    def method2(self):
>>        pass

How would cc-mode’s C-M-e move in position (2) under the go-outward tactic? Would it go to the end of method1, or the end of class1? From your description it seems that it will move to the end of class1. Is there anyway to configure it to move to the end of method1 in this case?

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-06 23:50                           ` Stefan Monnier
@ 2022-12-07  1:12                             ` João Paulo Labegalini de Carvalho
  2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-07  1:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Alan Mackenzie, Yuan Fu, emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1212 bytes --]

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.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #1.2: Type: text/html, Size: 1901 bytes --]

[-- Attachment #2: 0001-Basic-navigation-for-bash-ts-mode.patch --]
[-- Type: text/x-patch, Size: 9461 bytes --]

From e5430b13fe7f3945864e194c9207ed1a5e02835f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20P=2E=20L=2E=20de=20Carvalho?=
 <jaopaulolc@gmail.com>
Date: Sat, 3 Dec 2022 12:55:27 -0700
Subject: [PATCH] Basic navigation for bash-ts-mode

Enables navigation to beginning/end of function in bash-ts-mode.
  * lisp/progmodes/sh-script.el (bash-ts-mode): enable navegation via
  `beginning-of-defun-function' & `end-of-defun-function'.
  (sh-mode-treesit-beginning-of-defun)
  (sh-mode-treesit-end-of-defun)
  (sh-mode--treesit-end-of-defun-function)
  (sh-mode--treesit-defun-p)
  (sh-mode--treesit-not-sc-p)
  (sh-mode--treesit-next-sibling-defun)
  (sh-mode--treesit-prev-sibling-defun): New functions.
  (sh-mode--treesit-parent-defun): New macro.
---
 lisp/progmodes/sh-script.el | 184 +++++++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 408ebfc045..04193ca0f8 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,7 +1619,10 @@ bash-ts-mode
                   ( bracket delimiter misc-punctuation operator)))
     (setq-local treesit-font-lock-settings
                 sh-mode--treesit-settings)
-    (treesit-major-mode-setup)))
+    (setq-local treesit-defun-type-regexp "function_definition")
+    (treesit-major-mode-setup)
+    (setq-local beginning-of-defun-function #'sh-mode-treesit-beginning-of-defun)
+    (setq-local end-of-defun-function #'sh-mode--treesit-end-of-defun-function)))
 
 (advice-add 'bash-ts-mode :around #'sh--redirect-bash-ts-mode
             ;; Give it lower precedence than normal advice, so other
@@ -3364,5 +3367,184 @@ sh-mode--treesit-settings
    '((["$"]) @font-lock-misc-punctuation-face))
   "Tree-sitter font-lock settings for `sh-mode'.")
 
+\f
+;;; 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)
+  "Returns t if NODE is *not* a compound-statement
+and nil otherwise."
+  (lambda (p)
+    (not (string-match "compound_statement"
+                       (treesit-node-type p)))))
+
+(defun sh-mode--treesit-next-sibling-defun (node)
+  "Returns the next sibling function of NODE, if any, or nil."
+  (let ((sibling node))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-next-sibling sibling)))
+    sibling))
+
+(defun sh-mode--treesit-prev-sibling-defun (node)
+  "Returns the previous sibling function of NODE, if any, or nil."
+  (let ((sibling (treesit-node-prev-sibling node)))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-prev-sibling sibling)))
+    sibling))
+
+(defmacro sh-mode--treesit-parent-defun (node)
+  "Returns 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)
+  "Returns 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))
+
+(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
+beggining of closest sibling function, recursively."
+  (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))
+              (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 recurses to 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)))))
+
 (provide 'sh-script)
 ;;; sh-script.el ends here
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-07  1:12                             ` João Paulo Labegalini de Carvalho
@ 2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
  2022-12-10  4:58                                 ` Yuan Fu
                                                   ` (2 more replies)
  0 siblings, 3 replies; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-07 17:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Alan Mackenzie, Yuan Fu, emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1704 bytes --]

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.
>
> --
> João Paulo L. de Carvalho
> Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
> Postdoctoral Research Fellow | University of Alberta | Edmonton, AB -
> Canada
> joao.carvalho@ic.unicamp.br
> joao.carvalho@ualberta.ca
>


-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #1.2: Type: text/html, Size: 2921 bytes --]

[-- Attachment #2: 0001-Basic-navigation-for-bash-ts-mode.patch --]
[-- Type: text/x-patch, Size: 9540 bytes --]

From 0b893135655da22b944d090e8ccab31476eaa9a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20P=2E=20L=2E=20de=20Carvalho?=
 <jaopaulolc@gmail.com>
Date: Sat, 3 Dec 2022 12:55:27 -0700
Subject: [PATCH] Basic navigation for bash-ts-mode

Enables navigation to beginning/end of function in bash-ts-mode.
  * lisp/progmodes/sh-script.el (bash-ts-mode): enable navegation via
  `beginning-of-defun-function' & `end-of-defun-function'.
  (sh-mode-treesit-beginning-of-defun)
  (sh-mode-treesit-end-of-defun)
  (sh-mode--treesit-end-of-defun-function)
  (sh-mode--treesit-defun-p)
  (sh-mode--treesit-not-sc-p)
  (sh-mode--treesit-next-sibling-defun)
  (sh-mode--treesit-prev-sibling-defun): New functions.
  (sh-mode--treesit-parent-defun): New macro.
---
 lisp/progmodes/sh-script.el | 184 +++++++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/sh-script.el b/lisp/progmodes/sh-script.el
index 408ebfc045..90212e2aba 100644
--- a/lisp/progmodes/sh-script.el
+++ b/lisp/progmodes/sh-script.el
@@ -1619,7 +1619,10 @@ bash-ts-mode
                   ( bracket delimiter misc-punctuation operator)))
     (setq-local treesit-font-lock-settings
                 sh-mode--treesit-settings)
-    (treesit-major-mode-setup)))
+    (setq-local treesit-defun-type-regexp "function_definition")
+    (treesit-major-mode-setup)
+    (setq-local beginning-of-defun-function #'sh-mode-treesit-beginning-of-defun)
+    (setq-local end-of-defun-function #'sh-mode--treesit-end-of-defun-function)))
 
 (advice-add 'bash-ts-mode :around #'sh--redirect-bash-ts-mode
             ;; Give it lower precedence than normal advice, so other
@@ -3364,5 +3367,184 @@ sh-mode--treesit-settings
    '((["$"]) @font-lock-misc-punctuation-face))
   "Tree-sitter font-lock settings for `sh-mode'.")
 
+\f
+;;; 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)))))
+
+(defun sh-mode--treesit-next-sibling-defun (node)
+  "Return the next sibling function of NODE, if any, or nil."
+  (let ((sibling node))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-next-sibling sibling)))
+    sibling))
+
+(defun sh-mode--treesit-prev-sibling-defun (node)
+  "Return the previous sibling function of NODE, if any, or nil."
+  (let ((sibling (treesit-node-prev-sibling node)))
+    (while (and sibling
+                (not (sh-mode--treesit-defun-p sibling)))
+      (setq sibling (treesit-node-prev-sibling sibling)))
+    sibling))
+
+(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))
+
+(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))
+              (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)))))
+
 (provide 'sh-script)
 ;;; sh-script.el ends here
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
@ 2022-12-10  4:58                                 ` Yuan Fu
  2022-12-13  4:55                                 ` Yuan Fu
  2022-12-13  5:20                                 ` New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter) Yuan Fu
  2 siblings, 0 replies; 35+ messages in thread
From: Yuan Fu @ 2022-12-10  4:58 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel



> 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.

Just want you to know I didn’t forget about this :-) I’ll soon review the patch, thanks!

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
  2022-12-10  4:58                                 ` Yuan Fu
@ 2022-12-13  4:55                                 ` Yuan Fu
  2022-12-13 16:00                                   ` João Paulo Labegalini de Carvalho
  2022-12-13  5:20                                 ` New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter) Yuan Fu
  2 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-13  4:55 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

[-- 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

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
  2022-12-10  4:58                                 ` Yuan Fu
  2022-12-13  4:55                                 ` Yuan Fu
@ 2022-12-13  5:20                                 ` Yuan Fu
  2022-12-13 16:11                                   ` João Paulo Labegalini de Carvalho
  2 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-13  5:20 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

Thanks to Alan and João, 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’t 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’t think the current beginning/end-of-defun can support nested navigation without large changes. So I’d prefer we use new beg/end-of-defun commands in tree-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 can 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 = 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 = 0[9]
[108]
[109]class Class3():
[999]    prop = 0[10]
[110]

Then we have a list of positions like this, basically saying “if point is at marker X, and we find the next/previous beginning/end of defun, point should end up at Y”, and we have X for all the possible positions in a nested defun, and Y for all four operations. So the first line says “if we start at marker 0, and find prev-beg-of-defun, we should end up at marker 103”.

;; 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.
    )





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: Code navigation for sh-mode with Tree-sitter
  2022-12-13  4:55                                 ` Yuan Fu
@ 2022-12-13 16:00                                   ` João Paulo Labegalini de Carvalho
  0 siblings, 0 replies; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-13 16:00 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

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

>
> 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.
>

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 general
> 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.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 4478 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13  5:20                                 ` New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter) Yuan Fu
@ 2022-12-13 16:11                                   ` João Paulo Labegalini de Carvalho
  2022-12-13 16:38                                     ` Eli Zaretskii
  2022-12-13 18:07                                     ` Yuan Fu
  0 siblings, 2 replies; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-13 16:11 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

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

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 <casouri@gmail.com> wrote:

> Thanks to Alan and João, 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’t 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’t think the
> current beginning/end-of-defun can support nested navigation without large
> changes. So I’d prefer we use new beg/end-of-defun commands in tree-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 can
> 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 = 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 = 0[9]
> [108]
> [109]class Class3():
> [999]    prop = 0[10]
> [110]
>
> Then we have a list of positions like this, basically saying “if point is
> at marker X, and we find the next/previous beginning/end of defun, point
> should end up at Y”, and we have X for all the possible positions in a
> nested defun, and Y for all four operations. So the first line says “if we
> start at marker 0, and find prev-beg-of-defun, we should end up at marker
> 103”.
>
> ;; 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.
>     )
>
>
>

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 4989 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 16:11                                   ` João Paulo Labegalini de Carvalho
@ 2022-12-13 16:38                                     ` Eli Zaretskii
  2022-12-13 18:03                                       ` João Paulo Labegalini de Carvalho
  2022-12-13 18:07                                     ` Yuan Fu
  1 sibling, 1 reply; 35+ messages in thread
From: Eli Zaretskii @ 2022-12-13 16:38 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho; +Cc: casouri, monnier, acm, emacs-devel

> From: João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com>
> Date: Tue, 13 Dec 2022 09:11:41 -0700
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Alan Mackenzie <acm@muc.de>,
>  emacs-devel@gnu.org
> 
> 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.

I think the command that goes to the firs leaf backwards should be
bound to C-M-u, not C-M-a.  C-M-u takes a numeric argument to specify
how many levels up to move, so the user can control to which leaf top
go.  Or maybe we should have an entirely new key binding for these
fine-tuned movements.



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 16:38                                     ` Eli Zaretskii
@ 2022-12-13 18:03                                       ` João Paulo Labegalini de Carvalho
  0 siblings, 0 replies; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-13 18:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: casouri, monnier, acm, emacs-devel

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

On Tue, Dec 13, 2022 at 9:38 AM Eli Zaretskii <eliz@gnu.org> wrote:

> I think the command that goes to the firs leaf backwards should be
> bound to C-M-u, not C-M-a.


I think C-M-u is doing what I would expect, although it is currently moving
point to "{" instead of the beginning of the function definition (in .

But I like the idea of using C-M-u/C-M-d to move outwards and inwards. And
maybe we can use C-M-a/C-M-e for horizontal tree movements across sibling
functions, or upwards when the current level does not have sibling
functions.


> Or maybe we should have an entirely new key binding for these
> fine-tuned movements.
>

That might be needed as C-M-u/C-M-d moves across a class of "list-defining"
characters (e.g. "{" , "(", and "[").


-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 1809 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 16:11                                   ` João Paulo Labegalini de Carvalho
  2022-12-13 16:38                                     ` Eli Zaretskii
@ 2022-12-13 18:07                                     ` Yuan Fu
  2022-12-13 18:48                                       ` João Paulo Labegalini de Carvalho
  1 sibling, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-13 18:07 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel



> On Dec 13, 2022, at 8:11 AM, João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> 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.

Treesit--navigate-defun should, as you did in bash-ts-mode, only navigate in the same level (siblings) or higher (parents). Could you show an example of the unexpected behavior?

For a function like the following and point at (1), moving back to beg-of-defun goes to (2), then (3), and never goes to (X).

(3)def method():
    (2)def method1():
        (X)def method11():
            return 0
        return 0
    (1)def method2():
        def method22():
            return 0
        return 0

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 18:07                                     ` Yuan Fu
@ 2022-12-13 18:48                                       ` João Paulo Labegalini de Carvalho
  2022-12-13 18:56                                         ` Yuan Fu
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-13 18:48 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

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

On Tue, Dec 13, 2022 at 11:07 AM Yuan Fu <casouri@gmail.com> wrote:

> Treesit--navigate-defun should, as you did in bash-ts-mode, only navigate
> in the same level (siblings) or higher (parents). Could you show an example
> of the unexpected behavior?


I must have forgotten to evaluate something on my setup file. It is indeed
working as you described. Sorry for the noise.

Should I add key bindings to use the new tree-sitter navigation functions
to bash-ts-mode or is it planned an integration with beginning/end-of-defun?

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 1354 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 18:48                                       ` João Paulo Labegalini de Carvalho
@ 2022-12-13 18:56                                         ` Yuan Fu
  2022-12-13 19:46                                           ` João Paulo Labegalini de Carvalho
  0 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-13 18:56 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel



> On Dec 13, 2022, at 10:48 AM, João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> 
> On Tue, Dec 13, 2022 at 11:07 AM Yuan Fu <casouri@gmail.com> wrote:
> Treesit--navigate-defun should, as you did in bash-ts-mode, only navigate in the same level (siblings) or higher (parents). Could you show an example of the unexpected behavior?
> 
> I must have forgotten to evaluate something on my setup file. It is indeed working as you described. Sorry for the noise.
> 
> Should I add key bindings to use the new tree-sitter navigation functions to bash-ts-mode or is it planned an integration with beginning/end-of-defun?

I will repurpose two functions, treesit-beginning-of-defun/end, and make treesit-major-mode-setup set them up automatically. I’m thinking about remapping beginning/end-of-defun to their tree-sitter counterparts in (current-local-map), in treesit-major-mode-setup. That way major modes don’t need to explicitly bind tree-sitter functions in their keymaps. Stefan, would that be a good idea?

So if everything goes as planned, bash-ts-mode only needs to set treesit-defun-type-regexp.

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 18:56                                         ` Yuan Fu
@ 2022-12-13 19:46                                           ` João Paulo Labegalini de Carvalho
  2022-12-16  1:49                                             ` Yuan Fu
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-13 19:46 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

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

On Tue, Dec 13, 2022 at 11:56 AM Yuan Fu <casouri@gmail.com> wrote:

> I will repurpose two functions, treesit-beginning-of-defun/end, and make
> treesit-major-mode-setup set them up automatically. I’m thinking about
> remapping beginning/end-of-defun to their tree-sitter counterparts in
> (current-local-map), in treesit-major-mode-setup. That way major modes
> don’t need to explicitly bind tree-sitter functions in their keymaps.
>

That would be great. A very clean solution IMO.

-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 1339 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-13 19:46                                           ` João Paulo Labegalini de Carvalho
@ 2022-12-16  1:49                                             ` Yuan Fu
  2022-12-16 16:24                                               ` João Paulo Labegalini de Carvalho
  0 siblings, 1 reply; 35+ messages in thread
From: Yuan Fu @ 2022-12-16  1:49 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel



> On Dec 13, 2022, at 11:46 AM, João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> 
> 
> On Tue, Dec 13, 2022 at 11:56 AM Yuan Fu <casouri@gmail.com> wrote:
> I will repurpose two functions, treesit-beginning-of-defun/end, and make treesit-major-mode-setup set them up automatically. I’m thinking about remapping beginning/end-of-defun to their tree-sitter counterparts in (current-local-map), in treesit-major-mode-setup. That way major modes don’t need to explicitly bind tree-sitter functions in their keymaps. 
> 
> That would be great. A very clean solution IMO. 

No one seem to object, so I made the change on emacs-29. Now tree-sitter major modes should all use the new navigation commands.

Yuan


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-16  1:49                                             ` Yuan Fu
@ 2022-12-16 16:24                                               ` João Paulo Labegalini de Carvalho
  2022-12-17 23:32                                                 ` Yuan Fu
  0 siblings, 1 reply; 35+ messages in thread
From: João Paulo Labegalini de Carvalho @ 2022-12-16 16:24 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Alan Mackenzie, emacs-devel

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

Thanks Yuan,

I pulled your changes and navigation now works out of the box.

On Thu, Dec 15, 2022 at 6:49 PM Yuan Fu <casouri@gmail.com> wrote:

>
>
> > On Dec 13, 2022, at 11:46 AM, João Paulo Labegalini de Carvalho <
> jaopaulolc@gmail.com> wrote:
> >
> >
> >
> > On Tue, Dec 13, 2022 at 11:56 AM Yuan Fu <casouri@gmail.com> wrote:
> > I will repurpose two functions, treesit-beginning-of-defun/end, and make
> treesit-major-mode-setup set them up automatically. I’m thinking about
> remapping beginning/end-of-defun to their tree-sitter counterparts in
> (current-local-map), in treesit-major-mode-setup. That way major modes
> don’t need to explicitly bind tree-sitter functions in their keymaps.
> >
> > That would be great. A very clean solution IMO.
>
> No one seem to object, so I made the change on emacs-29. Now tree-sitter
> major modes should all use the new navigation commands.
>
> Yuan



-- 
João Paulo L. de Carvalho
Ph.D Computer Science |  IC-UNICAMP | Campinas , SP - Brazil
Postdoctoral Research Fellow | University of Alberta | Edmonton, AB - Canada
joao.carvalho@ic.unicamp.br
joao.carvalho@ualberta.ca

[-- Attachment #2: Type: text/html, Size: 1913 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter)
  2022-12-16 16:24                                               ` João Paulo Labegalini de Carvalho
@ 2022-12-17 23:32                                                 ` Yuan Fu
  0 siblings, 0 replies; 35+ messages in thread
From: Yuan Fu @ 2022-12-17 23:32 UTC (permalink / raw)
  To: João Paulo Labegalini de Carvalho
  Cc: Stefan Monnier, Alan Mackenzie, emacs-devel



> On Dec 16, 2022, at 8:24 AM, João Paulo Labegalini de Carvalho <jaopaulolc@gmail.com> wrote:
> 
> Thanks Yuan,
> 
> I pulled your changes and navigation now works out of the box.

Great!




^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2022-12-17 23:32 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-03 20:23 Code navigation for sh-mode with Tree-sitter João Paulo Labegalini de Carvalho
2022-12-03 21:46 ` Alan Mackenzie
2022-12-05 15:24   ` João Paulo Labegalini de Carvalho
2022-12-05 20:12     ` Stefan Monnier
2022-12-05 21:29       ` Alan Mackenzie
2022-12-05 21:56         ` Stefan Monnier
2022-12-06 15:51       ` João Paulo Labegalini de Carvalho
2022-12-06 16:48         ` Stefan Monnier
2022-12-06 21:04           ` Yuan Fu
2022-12-06 21:08             ` Yuan Fu
2022-12-06 21:40               ` Alan Mackenzie
2022-12-06 21:46                 ` João Paulo Labegalini de Carvalho
2022-12-06 21:55                   ` João Paulo Labegalini de Carvalho
2022-12-06 22:35                     ` Stefan Monnier
2022-12-06 22:41                       ` João Paulo Labegalini de Carvalho
2022-12-06 22:57                       ` Stefan Monnier
2022-12-06 23:43                         ` João Paulo Labegalini de Carvalho
2022-12-06 23:50                           ` Stefan Monnier
2022-12-07  1:12                             ` João Paulo Labegalini de Carvalho
2022-12-07 17:20                               ` João Paulo Labegalini de Carvalho
2022-12-10  4:58                                 ` Yuan Fu
2022-12-13  4:55                                 ` Yuan Fu
2022-12-13 16:00                                   ` João Paulo Labegalini de Carvalho
2022-12-13  5:20                                 ` New defun navigation for tree-sitter (Was: Code navigation for sh-mode with Tree-sitter) Yuan Fu
2022-12-13 16:11                                   ` João Paulo Labegalini de Carvalho
2022-12-13 16:38                                     ` Eli Zaretskii
2022-12-13 18:03                                       ` João Paulo Labegalini de Carvalho
2022-12-13 18:07                                     ` Yuan Fu
2022-12-13 18:48                                       ` João Paulo Labegalini de Carvalho
2022-12-13 18:56                                         ` Yuan Fu
2022-12-13 19:46                                           ` João Paulo Labegalini de Carvalho
2022-12-16  1:49                                             ` Yuan Fu
2022-12-16 16:24                                               ` João Paulo Labegalini de Carvalho
2022-12-17 23:32                                                 ` Yuan Fu
2022-12-07  0:41                 ` Code navigation for sh-mode with Tree-sitter Yuan Fu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).