unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Plug treesit.el into other emacs constructs
@ 2022-12-12 14:33 Theodor Thornhill
  2022-12-12 14:45 ` Eli Zaretskii
  2022-12-12 15:46 ` Stefan Monnier
  0 siblings, 2 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-12 14:33 UTC (permalink / raw)
  To: emacs-devel; +Cc: eliz, casouri

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


Hello!

I've started plugging tree-sitter support into other constructs in
emacs, and I would love some feedback and suggestions for how to
proceed.  Emacs has some nice utilities we could piggyback on, namely:

- forward-sexp
- forward-sentence
- forward-*

What they provide is suddenly access to all of the transpose-*
functions.  I'll show with some examples, where | denotes point, and if
there are two | it denotes active region, like this: |String foo|.

The patch supplied below enables the following:

* Navigation:
** Forward-sentence:
Executing M-e repeatedly will go from:
```
public void foo() {
  |int x = 5;
  int x = 6;
}
```
to
```
public void foo() {
  int x = 5;|
  int x = 6;
}
```
then
```
public void foo() {
  int x = 5;
  int x = 6;|
}
```
** transpose-sentence:
Executing M-x transpose-sentence repeatedly will go from:
```
public void foo() {
  int x = 5;|
  if (r) {
  
  }
  int x = 6;
}
```
to
```
public void foo() {
  if (r) {
  
  }
  int x = 5;|
  int x = 6;
}
```
then
```
public void foo() {
  if (r) {
  
  }
  int x = 6;
  int x = 5;|
}
```
Further transpose-sentence will not go out of the method because there
are no siblings

** kill-sentence:
Executing M-k repeatedly will go from:
```
public void foo() {
  |int x = 6;
  if (foo) {
            
  }
}
```
to
```
public void foo() {
  |
  if (foo) {
            
  }
}
```
then
```
public void foo() {
    |
}
```

** Forward-sexp:
Executing C-M-f repeatedly will go from:
```
public void foo(|String bar, String baz) {}
```
to
```
public void foo(String bar|, String baz) {}
```
and
```
public void foo(String bar, String baz|) {}
```
** Mark-sexp:
Executing C-M-@ repeatedly will go from:
```
public void foo(|String bar, String baz) {}
```
to
```
public void foo(|String bar|, String baz) {}
```
then
```
public void foo(|String bar, String baz|) {}
```

** transpose-sexp:
Executing C-M-t repeatedly will go from:
```
public void foo(int bar,| String baz) {}
```
to
```
public void foo(String baz, int bar|) {}
```

** kill-sexp:
Executing C-M-k repeatedly will go from:
```
public void foo(|int bar, String baz) {}
```
to
```
public void foo(|, int bar) {}
```
then
```
public void foo() {}
```

And more.


The patch only adds a few things:

- (defvar-local forward-sentence-function nil), the analog to
  forward-sexp-function and beginning-of-defun-function
- treesit-sexp-type-regexp and treesit-sentence-type-regexp, both to be
  added to treesit-major-mode-setup, and added to each *-ts-mode.

Is the general idea ok?  Could we add these or similar changes to
paragraphs.el and just piggyback on it inside of treesit?

If we want this I can start working on preparing a patch for this on the
master branch.

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-forward-implementation.patch --]
[-- Type: text/x-diff, Size: 7670 bytes --]

From e67e26f6d4dc39436b169b4fc026376d9f74b6b9 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Mon, 12 Dec 2022 15:28:05 +0100
Subject: [PATCH] WIP forward-* implementation

---
 lisp/progmodes/java-ts-mode.el | 10 +++++
 lisp/textmodes/paragraphs.el   | 73 +++++++++++++++++++---------------
 lisp/treesit.el                | 50 ++++++++++++++++++++++-
 3 files changed, 100 insertions(+), 33 deletions(-)

diff --git a/lisp/progmodes/java-ts-mode.el b/lisp/progmodes/java-ts-mode.el
index d5f4f55fe0..e76380ec2a 100644
--- a/lisp/progmodes/java-ts-mode.el
+++ b/lisp/progmodes/java-ts-mode.el
@@ -336,6 +336,16 @@ java-ts-mode
                             "package_declaration"
                             "module_declaration")))
 
+  (setq-local treesit-sentence-type-regexp
+              (regexp-opt '("declaration"
+                            "statement")))
+
+  (setq-local treesit-sexp-type-regexp
+              (regexp-opt '("declaration"
+                            "statement"
+                            "formal_parameter"
+                            "string_literal")))
+
   ;; Font-lock.
   (setq-local treesit-font-lock-settings java-ts-mode--font-lock-settings)
   (setq-local treesit-font-lock-feature-list
diff --git a/lisp/textmodes/paragraphs.el b/lisp/textmodes/paragraphs.el
index c500dc014f..b53fe66eee 100644
--- a/lisp/textmodes/paragraphs.el
+++ b/lisp/textmodes/paragraphs.el
@@ -441,6 +441,8 @@ end-of-paragraph-text
 	  (if (< (point) (point-max))
 	      (end-of-paragraph-text))))))
 
+(defvar-local forward-sentence-function nil)
+
 (defun forward-sentence (&optional arg)
   "Move forward to next end of sentence.  With argument, repeat.
 When ARG is negative, move backward repeatedly to start of sentence.
@@ -449,36 +451,40 @@ forward-sentence
 sentences.  Also, every paragraph boundary terminates sentences as well."
   (interactive "^p")
   (or arg (setq arg 1))
-  (let ((opoint (point))
-        (sentence-end (sentence-end)))
-    (while (< arg 0)
-      (let ((pos (point))
-	    par-beg par-text-beg)
-	(save-excursion
-	  (start-of-paragraph-text)
-	  ;; Start of real text in the paragraph.
-	  ;; We move back to here if we don't see a sentence-end.
-	  (setq par-text-beg (point))
-	  ;; Start of the first line of the paragraph.
-	  ;; We use this as the search limit
-	  ;; to allow sentence-end to match if it is anchored at
-	  ;; BOL and the paragraph starts indented.
-	  (beginning-of-line)
-	  (setq par-beg (point)))
-	(if (and (re-search-backward sentence-end par-beg t)
-		 (or (< (match-end 0) pos)
-		     (re-search-backward sentence-end par-beg t)))
-	    (goto-char (match-end 0))
-	  (goto-char par-text-beg)))
-      (setq arg (1+ arg)))
-    (while (> arg 0)
-      (let ((par-end (save-excursion (end-of-paragraph-text) (point))))
-	(if (re-search-forward sentence-end par-end t)
-	    (skip-chars-backward " \t\n")
-	  (goto-char par-end)))
-      (setq arg (1- arg)))
-    (let ((npoint (constrain-to-field nil opoint t)))
-      (not (= npoint opoint)))))
+  (cond
+   (forward-sentence-function
+    (funcall forward-sentence-function arg))
+   (t
+    (let ((opoint (point))
+          (sentence-end (sentence-end)))
+      (while (< arg 0)
+        (let ((pos (point))
+	      par-beg par-text-beg)
+	  (save-excursion
+	    (start-of-paragraph-text)
+	    ;; Start of real text in the paragraph.
+	    ;; We move back to here if we don't see a sentence-end.
+	    (setq par-text-beg (point))
+	    ;; Start of the first line of the paragraph.
+	    ;; We use this as the search limit
+	    ;; to allow sentence-end to match if it is anchored at
+	    ;; BOL and the paragraph starts indented.
+	    (beginning-of-line)
+	    (setq par-beg (point)))
+	  (if (and (re-search-backward sentence-end par-beg t)
+		   (or (< (match-end 0) pos)
+		       (re-search-backward sentence-end par-beg t)))
+	      (goto-char (match-end 0))
+	    (goto-char par-text-beg)))
+        (setq arg (1+ arg)))
+      (while (> arg 0)
+        (let ((par-end (save-excursion (end-of-paragraph-text) (point))))
+	  (if (re-search-forward sentence-end par-end t)
+	      (skip-chars-backward " \t\n")
+	    (goto-char par-end)))
+        (setq arg (1- arg)))
+      (let ((npoint (constrain-to-field nil opoint t)))
+        (not (= npoint opoint)))))))
 
 (defun count-sentences (start end)
   "Count sentences in current buffer from START to END."
@@ -532,14 +538,17 @@ repunctuate-sentences
         (remove-function isearch-filter-predicate
                          repunctuate-sentences-filter)))))
 
-
 (defun backward-sentence (&optional arg)
   "Move backward to start of sentence.
 With ARG, do it ARG times.  See `forward-sentence' for more
 information."
   (interactive "^p")
   (or arg (setq arg 1))
-  (forward-sentence (- arg)))
+  (cond
+   (forward-sentence-function
+    (funcall forward-sentence-function (- arg)))
+   (t
+    (forward-sentence (- arg)))))
 
 (defun kill-sentence (&optional arg)
   "Kill from point to end of sentence.
diff --git a/lisp/treesit.el b/lisp/treesit.el
index 133564f6c8..4a1c10211e 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1603,6 +1603,50 @@ treesit--defun-maybe-top-level
                  node)
       finally return node))))
 
+(defvar-local treesit-sexp-type-regexp "\\`field_declaration\\'"
+  "A regexp that matches the node type of sexp nodes.")
+
+(defun treesit-forward-sexp (&optional arg)
+  "Tree-sitter `beginning-of-defun' function.
+ARG is the same as in `beginning-of-defun'."
+  (let ((arg (or arg 1))
+        (node (treesit-node-at (point))))
+    (while (and (> arg 0)
+                (setq node (treesit-search-forward-goto
+                            node
+                            treesit-sexp-type-regexp)))
+      (setq arg (1- arg))
+      (goto-char (treesit-node-end node)))
+    (while (and (< arg 0)
+                (setq node (treesit-search-forward-goto
+                            node
+                            treesit-sexp-type-regexp t t)))
+      (setq arg (1+ arg))
+      (goto-char (treesit-node-start node)))
+    t))
+
+(defvar-local treesit-sentence-type-regexp "\\`field_declaration\\'"
+  "A regexp that matches the node type of textual nodes.")
+
+(defun treesit-forward-sentence (&optional arg)
+  "Tree-sitter `beginning-of-defun' function.
+ARG is the same as in `beginning-of-defun'."
+  (let ((arg (or arg 1))
+        (node (treesit-node-at (point))))
+    (while (and (> arg 0)
+                (setq node (treesit-search-forward-goto
+                            node
+                            treesit-sentence-type-regexp)))
+      (setq arg (1- arg))
+      (goto-char (treesit-node-end node)))
+    (while (and (< arg 0)
+                (setq node (treesit-search-forward-goto
+                            node
+                            treesit-sentence-type-regexp t t)))
+      (setq arg (1+ arg))
+      (goto-char (treesit-node-start node)))
+    t))
+
 (defun treesit-beginning-of-defun (&optional arg)
   "Tree-sitter `beginning-of-defun' function.
 ARG is the same as in `beginning-of-defun'."
@@ -1730,7 +1774,11 @@ treesit-major-mode-setup
   ;; Navigation.
   (when treesit-defun-type-regexp
     (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
-    (setq-local end-of-defun-function #'treesit-end-of-defun)))
+    (setq-local end-of-defun-function #'treesit-end-of-defun))
+  (when treesit-sentence-type-regexp
+    (setq-local forward-sentence-function #'treesit-forward-sentence))
+  (when treesit-sexp-type-regexp
+    (setq-local forward-sexp-function #'treesit-forward-sexp)))
 
 ;;; Debugging
 
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-12 14:33 Plug treesit.el into other emacs constructs Theodor Thornhill
@ 2022-12-12 14:45 ` Eli Zaretskii
  2022-12-13 18:17   ` Theodor Thornhill
  2022-12-12 15:46 ` Stefan Monnier
  1 sibling, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2022-12-12 14:45 UTC (permalink / raw)
  To: Theodor Thornhill, Stefan Monnier; +Cc: emacs-devel, casouri

> From: Theodor Thornhill <theo@thornhill.no>
> Cc: eliz@gnu.org, casouri@gmail.com
> Date: Mon, 12 Dec 2022 15:33:33 +0100
> 
> The patch only adds a few things:
> 
> - (defvar-local forward-sentence-function nil), the analog to
>   forward-sexp-function and beginning-of-defun-function
> - treesit-sexp-type-regexp and treesit-sentence-type-regexp, both to be
>   added to treesit-major-mode-setup, and added to each *-ts-mode.
> 
> Is the general idea ok?

Yes, because you can see that CC Mode supports these key bindings as
well, with its own commands.

> Could we add these or similar changes to
> paragraphs.el and just piggyback on it inside of treesit?

I'm not sure we want to go this far in the release branch, but it
would be nice to have these in C mode, for example (or any other
tree-sitter supported mode whose "regular" sibling has similar
commands).  The generalizations should IMO be left for master.

> If we want this I can start working on preparing a patch for this on the
> master branch.

I think so, but let's hear from others, like Stefan (CC'ed).



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-12 14:33 Plug treesit.el into other emacs constructs Theodor Thornhill
  2022-12-12 14:45 ` Eli Zaretskii
@ 2022-12-12 15:46 ` Stefan Monnier
  2022-12-13 18:27   ` Theodor Thornhill
  2022-12-14 23:32   ` Stephen Leake
  1 sibling, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-12 15:46 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: emacs-devel, eliz, casouri

Cool, thanks, a few comments (based on my experience with adding
similar things based on `smie`):

> ** Forward-sexp:
> Executing C-M-f repeatedly will go from:
> ```
> public void foo(|String bar, String baz) {}
> ```
> to
> ```
> public void foo(String bar|, String baz) {}
> ```

That looks wrong.  `String` is a valid AST node.  Whether it gets a node
in tree-sitter or not, I don't know, but here there are several "sexps"
that start at point and I think `forward-sexp` should be conservative
and keep advancing by the smallest option.

There can be many more than 2 choices, of course, e.g.:

    x = |f (x) * 3 + 2;

Here "f" is the smallest sexp after point, "f (x)" is the next one up,
then "f (x) * 3" and finally "f (x) * 3 + 2".

> ```
> public void foo(String bar, String baz|) {}
> ```

That one's right :-)

> ** transpose-sexp:
> Executing C-M-t repeatedly will go from:
> ```
> public void foo(int bar,| String baz) {}
> ```
> to
> ```
> public void foo(String baz, int bar|) {}
> ```

And this one it right as well (regardless if | starts after or before the comma).

Does it work as well for infix keywords that are made of normal letters,
like say `else` (or the `and` and `or` used in some languages instead
of `&&` and `||`)?


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-12 14:45 ` Eli Zaretskii
@ 2022-12-13 18:17   ` Theodor Thornhill
  0 siblings, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-13 18:17 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Monnier; +Cc: emacs-devel, casouri

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Theodor Thornhill <theo@thornhill.no>
>> Cc: eliz@gnu.org, casouri@gmail.com
>> Date: Mon, 12 Dec 2022 15:33:33 +0100
>> 
>> The patch only adds a few things:
>> 
>> - (defvar-local forward-sentence-function nil), the analog to
>>   forward-sexp-function and beginning-of-defun-function
>> - treesit-sexp-type-regexp and treesit-sentence-type-regexp, both to be
>>   added to treesit-major-mode-setup, and added to each *-ts-mode.
>> 
>> Is the general idea ok?
>
> Yes, because you can see that CC Mode supports these key bindings as
> well, with its own commands.
>

>> Could we add these or similar changes to
>> paragraphs.el and just piggyback on it inside of treesit?
>
> I'm not sure we want to go this far in the release branch, but it
> would be nice to have these in C mode, for example (or any other
> tree-sitter supported mode whose "regular" sibling has similar
> commands).  The generalizations should IMO be left for master.
>
>> If we want this I can start working on preparing a patch for this on the
>> master branch.
>
> I think so, but let's hear from others, like Stefan (CC'ed).

Great, thanks :)



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-12 15:46 ` Stefan Monnier
@ 2022-12-13 18:27   ` Theodor Thornhill
  2022-12-13 19:37     ` Stefan Monnier
  2022-12-14 23:32   ` Stephen Leake
  1 sibling, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-13 18:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, eliz, casouri

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Cool, thanks, a few comments (based on my experience with adding
> similar things based on `smie`):
>
>> ** Forward-sexp:
>> Executing C-M-f repeatedly will go from:
>> ```
>> public void foo(|String bar, String baz) {}
>> ```
>> to
>> ```
>> public void foo(String bar|, String baz) {}
>> ```
>
> That looks wrong.  `String` is a valid AST node.  Whether it gets a node
> in tree-sitter or not, I don't know, but here there are several "sexps"
> that start at point and I think `forward-sexp` should be conservative
> and keep advancing by the smallest option.

I understand.  My reasoning is that 'forward-word' is suitable for that,
and to actually gain something from these we need to use a little bigger
constructs.  In tree-sitter 'String' isn't really valid, because you
need the identifier to create a complete node.

>
> There can be many more than 2 choices, of course, e.g.:
>
>     x = |f (x) * 3 + 2;
>
> Here "f" is the smallest sexp after point, "f (x)" is the next one up,
> then "f (x) * 3" and finally "f (x) * 3 + 2".
>

In this case I'd think that forward-sexp would do:
```
x = |f (x) * 3 + 2;
x = f (x)| * 3 + 2;
x = f (x) * 3| + 2;
x = f (x) * 3 + 2;|
```
Or something like that.  So that multiple transpose-sexps would move
'f(x)' over the operators, swapping with the integers.


>> ```
>> public void foo(String bar, String baz|) {}
>> ```
>
> That one's right :-)
>

Why is this one right, and the above not?

>> ** transpose-sexp:
>> Executing C-M-t repeatedly will go from:
>> ```
>> public void foo(int bar,| String baz) {}
>> ```
>> to
>> ```
>> public void foo(String baz, int bar|) {}
>> ```
>
> And this one it right as well (regardless if | starts after or before the comma).
>
> Does it work as well for infix keywords that are made of normal letters,
> like say `else` (or the `and` and `or` used in some languages instead
> of `&&` and `||`)?

I see no reason it shouldn't but I need to investigate that a bit
further.  I'm still trying to understand how all the forward-* functions
work, to see whether I need to modify my functions.

Thanks for the feedback so far.  I interpret this that this feature is
wanted, so I'll make a more serious effort and get back to you.

BTW, where are the semantics for these movement functions defined?  I
mean, what construct is each one expected to jump over?

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 18:27   ` Theodor Thornhill
@ 2022-12-13 19:37     ` Stefan Monnier
  2022-12-13 19:53       ` Yuan Fu
  2022-12-13 20:02       ` Theodor Thornhill
  0 siblings, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-13 19:37 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: emacs-devel, eliz, casouri

>>> ** Forward-sexp:
>>> Executing C-M-f repeatedly will go from:
>>> ```
>>> public void foo(|String bar, String baz) {}
>>> ```
>>> to
>>> ```
>>> public void foo(String bar|, String baz) {}
>>> ```
>>
>> That looks wrong.  `String` is a valid AST node.  Whether it gets a node
>> in tree-sitter or not, I don't know, but here there are several "sexps"
>> that start at point and I think `forward-sexp` should be conservative
>> and keep advancing by the smallest option.
> I understand.  My reasoning is that 'forward-word' is suitable for that,

It's not, tho, because it stops within identifiers like "foo_bar".
There's a similar question for things like `String.match`.

> and to actually gain something from these we need to use a little bigger
> constructs.  In tree-sitter 'String' isn't really valid, because you
> need the identifier to create a complete node.

I think we should not define the "ideal" behavior based on what
Tree-sitter provides.
As I said, in the *A*ST, `String` is a valid node.
It's especially true if you consider more complex types like

    public void foo(Array<Foo<List<Int>, String>> bar, String baz)

> In this case I'd think that forward-sexp would do:
> ```
> x = |f (x) * 3 + 2;
> x = f (x)| * 3 + 2;
> x = f (x) * 3| + 2;
> x = f (x) * 3 + 2;|
> ```
> Or something like that.

Similarly here I think it should first stop after `f`.
The other ones look right to me.

> So that multiple transpose-sexps would move
> 'f(x)' over the operators, swapping with the integers.

You could still do that, but you'd have to start with point next to `*`
to specify the node whose children you want to swap.

>>> ```
>>> public void foo(String bar, String baz|) {}
>>> ```
>>
>> That one's right :-)
>
> Why is this one right, and the above not?

Because point was left of the comma and the smallest right child of the
corresponding node is "String bar" and not "String" (which is more like
the left child of the node that covers "String bar").

> Thanks for the feedback so far.  I interpret this that this feature is
> wanted, so I'll make a more serious effort and get back to you.

Yes, definitely.  It's one of the best features of SMIE compared to
"hand-written" indentation code, if you ask me :-)
Tree-sitter should be able to do it even better.

> BTW, where are the semantics for these movement functions defined?

In our heads.

> I mean, what construct is each one expected to jump over?

In my book "sexp" movement should jump over subtrees of the AST.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 19:37     ` Stefan Monnier
@ 2022-12-13 19:53       ` Yuan Fu
  2022-12-13 20:06         ` Perry Smith
  2022-12-13 23:19         ` Stefan Monnier
  2022-12-13 20:02       ` Theodor Thornhill
  1 sibling, 2 replies; 54+ messages in thread
From: Yuan Fu @ 2022-12-13 19:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, emacs-devel, eliz

> 
>> I mean, what construct is each one expected to jump over?
> 
> In my book "sexp" movement should jump over subtrees of the AST.

It’s pretty hard to judge which subtree to move over at a given point in an AST. For example, when point is at | in the following text:

(|X.y(z), alpha)

Should point move over X, or X.y, or X.y(z)? All three subtrees has their beg=(point). A human can tell (and might disagree on) which unit to move across, but a program couldn’t tell. Without language specific knowledge, it can’t really decide. 

Just a thought, but maybe we can let major modes define what’s an “abstract list”, and sexp-forward would move across the immediate children of abstract lists. Eg, abstract lists in C would contain block, argument list, statement, etc. And in the example above forward-sexp would move across X.y(z) because it’s an immediate children of the enclosing abstract list, the argument list.

Yuan


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 19:37     ` Stefan Monnier
  2022-12-13 19:53       ` Yuan Fu
@ 2022-12-13 20:02       ` Theodor Thornhill
  2022-12-13 23:10         ` Stefan Monnier
  1 sibling, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-13 20:02 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, eliz, casouri

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> ** Forward-sexp:
>>>> Executing C-M-f repeatedly will go from:
>>>> ```
>>>> public void foo(|String bar, String baz) {}
>>>> ```
>>>> to
>>>> ```
>>>> public void foo(String bar|, String baz) {}
>>>> ```
>>>
>>> That looks wrong.  `String` is a valid AST node.  Whether it gets a node
>>> in tree-sitter or not, I don't know, but here there are several "sexps"
>>> that start at point and I think `forward-sexp` should be conservative
>>> and keep advancing by the smallest option.
>> I understand.  My reasoning is that 'forward-word' is suitable for that,
>
> It's not, tho, because it stops within identifiers like "foo_bar".
> There's a similar question for things like `String.match`.
>

Ok, I think I understand.  But 'void foo(String bar)' is not the same as
'String.format', where 'void foo(bar String)' would make to sense, but
'format.String' could.

>> and to actually gain something from these we need to use a little bigger
>> constructs.  In tree-sitter 'String' isn't really valid, because you
>> need the identifier to create a complete node.
>
> I think we should not define the "ideal" behavior based on what
> Tree-sitter provides.
> As I said, in the *A*ST, `String` is a valid node.
> It's especially true if you consider more complex types like
>
>     public void foo(Array<Foo<List<Int>, String>> bar, String baz)
>

And in this case multiple forward-sexps would be

```
|public void foo(Array<Foo<List<Int>, String>> bar, String baz)
public| void foo(Array<Foo<List<Int>, String>> bar, String baz)
public void| foo|(Array<Foo<List<Int>, String>> bar, String baz)
public void foo(Array|<Foo<List<Int>, String>> bar, String baz)
public void foo(Array<Foo|<List<Int>, String>> bar, String baz)
public void foo(Array<Foo<List|<Int>, String>> bar, String baz)
public void foo(Array<Foo<List<Int|>, String>> bar, String baz)
public void foo(Array<Foo<List<Int>, String|>> bar, String baz)
public void foo(Array<Foo<List<Int>, String>> bar|, String baz)
public void foo(Array<Foo<List<Int>, String>> bar, String| baz)
public void foo(Array<Foo<List<Int>, String>> bar, String baz|)
```

and transpose-sexp would be:

```
public void foo(Array<Foo<List<Int>, String>> bar,| String baz)
public void foo(String baz, Array<Foo<List<Int>, String>> bar|)
```

Not really sure how to accomodate the two behaviors in the same
function, but I'll get there.

>> In this case I'd think that forward-sexp would do:
>> ```
>> x = |f (x) * 3 + 2;
>> x = f (x)| * 3 + 2;
>> x = f (x) * 3| + 2;
>> x = f (x) * 3 + 2;|
>> ```
>> Or something like that.
>
> Similarly here I think it should first stop after `f`.
> The other ones look right to me.
>

Ok

>> So that multiple transpose-sexps would move
>> 'f(x)' over the operators, swapping with the integers.
>
> You could still do that, but you'd have to start with point next to `*`
> to specify the node whose children you want to swap.
>

Yeah.

>>>> ```
>>>> public void foo(String bar, String baz|) {}
>>>> ```
>>>
>>> That one's right :-)
>>
>> Why is this one right, and the above not?
>
> Because point was left of the comma and the smallest right child of the
> corresponding node is "String bar" and not "String" (which is more like
> the left child of the node that covers "String bar").
>

Ok, so you mean that forward-sexp should incrementally cover more and
more of a node, but transpose-sexp would find the _whole_ node, then
swap it with the one "in front" of it?

so in 'void(String foo, int bar)'

forward-sexp would go word by word, but transpose-sexp would capture
"String foo" and "int bar" when point is on the comma?

>> Thanks for the feedback so far.  I interpret this that this feature is
>> wanted, so I'll make a more serious effort and get back to you.
>
> Yes, definitely.  It's one of the best features of SMIE compared to
> "hand-written" indentation code, if you ask me :-)
> Tree-sitter should be able to do it even better.
>

Nice!

>> BTW, where are the semantics for these movement functions defined?
>
> In our heads.
>

Sweet - I might bother you more, then.

>> I mean, what construct is each one expected to jump over?
>
> In my book "sexp" movement should jump over subtrees of the AST.

So given this ast point should move over each named node, no matter if
transposing them would create broken code?

```
(class_declaration
 (modifiers public)
 class name: (identifier)
 body: 
  (class_body {
   (method_declaration
    dimensions: 
     (generic_type (type_identifier)
      (type_arguments <
       (generic_type (type_identifier)
        (type_arguments < (type_identifier) >))
       >))
    body: (identifier)
    (formal_parameters (
     (formal_parameter type: (type_identifier) dimensions: (identifier))
     ,
     (formal_parameter type: (type_identifier) dimensions: (identifier))
     ))
    (block { }))
   }))
```

Forgive my stupid questions, I just want it to be clear to me what I'm
doing here ;)

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 19:53       ` Yuan Fu
@ 2022-12-13 20:06         ` Perry Smith
  2022-12-13 23:19         ` Stefan Monnier
  1 sibling, 0 replies; 54+ messages in thread
From: Perry Smith @ 2022-12-13 20:06 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, Theodor Thornhill, emacs-devel, eliz

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



> On Dec 13, 2022, at 13:53, Yuan Fu <casouri@gmail.com> wrote:
> 
>> 
>>> I mean, what construct is each one expected to jump over?
>> 
>> In my book "sexp" movement should jump over subtrees of the AST.
> 
> It’s pretty hard to judge which subtree to move over at a given point in an AST. For example, when point is at | in the following text:
> 
> (|X.y(z), alpha)
> 
> Should point move over X, or X.y, or X.y(z)? All three subtrees has their beg=(point). A human can tell (and might disagree on) which unit to move across, but a program couldn’t tell. Without language specific knowledge, it can’t really decide.
> 
> Just a thought, but maybe we can let major modes define what’s an “abstract list”, and sexp-forward would move across the immediate children of abstract lists. Eg, abstract lists in C would contain block, argument list, statement, etc. And in the example above forward-sexp would move across X.y(z) because it’s an immediate children of the enclosing abstract list, the argument list.

How about this:

Most languages have a well known precedence for operators and usually these are documented with words like “additive operators”, “multiplicative operators”, etc.

The difference between X, X.y, and X.y(z) I would say are likely known to the user.  In other words, they are probably thinking “I want to move past the function call” or more reasonably, “I want to move to the next parameter”.

With some thought, a keymap could be devised so that <prefix>-p would move to the next parameter, <prefix>-m would move past the multiplicative subexpression.


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 20:02       ` Theodor Thornhill
@ 2022-12-13 23:10         ` Stefan Monnier
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-13 23:10 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: emacs-devel, eliz, casouri

> And in this case multiple forward-sexps would be
>
> ```
> |public void foo(Array<Foo<List<Int>, String>> bar, String baz)
> public| void foo(Array<Foo<List<Int>, String>> bar, String baz)
> public void| foo|(Array<Foo<List<Int>, String>> bar, String baz)
> public void foo(Array|<Foo<List<Int>, String>> bar, String baz)

No, from before the paren, it should skip to after the paren, like it
has done in the past.

> public void foo(Array<Foo|<List<Int>, String>> bar, String baz)
> public void foo(Array<Foo<List|<Int>, String>> bar, String baz)

Similarly here, I'd expect to jump over the whole `<List<Int>, String>`.

>> Because point was left of the comma and the smallest right child of the
>> corresponding node is "String bar" and not "String" (which is more like
>> the left child of the node that covers "String bar").
> Ok, so you mean that forward-sexp should incrementally cover more and
> more of a node, but transpose-sexp would find the _whole_ node, then
> swap it with the one "in front" of it?
>
> so in 'void(String foo, int bar)'
>
> forward-sexp would go word by word, but transpose-sexp would capture
> "String foo" and "int bar" when point is on the comma?

When point is left of a comma, it can't be claimed to be "just before
String foo" because there's a comma between the two, so `forward-sexp`
can skip over the whole right-hand-side of the comma:

    (y + 2,| x + 4)   ==forward-sexp==>  (y + 2, x| + 4)
    (y + 2|, x + 4)   ==forward-sexp==>  (y + 2, x + 4|)

Similarly when going backward, if we're to the *right* of a comma, we'd
want to jump over the whole left-hand side of the comma:

    (y + 2,| x + 4)   ==backward-sexp==>  (|y + 2, x + 4)
    (y + 2|, x + 4)   ==backward-sexp==>  (y + |2, x + 4|)

For `transpose-sexp` we want to transpose two nodes at the same level of
the tree, so regardless if we're to the left or to the right of a comma,
we want to swap the whole left/right hand sides:

    (y + 2,| x + 4)   ==backward-sexp==>  (x + 4, y + 2|)
    (y + 2|, x + 4)   ==backward-sexp==>  (x + 4, y + 2|)

For SMIE this happens "automatically" because `transpose-sexp` first
"steps back" before jumping over a sexp: it skips punctuation backward
before jumping with `forward-sexp` and skips punctuation forward before
jump with `backward-sexp`.

This works for infix operators that use punctuation syntax, but not for
infix operators like `else`.

>>> I mean, what construct is each one expected to jump over?
>> In my book "sexp" movement should jump over subtrees of the AST.
> So given this ast point should move over each named node, no matter if
> transposing them would create broken code?

Sorry, I don't understand what you mean by "move over each named node".

> Forgive my stupid questions, I just want it to be clear to me what I'm
> doing here ;)

The semantics I advocate aren't necessarily the "right" one.
It's the one I came up with when I tried to make sense of it for SMIE.
I think they make sense and I find it hard to imagine others that make
as much sense, but if I can't convince you, maybe there's something
better out there.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 19:53       ` Yuan Fu
  2022-12-13 20:06         ` Perry Smith
@ 2022-12-13 23:19         ` Stefan Monnier
  2022-12-14  8:14           ` Yuan Fu
  1 sibling, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-13 23:19 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Theodor Thornhill, emacs-devel, eliz

>>> I mean, what construct is each one expected to jump over?
>> In my book "sexp" movement should jump over subtrees of the AST.
> It’s pretty hard to judge which subtree to move over at a given point in an
> AST. For example, when point is at | in the following text:
>
> (|X.y(z), alpha)
>
> Should point move over X, or X.y, or X.y(z)? All three subtrees has their
> beg=(point).

Exactly.  It's even a bit worse: I'd also argue that an additional valid
choice is to move over the whole "X.y(z), alpha".

The semantics I opted for in SMIE is to choose the smallest/deepest
subtree.  That's what best matches the previous behavior of
`forward-sexp`.  If the users want to move over larger units they have
to place their point elsewhere (e.g. if it's just before ".", then
moving over "y" wouldn't make sense because "y" is attached to "y(z)"
and not to ".", only "y(z)" is attached to ".").

> Just a thought, but maybe we can let major modes define what’s an “abstract
> list”, and sexp-forward would move across the immediate children of abstract
> lists. Eg, abstract lists in C would contain block, argument list,
> statement, etc. And in the example above forward-sexp would move across
> X.y(z) because it’s an immediate children of the enclosing abstract list,
> the argument list.

Using the semantics I advocate, the user needs to place his point just
to the left of `;` in order for `forward-sexp` to jump over the next
instruction (or to the right of the `;` in order to jump over the
previous instruction with `backward-sexp`).


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-13 23:19         ` Stefan Monnier
@ 2022-12-14  8:14           ` Yuan Fu
  2022-12-14  8:42             ` Theodor Thornhill
  2022-12-14 14:01             ` Stefan Monnier
  0 siblings, 2 replies; 54+ messages in thread
From: Yuan Fu @ 2022-12-14  8:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, emacs-devel, eliz



> On Dec 13, 2022, at 3:19 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
>>>> I mean, what construct is each one expected to jump over?
>>> In my book "sexp" movement should jump over subtrees of the AST.
>> It’s pretty hard to judge which subtree to move over at a given point in an
>> AST. For example, when point is at | in the following text:
>> 
>> (|X.y(z), alpha)
>> 
>> Should point move over X, or X.y, or X.y(z)? All three subtrees has their
>> beg=(point).
> 
> Exactly.  It's even a bit worse: I'd also argue that an additional valid
> choice is to move over the whole "X.y(z), alpha".
> 
> The semantics I opted for in SMIE is to choose the smallest/deepest
> subtree.  That's what best matches the previous behavior of
> `forward-sexp`.  If the users want to move over larger units they have
> to place their point elsewhere (e.g. if it's just before ".", then
> moving over "y" wouldn't make sense because "y" is attached to "y(z)"
> and not to ".", only "y(z)" is attached to ".").

I would argue that the purpose of forward-sexp is to move over items in a list. Always going for the smallest subtree doesn’t seem to align with it. Take that example above, going across the smallest subtree means moving over X, then moving over “.”, that doesn’t feel like what forward-sexp should do to me. I think I'm misunderstanding what you mean.

> 
>> Just a thought, but maybe we can let major modes define what’s an “abstract
>> list”, and sexp-forward would move across the immediate children of abstract
>> lists. Eg, abstract lists in C would contain block, argument list,
>> statement, etc. And in the example above forward-sexp would move across
>> X.y(z) because it’s an immediate children of the enclosing abstract list,
>> the argument list.
> 
> Using the semantics I advocate, the user needs to place his point just
> to the left of `;` in order for `forward-sexp` to jump over the next
> instruction (or to the right of the `;` in order to jump over the
> previous instruction with `backward-sexp`).

You mean in the following code

int a = 0[1];
int b = 1;[2]

Forward-sexp would move [1] to [2]? But if we move over the smallest subtree, I’d imagine it only move across the semicolon after [1]. Even if it moves from [1] to [2], needing to adjust point feels very inconvenient to me, at least I wouldn’t want to use something like that. I want to type a single binding and move to where I want, and type that binding multiple times to move multiple steps. Both doesn’t seem to be possible with what you described.

Yuan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14  8:14           ` Yuan Fu
@ 2022-12-14  8:42             ` Theodor Thornhill
  2022-12-14 14:01             ` Stefan Monnier
  1 sibling, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-14  8:42 UTC (permalink / raw)
  To: Yuan Fu, Stefan Monnier; +Cc: emacs-devel, eliz

Yuan Fu <casouri@gmail.com> writes:

>> On Dec 13, 2022, at 3:19 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

[...]

>> 
>>> Just a thought, but maybe we can let major modes define what’s an “abstract
>>> list”, and sexp-forward would move across the immediate children of abstract
>>> lists. Eg, abstract lists in C would contain block, argument list,
>>> statement, etc. And in the example above forward-sexp would move across
>>> X.y(z) because it’s an immediate children of the enclosing abstract list,
>>> the argument list.
>> 
>> Using the semantics I advocate, the user needs to place his point just
>> to the left of `;` in order for `forward-sexp` to jump over the next
>> instruction (or to the right of the `;` in order to jump over the
>> previous instruction with `backward-sexp`).
>
> You mean in the following code
>
> int a = 0[1];
> int b = 1;[2]
>
> Forward-sexp would move [1] to [2]? But if we move over the smallest
> subtree, I’d imagine it only move across the semicolon after [1]. Even
> if it moves from [1] to [2], needing to adjust point feels very
> inconvenient to me, at least I wouldn’t want to use something like
> that. I want to type a single binding and move to where I want, and
> type that binding multiple times to move multiple steps. Both doesn’t
> seem to be possible with what you described.
>

Yeah.  My intuition for forward-sexp was always "some construct bigger
than a 'word'".  But in tree-sitter we also get the opportunity to make
code stay valid.  For example:

void foo(String bar, int baz) | {}

In this case it wouldn't make sense for transpose-sexps to do the
following:

void foo {} | (String bar, int baz)

Because that wouldn't be valid java.  But swapping the params inside
would make sense, but only when the whole node is pulled over.

And it point is directly between the paren opener and 'String',
forward-sexp would make sense to jump to the comma, because forward-word
would do the smaller movement.

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14  8:14           ` Yuan Fu
  2022-12-14  8:42             ` Theodor Thornhill
@ 2022-12-14 14:01             ` Stefan Monnier
  2022-12-14 16:24               ` Theodor Thornhill
  2022-12-14 23:31               ` Yuan Fu
  1 sibling, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 14:01 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Theodor Thornhill, emacs-devel, eliz

> I would argue that the purpose of forward-sexp is to move over items in
> a list.

There are different ways to look at it.  In the Lisp context where it
emerged, we only have "identifiers" and "parenthesized thingies", so
that doesn't give much guidance about what to do in-between.

The semantics I chose for SMIE is what I found to be closest to
past practice.

> Always going for the smallest subtree doesn’t seem to align with it.
> Take that example above, going across the smallest subtree means
> moving over X, then moving over “.”,

No: when you're left of ".", as in "(X|.y(z), alpha)", the SMIE
semantics of `forward-sexp` moves over ".y(z)", i.e. to
"(X.y(z)|, alpha)"

> that doesn’t feel like what forward-sexp should do to me.

Indeed, moving just over "." would be very far from my understanding of
what `forward-sexp` intends to do.

> You mean in the following code
>
> int a = 0[1];
> int b = 1;[2]
>
> Forward-sexp would move [1] to [2]?

No, SMIE's `forward-sexp` moves from

    int a = 0|;
    int b = 1;

to

    int a = 0;
    int b = 1|;

[ You can try it by installing Tuareg and moving around in an OCaml
  file, for example.  ]

and when moving backward it moves from

    int a = 0;
    int b = 1;|

to

    int a = 0;|
    int b = 1;

> But if we move over the smallest
> subtree, I’d imagine it only move across the semicolon after [1].

In my view ";" is not a substree.  It's the node of a substree.
We can't actually move over a proper subtree in that case because there
is no substree whose left boundary starts right before the ";", so the
closest is to move over the ";" *plus* its right child.

> Even if it moves from [1] to [2], needing to adjust point feels very
> inconvenient to me, at least I wouldn’t want to use something
> like that.

Moving point is the way to tell SMIE's `forward-sexp` which level of the tree
we want to navigate.  I don't believe Emacs can reliably guess the right
level, and I don't believe choosing an arbitrary level for the users
serves them best either.  I can imagine other ways to specify the
intended tree level, tho: maybe we could have a kind of prefix command
for that.  E.g. a new command that would work a bit like `C-M-u` (or its
younger sibling `expand-region`) but would only affect the next sexp
command instead.

> I want to type a single binding and move to where I want, and
> type that binding multiple times to move multiple steps.

I don't think a single binding can always jump to where you want.
That would require magic :-)

But yes, SMIE's `forward-sexp` does work well when repeated to jump over
N instructions, it was indeed an important design goal.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 14:01             ` Stefan Monnier
@ 2022-12-14 16:24               ` Theodor Thornhill
  2022-12-14 17:46                 ` Stefan Monnier
  2022-12-14 23:31               ` Yuan Fu
  1 sibling, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-14 16:24 UTC (permalink / raw)
  To: Stefan Monnier, Yuan Fu; +Cc: emacs-devel, eliz



On 14 December 2022 15:01:55 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> I would argue that the purpose of forward-sexp is to move over items in
>> a list.
>
>There are different ways to look at it.  In the Lisp context where it
>emerged, we only have "identifiers" and "parenthesized thingies", so
>that doesn't give much guidance about what to do in-between.
>
>The semantics I chose for SMIE is what I found to be closest to
>past practice.
>
>> Always going for the smallest subtree doesn’t seem to align with it.
>> Take that example above, going across the smallest subtree means
>> moving over X, then moving over “.”,
>
>No: when you're left of ".", as in "(X|.y(z), alpha)", the SMIE
>semantics of `forward-sexp` moves over ".y(z)", i.e. to
>"(X.y(z)|, alpha)"
>
>> that doesn’t feel like what forward-sexp should do to me.
>
>Indeed, moving just over "." would be very far from my understanding of
>what `forward-sexp` intends to do.
>
>> You mean in the following code
>>
>> int a = 0[1];
>> int b = 1;[2]
>>
>> Forward-sexp would move [1] to [2]?
>
>No, SMIE's `forward-sexp` moves from
>
>    int a = 0|;
>    int b = 1;
>
>to
>
>    int a = 0;
>    int b = 1|;
>
>[ You can try it by installing Tuareg and moving around in an OCaml
>  file, for example.  ]
>
>and when moving backward it moves from
>
>    int a = 0;
>    int b = 1;|
>
>to
>
>    int a = 0;|
>    int b = 1;
>
>> But if we move over the smallest
>> subtree, I’d imagine it only move across the semicolon after [1].
>

Doesn't this look like forward-sentence?


>In my view ";" is not a substree.  It's the node of a substree.
>We can't actually move over a proper subtree in that case because there
>is no substree whose left boundary starts right before the ";", so the
>closest is to move over the ";" *plus* its right child.
>
>> Even if it moves from [1] to [2], needing to adjust point feels very
>> inconvenient to me, at least I wouldn’t want to use something
>> like that.
>
>Moving point is the way to tell SMIE's `forward-sexp` which level of the tree
>we want to navigate.  I don't believe Emacs can reliably guess the right
>level, and I don't believe choosing an arbitrary level for the users
>serves them best either.  I can imagine other ways to specify the
>intended tree level, tho: maybe we could have a kind of prefix command
>for that.  E.g. a new command that would work a bit like `C-M-u` (or its
>younger sibling `expand-region`) but would only affect the next sexp
>command instead.
>
>> I want to type a single binding and move to where I want, and
>> type that binding multiple times to move multiple steps.
>
>I don't think a single binding can always jump to where you want.
>That would require magic :-)
>
>But yes, SMIE's `forward-sexp` does work well when repeated to jump over
>N instructions, it was indeed an important design goal.
>
>
>        Stefan
>



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 16:24               ` Theodor Thornhill
@ 2022-12-14 17:46                 ` Stefan Monnier
  2022-12-14 18:07                   ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 17:46 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

>>and when moving backward it moves from
>>
>>    int a = 0;
>>    int b = 1;|
>>
>>to
>>
>>    int a = 0;|
>>    int b = 1;
>>
>>> But if we move over the smallest
>>> subtree, I’d imagine it only move across the semicolon after [1].
>>
>
> Doesn't this look like forward-sentence?

In this case, yes.  But in other cases it will move at different levels
of the tree.  E.g.:

   int x = f (b + 4, c * 7 - z * 2, d, e);

It will sometimes move over the whole instruction, and other times over
just a single variable or over a whole argument or over just a "factor".
This depends on where point is when `forward/backward-sexp` is called.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 17:46                 ` Stefan Monnier
@ 2022-12-14 18:07                   ` Theodor Thornhill
  2022-12-14 19:25                     ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-14 18:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz



On 14 December 2022 18:46:36 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>>and when moving backward it moves from
>>>
>>>    int a = 0;
>>>    int b = 1;|
>>>
>>>to
>>>
>>>    int a = 0;|
>>>    int b = 1;
>>>
>>>> But if we move over the smallest
>>>> subtree, I’d imagine it only move across the semicolon after [1].
>>>
>>
>> Doesn't this look like forward-sentence?
>
>In this case, yes.  But in other cases it will move at different levels
>of the tree.  E.g.:
>
>   int x = f (b + 4, c * 7 - z * 2, d, e);
>
>It will sometimes move over the whole instruction, and other times over
>just a single variable or over a whole argument or over just a "factor".
>This depends on where point is when `forward/backward-sexp` is called.

Yeah. I think this example shows what I find unintuitive. If point is right before the first comma, and we transpose-sexps, it could end up swapping 4 for c * 7 - z * 2, which would rarely make sense in this context. Swapping b + 4 with c * 7 - z * 2 would make sense here, imo. I believe this is not how you see it? 

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 18:07                   ` Theodor Thornhill
@ 2022-12-14 19:25                     ` Stefan Monnier
  2022-12-14 19:35                       ` Stefan Monnier
  2022-12-14 20:04                       ` Theodor Thornhill
  0 siblings, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 19:25 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

>>In this case, yes.  But in other cases it will move at different levels
>>of the tree.  E.g.:
>>
>>   int x = f (b + 4, c * 7 - z * 2, d, e);
>>
>>It will sometimes move over the whole instruction, and other times over
>>just a single variable or over a whole argument or over just a "factor".
>>This depends on where point is when `forward/backward-sexp` is called.
>
> Yeah. I think this example shows what I find unintuitive. If point is right
> before the first comma, and we transpose-sexps, it could end up swapping
> 4 for c * 7 - z * 2, which would rarely make sense in this context.

If so, that would be a bug in `transpose-sexp`, agreed.
I'm talking here about `forward/backward-sexp`.
The two are linked, but we shouldn't use one to justify a bug in the other.

`Forward-sexp` from

    int x = f (b + 4|, c * 7 - z * 2, d, e);

should work by delimiting the two things to swap *plus* the thing
in-between, and in this case it should be:

    int x = f (<b + 4>, <c * 7 - z * 2>, d, e);

Notice how it needs to figure out the ", ".  Once this is figured out,
it's easy to use `forward/backward-sexp` to find the other 2 boundaries
(if you want to re-use the `forward/backward-sexp`, like the code
currently does):

Use `forward-sexp` from

    int x = f (b + 4|, c * 7 - z * 2, d, e);

and `backward-sexp` from

    int x = f (b + 4, |c * 7 - z * 2, d, e);

> Swapping b + 4 with c * 7 - z * 2 would make sense here,  imo.
> I believe this is not how you see it? 

Looks like I wasn't clear enough.  I do agree with you on this, and SMIE
agrees with you as well, if you try `M-C-t` on the above code in
tuareg-mode.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 19:25                     ` Stefan Monnier
@ 2022-12-14 19:35                       ` Stefan Monnier
  2022-12-14 20:04                       ` Theodor Thornhill
  1 sibling, 0 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 19:35 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

>>>In this case, yes.  But in other cases it will move at different levels
>>>of the tree.  E.g.:
>>>
>>>   int x = f (b + 4, c * 7 - z * 2, d, e);
>>>
>>>It will sometimes move over the whole instruction, and other times over
>>>just a single variable or over a whole argument or over just a "factor".
>>>This depends on where point is when `forward/backward-sexp` is called.
>>
>> Yeah. I think this example shows what I find unintuitive. If point is right
>> before the first comma, and we transpose-sexps, it could end up swapping
>> 4 for c * 7 - z * 2, which would rarely make sense in this context.
>
> If so, that would be a bug in `transpose-sexp`, agreed.
> I'm talking here about `forward/backward-sexp`.
> The two are linked, but we shouldn't use one to justify a bug in the other.
>
> `Forward-sexp` from
   ^^^^^^^^^^^^
   transpose-sexp

Duh!


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 19:25                     ` Stefan Monnier
  2022-12-14 19:35                       ` Stefan Monnier
@ 2022-12-14 20:04                       ` Theodor Thornhill
  2022-12-14 20:50                         ` Stefan Monnier
  1 sibling, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-14 20:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>In this case, yes.  But in other cases it will move at different levels
>>>of the tree.  E.g.:
>>>
>>>   int x = f (b + 4, c * 7 - z * 2, d, e);
>>>
>>>It will sometimes move over the whole instruction, and other times over
>>>just a single variable or over a whole argument or over just a "factor".
>>>This depends on where point is when `forward/backward-sexp` is called.
>>
>> Yeah. I think this example shows what I find unintuitive. If point is right
>> before the first comma, and we transpose-sexps, it could end up swapping
>> 4 for c * 7 - z * 2, which would rarely make sense in this context.
>
> If so, that would be a bug in `transpose-sexp`, agreed.
> I'm talking here about `forward/backward-sexp`.
> The two are linked, but we shouldn't use one to justify a bug in the other.

Sure, but I think they necessarily needs to be viewed as a whole.  If we
drop tree-sitter or SMIE (which I actually know pretty well) for one
moment, the cc-mode based java-mode would exhibit the exact behavior I
described.  If it's a bug in tranpsose-sexps it is definitely an issue
with forward/backward-sexp, because in every situation the positions to
be swapped is just "backward-sexp - forward-sexp - forward-sexp -
backward-sexp", right? And the thing in the middle, usually a comma,
operators or other is the space between that doesn't move.  I also
observe this fixme inside of transpose-words:

  ;; FIXME: `foo a!nd bar' should transpose into `bar and foo'.

I read this more like it's how transpose-sexps should behave on text.
There are almost no differences between forward-word and forward-sexp in
normal prose, bar the case of delimiters, IIUC.  Wouldn't it make sense
to make transpose-sexps actually do what that fixme asks?

And why is the

		 (cons (progn (funcall mover x) (point))
		       (progn (funcall mover (- x)) (point)))

in this form, and not some pseudo-code like:
(cons '(backward-thing-from-start-point forward-thing-point)
      '(forward-thing-from-start-point backward-thing-point))


So that 'foo a|nd bar' would create these points:

|foo| a|nd |bar|
1   2  ^   4   3
       start


Then forward-word could behave like it does now.  Now I'm having issues
where movement over sexps ends up not in the same place.


>
> `Forward-sexp` from
>
>     int x = f (b + 4|, c * 7 - z * 2, d, e);
>
> should work by delimiting the two things to swap *plus* the thing
> in-between, and in this case it should be:
>
>     int x = f (<b + 4>, <c * 7 - z * 2>, d, e);
>
> Notice how it needs to figure out the ", ".  Once this is figured out,
> it's easy to use `forward/backward-sexp` to find the other 2 boundaries
> (if you want to re-use the `forward/backward-sexp`, like the code
> currently does):
>
> Use `forward-sexp` from
>
>     int x = f (b + 4|, c * 7 - z * 2, d, e);
>
> and `backward-sexp` from
>
>     int x = f (b + 4, |c * 7 - z * 2, d, e);
>
>> Swapping b + 4 with c * 7 - z * 2 would make sense here,  imo.
>> I believe this is not how you see it? 
>
> Looks like I wasn't clear enough.  I do agree with you on this, and SMIE
> agrees with you as well, if you try `M-C-t` on the above code in
> tuareg-mode.

I think we agree, yes.  Thanks for taking the time :-)

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 20:04                       ` Theodor Thornhill
@ 2022-12-14 20:50                         ` Stefan Monnier
  2022-12-14 21:15                           ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 20:50 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

>>>>In this case, yes.  But in other cases it will move at different levels
>>>>of the tree.  E.g.:
>>>>
>>>>   int x = f (b + 4, c * 7 - z * 2, d, e);
>>>>
>>>>It will sometimes move over the whole instruction, and other times over
>>>>just a single variable or over a whole argument or over just a "factor".
>>>>This depends on where point is when `forward/backward-sexp` is called.
>>>
>>> Yeah. I think this example shows what I find unintuitive. If point is right
>>> before the first comma, and we transpose-sexps, it could end up swapping
>>> 4 for c * 7 - z * 2, which would rarely make sense in this context.
>>
>> If so, that would be a bug in `transpose-sexp`, agreed.
>> I'm talking here about `forward/backward-sexp`.
>> The two are linked, but we shouldn't use one to justify a bug in the other.
>
> Sure, but I think they necessarily needs to be viewed as a whole.  If we
> drop tree-sitter or SMIE (which I actually know pretty well) for one
> moment, the cc-mode based java-mode would exhibit the exact behavior I
> described.

Really?  When I try it out in CC-mode's java-mode, I get from

    int x = f (b + 4|, c * 7 - z * 2, d, e);
to
    int x = f (b + c, 4| * 7 - z * 2, d, e);

which is not completely non-sensical, but is somewhere between a bug and
a misfeature.

> If it's a bug in tranpsose-sexps it is definitely an issue
> with forward/backward-sexp, because in every situation the positions to
> be swapped is just "backward-sexp - forward-sexp - forward-sexp -
> backward-sexp", right?

The way I see it, the problem with sexp movement and infix syntax is
that a given buffer position maps to several positions at different
levels in the AST (contrary to Lisp style syntax where there is no such
ambiguity).

So for every command, we need to decide/guess at which level of the AST
the user wants to operate.  For `transpose-sexp` we have more
information than for `forward/backward-sexp` because some of those
positions are "non-sensical" in the sense that they would end up swapping
subtrees that live at different levels or that do not share their
immediate parent.

For this reason, what we should do with `transpose-sexp` is not necessarily
exactly the same as what we should do with `backward/forward-sexp`.

> And the thing in the middle, usually a comma,
> operators or other is the space between that doesn't move.  I also
> observe this fixme inside of transpose-words:
>
>   ;; FIXME: `foo a!nd bar' should transpose into `bar and foo'.
>
> I read this more like it's how transpose-sexps should behave on text.

IIRC I wrote this when I was working on the SMIE `transpose-sexp` code :-)

> Wouldn't it make sense to make transpose-sexps actually do what that
> fixme asks?

I obviously agree, since I wrote that fixme.

> And why is the
>
> 		 (cons (progn (funcall mover x) (point))
> 		       (progn (funcall mover (- x)) (point)))
>
> in this form, and not some pseudo-code like:
> (cons '(backward-thing-from-start-point forward-thing-point)
>       '(forward-thing-from-start-point backward-thing-point))

Sorry, haven't looked at the code in a while.
Not sure what you're getting at here.  I suspect that in the case of
tree-sitter you'd ideally want to implement `transpose-sexp` directly
rather than via something like `forward/backward-sexp`:
- Go from point to a node in the tree.
- Find the node whose children we want to swap.
- Find the bounds of those two children.
- Do the actual textual swap.

> Now I'm having issues where movement over sexps ends up not in the
> same place.

Same place as?


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 20:50                         ` Stefan Monnier
@ 2022-12-14 21:15                           ` Theodor Thornhill
  2022-12-14 21:34                             ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-14 21:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>>>In this case, yes.  But in other cases it will move at different levels
>>>>>of the tree.  E.g.:
>>>>>
>>>>>   int x = f (b + 4, c * 7 - z * 2, d, e);
>>>>>
>>>>>It will sometimes move over the whole instruction, and other times over
>>>>>just a single variable or over a whole argument or over just a "factor".
>>>>>This depends on where point is when `forward/backward-sexp` is called.
>>>>
>>>> Yeah. I think this example shows what I find unintuitive. If point is right
>>>> before the first comma, and we transpose-sexps, it could end up swapping
>>>> 4 for c * 7 - z * 2, which would rarely make sense in this context.
>>>
>>> If so, that would be a bug in `transpose-sexp`, agreed.
>>> I'm talking here about `forward/backward-sexp`.
>>> The two are linked, but we shouldn't use one to justify a bug in the other.
>>
>> Sure, but I think they necessarily needs to be viewed as a whole.  If we
>> drop tree-sitter or SMIE (which I actually know pretty well) for one
>> moment, the cc-mode based java-mode would exhibit the exact behavior I
>> described.
>
> Really?  When I try it out in CC-mode's java-mode, I get from
>
>     int x = f (b + 4|, c * 7 - z * 2, d, e);
> to
>     int x = f (b + c, 4| * 7 - z * 2, d, e);
>
> which is not completely non-sensical, but is somewhere between a bug and
> a misfeature.

Yeah, I misspoke, sorry.  I get the same, and obviously agree.  I
believe I wanted to say something like "I get the same behavior with
transpose-sexps as with transpose-word", or something like that.

>
>> If it's a bug in tranpsose-sexps it is definitely an issue
>> with forward/backward-sexp, because in every situation the positions to
>> be swapped is just "backward-sexp - forward-sexp - forward-sexp -
>> backward-sexp", right?
>
> The way I see it, the problem with sexp movement and infix syntax is
> that a given buffer position maps to several positions at different
> levels in the AST (contrary to Lisp style syntax where there is no such
> ambiguity).
>
> So for every command, we need to decide/guess at which level of the AST
> the user wants to operate.  For `transpose-sexp` we have more
> information than for `forward/backward-sexp` because some of those
> positions are "non-sensical" in the sense that they would end up swapping
> subtrees that live at different levels or that do not share their
> immediate parent.
>
> For this reason, what we should do with `transpose-sexp` is not necessarily
> exactly the same as what we should do with `backward/forward-sexp`.
>

Yeah, I agree.  I could create a treesit-transpose-sexps that doesn't
use forward-sexp and uses the 'special (which probably should be
documented) argument, similarly to how it's implemented now.

>> And the thing in the middle, usually a comma,
>> operators or other is the space between that doesn't move.  I also
>> observe this fixme inside of transpose-words:
>>
>>   ;; FIXME: `foo a!nd bar' should transpose into `bar and foo'.
>>
>> I read this more like it's how transpose-sexps should behave on text.
>
> IIRC I wrote this when I was working on the SMIE `transpose-sexp` code :-)
>
>> Wouldn't it make sense to make transpose-sexps actually do what that
>> fixme asks?
>
> I obviously agree, since I wrote that fixme.
>

Great.  It seems there has been almost no development, nor documentation
done in this area for a long time.  Should I try to improve on this part
of the code while I'm at it?

>> And why is the
>>
>> 		 (cons (progn (funcall mover x) (point))
>> 		       (progn (funcall mover (- x)) (point)))
>>
>> in this form, and not some pseudo-code like:
>> (cons '(backward-thing-from-start-point forward-thing-point)
>>       '(forward-thing-from-start-point backward-thing-point))
>
> Sorry, haven't looked at the code in a while.
> Not sure what you're getting at here.  I suspect that in the case of
> tree-sitter you'd ideally want to implement `transpose-sexp` directly
> rather than via something like `forward/backward-sexp`:
> - Go from point to a node in the tree.
> - Find the node whose children we want to swap.
> - Find the bounds of those two children.
> - Do the actual textual swap.
>

Yeah, that's what I'm thinking too.  I'm just thinking we should be
clear on what a word/sexp/sentence/paragraph/defun etc is in non-lisp
and non-human languages.


>> Now I'm having issues where movement over sexps ends up not in the
>> same place.
>
> Same place as?
>

IIRC there's no guarantee that the movement sequence used for
transpose-sexp moves over the same blocks of code, so in non-lisp
languages there's no real semantic to go from.  I'll find an example.

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 21:15                           ` Theodor Thornhill
@ 2022-12-14 21:34                             ` Stefan Monnier
  2022-12-15 19:37                               ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-14 21:34 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> Great.  It seems there has been almost no development, nor documentation
> done in this area for a long time.  Should I try to improve on this part
> of the code while I'm at it?

That would be welcome, yes.

> Yeah, that's what I'm thinking too.  I'm just thinking we should be
> clear on what a word/sexp/sentence/paragraph/defun etc is in non-lisp
> and non-human languages.

In the context of SMIE I came up with a usable meaning for sexp (which
I've been trying to explain in this thread), but for sentence and
paragraph it seems harder and is likely to depend on the specifics of
the language (e.g. for some languages "sentence" might be mapped to
"instruction" or maybe "instruction that doesn't itself contain nested
instructions", but some languages don't have a notion of instruction).

>>> Now I'm having issues where movement over sexps ends up not in the
>>> same place.
>> Same place as?
> IIRC there's no guarantee that the movement sequence used for
> transpose-sexp moves over the same blocks of code, so in non-lisp
> languages there's no real semantic to go from.

I guess in general it can be difficult to be consistent, indeed.
In SMIE I preserve the following (or at least I try to):

    <FOO> infix <BAR>
    ^   ^       ^   ^
    AB  AE      BB  BE

if `transpose-sexp` swaps FOO and BAR, then `forward-sexp` from AE goes
to BE and `backward-sexp` from BB goes to AB.  But when you start to
consider mixfix syntax it can become much less clear what needs to
be done.  I don't think we should worry too much if `transpose-sexp` and
`forward/backward-sexp` don't align 100% is all cases: we should strive
to keep them consistent, but it's OK to break down in corner cases.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 14:01             ` Stefan Monnier
  2022-12-14 16:24               ` Theodor Thornhill
@ 2022-12-14 23:31               ` Yuan Fu
  2022-12-15  0:05                 ` Yuan Fu
                                   ` (2 more replies)
  1 sibling, 3 replies; 54+ messages in thread
From: Yuan Fu @ 2022-12-14 23:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, emacs-devel, eliz



> On Dec 14, 2022, at 6:01 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
>> I would argue that the purpose of forward-sexp is to move over items in
>> a list.
> 
> There are different ways to look at it.  In the Lisp context where it
> emerged, we only have "identifiers" and "parenthesized thingies", so
> that doesn't give much guidance about what to do in-between.

I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.

> 
> The semantics I chose for SMIE is what I found to be closest to
> past practice.

That’ great! You’ve done all the experiments and thinking, and all I need to do is to understand it ;-)

>> But if we move over the smallest
>> subtree, I’d imagine it only move across the semicolon after [1].
> 
> In my view ";" is not a substree.  It's the node of a substree.
> We can't actually move over a proper subtree in that case because there
> is no substree whose left boundary starts right before the ";", so the
> closest is to move over the ";" *plus* its right child.

Ah, so by “smallest subtree” you basically mean “smallest non-leaf node”? Is the following logic what you have in mind?

forward-sexp:
Among all nodes that starts right after point:
1. if we can find a smallest non-leaf node
   -> skip over it
2. if we can only find leaf node
   -> go to the end of the immediate (smallest) parent node
      that covers point, and skip over its next sibling (by recursively applying either 1 or 2)

Yuan


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-12 15:46 ` Stefan Monnier
  2022-12-13 18:27   ` Theodor Thornhill
@ 2022-12-14 23:32   ` Stephen Leake
  2022-12-16 10:02     ` Kévin Le Gouguec
  1 sibling, 1 reply; 54+ messages in thread
From: Stephen Leake @ 2022-12-14 23:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, emacs-devel, eliz, casouri

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Cool, thanks, a few comments (based on my experience with adding
> similar things based on `smie`):
>
>> ** Forward-sexp:
>> Executing C-M-f repeatedly will go from:
>> ```
>> public void foo(|String bar, String baz) {}
>> ```
>> to
>> ```
>> public void foo(String bar|, String baz) {}
>> ```
>
> That looks wrong.  `String` is a valid AST node.  Whether it gets a node
> in tree-sitter or not, I don't know, but here there are several "sexps"
> that start at point and I think `forward-sexp` should be conservative
> and keep advancing by the smallest option.

ada-mode has something similar; forward-sexp goes to the next labeled
keyword. That's more useful than stopping at every little AST node. It
relies on markup in the grammar to label the keywords; tree-sitter would
need another markup similar to the current indent markup.

-- 
-- Stephe



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 23:31               ` Yuan Fu
@ 2022-12-15  0:05                 ` Yuan Fu
  2022-12-15  7:09                   ` Eli Zaretskii
  2022-12-15  4:37                 ` Stefan Monnier
  2022-12-15  5:59                 ` Theodor Thornhill
  2 siblings, 1 reply; 54+ messages in thread
From: Yuan Fu @ 2022-12-15  0:05 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, emacs-devel, eliz



> On Dec 14, 2022, at 3:31 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Dec 14, 2022, at 6:01 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> 
>>> I would argue that the purpose of forward-sexp is to move over items in
>>> a list.
>> 
>> There are different ways to look at it.  In the Lisp context where it
>> emerged, we only have "identifiers" and "parenthesized thingies", so
>> that doesn't give much guidance about what to do in-between.
> 
> I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.

Ok, upon closer inspection, forward-list isn’t exactly what I’m thinking about, it moves over lists, but I’m thinking about moving over elements of a list.

If you think about it, there are two kinds of constructs in an AST/grammar: those are repeatable, and those are not. Statements are repeatable, you can stack multiple ones together and it still makes sense. Identifiers are not repeatable, put two together doesn’t make sense (in most languages).

And these repeatable constructs appears at every level of the AST, from top-level stuff like function definition, to statements, to small stuff like arguments in an argument list. That makes them very good unit of navigation.

Navigation commands, IMO, needs to be extremely easy to predict and requires no thinking. That’s the reason why avy never grow on me: yes you can move to anywhere you want, but it takes so much cognitive load to use. Much better to type a few C-n, C-M-f, etc to get their, because I don’t need to think about it.

Does this make sense? 

Yuan


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 23:31               ` Yuan Fu
  2022-12-15  0:05                 ` Yuan Fu
@ 2022-12-15  4:37                 ` Stefan Monnier
  2022-12-15  5:59                 ` Theodor Thornhill
  2 siblings, 0 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-15  4:37 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Theodor Thornhill, emacs-devel, eliz

>> In my view ";" is not a substree.  It's the node of a substree.
>> We can't actually move over a proper subtree in that case because there
>> is no substree whose left boundary starts right before the ";", so the
>> closest is to move over the ";" *plus* its right child.
>
> Ah, so by “smallest subtree” you basically mean “smallest non-leaf node”?

Hmm... I don't think so, tho I guess it depends on what is your notion
of "node" and "leaf".  In my book, "a = x + 1; b = y" is a tree of the
form:
                      ;
                     / \
                    /   \
                   =     =
                  /|     |\
                 / |     | \
                a  +     b  y
                  / \
                 /   \
                x     1

So there is no ";" subtree at all (there is a subtree with ";" in its
root but it contains all of "a = x + 1; b = y").  OTOH there is an "x"
subtree and it would be the "smallest subtree" if we're immediately to
the left of this "x" and we're doing a `forward-sexp`.  So my "smallest
subtree" can definitely be a "leaf node" since I consider "x" to be
a leaf node.

[ And the ";" node at the top can have N children, by the way.  ]

If point is to the left of the semi-colon, it can be considered to be:
A) right after "1"
B) right after "x + 1"
C) right after "a = x + 1"
But in order to be able to "move forward" you need to move up to the ";"
node, so only C makes sense, and the smallest meaningful subtree
over which we can move is the "b = y" subtree, since in order to move
only over "b" we'd have to go down inside the "=" node, so we'd be
moving over a subtree that's not connected to our starting node.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 23:31               ` Yuan Fu
  2022-12-15  0:05                 ` Yuan Fu
  2022-12-15  4:37                 ` Stefan Monnier
@ 2022-12-15  5:59                 ` Theodor Thornhill
  2022-12-15 21:23                   ` Yuan Fu
  2 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15  5:59 UTC (permalink / raw)
  To: Yuan Fu, Stefan Monnier; +Cc: emacs-devel, eliz



On 15 December 2022 00:31:20 CET, Yuan Fu <casouri@gmail.com> wrote:
>
>
>> On Dec 14, 2022, at 6:01 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> 
>>> I would argue that the purpose of forward-sexp is to move over items in
>>> a list.
>> 
>> There are different ways to look at it.  In the Lisp context where it
>> emerged, we only have "identifiers" and "parenthesized thingies", so
>> that doesn't give much guidance about what to do in-between.
>
>I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.
>
>> 
>> The semantics I chose for SMIE is what I found to be closest to
>> past practice.
>
>That’ great! You’ve done all the experiments and thinking, and all I need to do is to understand it ;-)
>

Are you working on this, yuan? If so I'll get out of your hair.

>>> But if we move over the smallest
>>> subtree, I’d imagine it only move across the semicolon after [1].
>> 
>> In my view ";" is not a substree.  It's the node of a substree.
>> We can't actually move over a proper subtree in that case because there
>> is no substree whose left boundary starts right before the ";", so the
>> closest is to move over the ";" *plus* its right child.
>
>Ah, so by “smallest subtree” you basically mean “smallest non-leaf node”? Is the following logic what you have in mind?
>
>forward-sexp:
>Among all nodes that starts right after point:
>1. if we can find a smallest non-leaf node
>   -> skip over it
>2. if we can only find leaf node
>   -> go to the end of the immediate (smallest) parent node
>      that covers point, and skip over its next sibling (by recursively applying either 1 or 2)
>
>Yuan



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15  0:05                 ` Yuan Fu
@ 2022-12-15  7:09                   ` Eli Zaretskii
  2022-12-15  7:14                     ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2022-12-15  7:09 UTC (permalink / raw)
  To: Yuan Fu; +Cc: monnier, theo, emacs-devel

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 14 Dec 2022 16:05:10 -0800
> Cc: Theodor Thornhill <theo@thornhill.no>,
>  emacs-devel <emacs-devel@gnu.org>,
>  eliz@gnu.org
> 
> > I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.
> 
> Ok, upon closer inspection, forward-list isn’t exactly what I’m thinking about, it moves over lists, but I’m thinking about moving over elements of a list.
> 
> If you think about it, there are two kinds of constructs in an AST/grammar: those are repeatable, and those are not. Statements are repeatable, you can stack multiple ones together and it still makes sense. Identifiers are not repeatable, put two together doesn’t make sense (in most languages).
> 
> And these repeatable constructs appears at every level of the AST, from top-level stuff like function definition, to statements, to small stuff like arguments in an argument list. That makes them very good unit of navigation.
> 
> Navigation commands, IMO, needs to be extremely easy to predict and requires no thinking. That’s the reason why avy never grow on me: yes you can move to anywhere you want, but it takes so much cognitive load to use. Much better to type a few C-n, C-M-f, etc to get their, because I don’t need to think about it.

FWIW, for me, "sexp" in its C/C++ interpretation always means
"expression".  So in

   foobar (a, b + c, 2 * d - f / 10 + pow (g, x), y);

C-M-f should move to the next comma on the top level (i.e. the comma
inside the 'pow' call doesn't count), and C-M-f inside the 'pow' call
should move by commas on that level.

More generally, C-M-f should move to the next expression
_on_the_same_level_, without entering inner levels.  Exactly like we
do in Lisp.



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15  7:09                   ` Eli Zaretskii
@ 2022-12-15  7:14                     ` Theodor Thornhill
  0 siblings, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15  7:14 UTC (permalink / raw)
  To: Eli Zaretskii, Yuan Fu; +Cc: monnier, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Yuan Fu <casouri@gmail.com>
>> Date: Wed, 14 Dec 2022 16:05:10 -0800
>> Cc: Theodor Thornhill <theo@thornhill.no>,
>>  emacs-devel <emacs-devel@gnu.org>,
>>  eliz@gnu.org
>> 
>> > I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.
>> 
>> Ok, upon closer inspection, forward-list isn’t exactly what I’m thinking about, it moves over lists, but I’m thinking about moving over elements of a list.
>> 
>> If you think about it, there are two kinds of constructs in an AST/grammar: those are repeatable, and those are not. Statements are repeatable, you can stack multiple ones together and it still makes sense. Identifiers are not repeatable, put two together doesn’t make sense (in most languages).
>> 
>> And these repeatable constructs appears at every level of the AST, from top-level stuff like function definition, to statements, to small stuff like arguments in an argument list. That makes them very good unit of navigation.
>> 
>> Navigation commands, IMO, needs to be extremely easy to predict and requires no thinking. That’s the reason why avy never grow on me: yes you can move to anywhere you want, but it takes so much cognitive load to use. Much better to type a few C-n, C-M-f, etc to get their, because I don’t need to think about it.
>
> FWIW, for me, "sexp" in its C/C++ interpretation always means
> "expression".  So in
>
>    foobar (a, b + c, 2 * d - f / 10 + pow (g, x), y);
>
> C-M-f should move to the next comma on the top level (i.e. the comma
> inside the 'pow' call doesn't count), and C-M-f inside the 'pow' call
> should move by commas on that level.
>
> More generally, C-M-f should move to the next expression
> _on_the_same_level_, without entering inner levels.  Exactly like we
> do in Lisp.

Agreed



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 21:34                             ` Stefan Monnier
@ 2022-12-15 19:37                               ` Theodor Thornhill
  2022-12-15 19:56                                 ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15 19:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Great.  It seems there has been almost no development, nor documentation
>> done in this area for a long time.  Should I try to improve on this part
>> of the code while I'm at it?
>
> That would be welcome, yes.
>

Ok, will do.

>> Yeah, that's what I'm thinking too.  I'm just thinking we should be
>> clear on what a word/sexp/sentence/paragraph/defun etc is in non-lisp
>> and non-human languages.
>
> In the context of SMIE I came up with a usable meaning for sexp (which
> I've been trying to explain in this thread), but for sentence and
> paragraph it seems harder and is likely to depend on the specifics of
> the language (e.g. for some languages "sentence" might be mapped to
> "instruction" or maybe "instruction that doesn't itself contain nested
> instructions", but some languages don't have a notion of instruction).
>

Yeah, I understand.

>>>> Now I'm having issues where movement over sexps ends up not in the
>>>> same place.
>>> Same place as?
>> IIRC there's no guarantee that the movement sequence used for
>> transpose-sexp moves over the same blocks of code, so in non-lisp
>> languages there's no real semantic to go from.
>
> I guess in general it can be difficult to be consistent, indeed.
> In SMIE I preserve the following (or at least I try to):
>
>     <FOO> infix <BAR>
>     ^   ^       ^   ^
>     AB  AE      BB  BE
>
> if `transpose-sexp` swaps FOO and BAR, then `forward-sexp` from AE goes
> to BE and `backward-sexp` from BB goes to AB.  But when you start to
> consider mixfix syntax it can become much less clear what needs to
> be done.  I don't think we should worry too much if `transpose-sexp` and
> `forward/backward-sexp` don't align 100% is all cases: we should strive
> to keep them consistent, but it's OK to break down in corner cases.
>

Actually, I think there is a nice solution waiting for us.  If we
consider the 'balanced expressions' to be swapped in transpose-sexps as
siblings, we could just:

(defun treesit-transpose-sexps (&optional arg)
  "Tree-sitter `beginning-of-defun' function.
ARG is the same as in `beginning-of-defun'."
  (interactive "*p")
  (if-let* ((node (treesit-node-at (point)))
            (prev (treesit-node-prev-sibling node))
            (next (treesit-node-next-sibling node)))
      (list (cons (treesit-node-start prev)
                  (treesit-node-end prev))
            (cons (treesit-node-start next)
                  (treesit-node-end next)))
    ;; Hack to trigger the error message in transpose-subr-1 when we
    ;; don't have siblings to swap.
    (list (cons 0 1) (cons 0 1))))

If this code is plugged into transpose-sexps we get this nice behavior:

Python:

```
def foo():
    return x |if x == 5 else False

def foo():
    return x == 5 if x| else False
```

and

```
def foo():
    return x if x == 5 el|se False

def foo():
    return x if False else x == 5|
```

and

```
def foo():
    return| x if x == 5 else False ;; Don't have two things to transpose
```

Java:

```
public class foo {
    void foo(String x,| int y) {}
}

public class foo {
    void foo(int y, String x|) {}
```

and

```
public class foo {
    void foo() {
        if (x =|= y) {}
    }
}

public class foo {
    void foo() {
        if (y == x|) {}
    }
}
```

and

```
public class foo {
    void foo(String x, int y) {
        foo(a + 4|, c * b + y, b, d).bar();
    }
}

public class foo {
    void foo(String x, int y) {
        foo(c * b + y, a + 4|, b, d).bar();
    }
}
```

and 

```
foo(a + 4, c * b |+ y, b, d);

foo(a + 4, y + c * b, b, d);
```


Now forward/backward-sexp can actually work a little differently, as you
suggest, or we can let it use the same "move over siblings"-semantic.
In that case we don't even need the treesit-sexp-type-regexp variables to
control this, I think.

What do you think?

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 19:37                               ` Theodor Thornhill
@ 2022-12-15 19:56                                 ` Stefan Monnier
  2022-12-15 20:03                                   ` Theodor Thornhill
  2022-12-27 15:46                                   ` Lynn Winebarger
  0 siblings, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-15 19:56 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> If this code is plugged into transpose-sexps we get this nice behavior:

It's a bit different from what SMIE would do, but there's a lot of
overlap and when it's different it's arguably better, so sounds good
to me.

> Now forward/backward-sexp can actually work a little differently, as you
> suggest, or we can let it use the same "move over siblings"-semantic.
> In that case we don't even need the treesit-sexp-type-regexp variables to
> control this, I think.
>
> What do you think?

I'm not sufficiently familiar with the tree-sitter tree to foresee
precisely how it would affect `forward/backward-sexp`, but I think you
have a good enough understanding to make a good judgment at this
point :-)


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 19:56                                 ` Stefan Monnier
@ 2022-12-15 20:03                                   ` Theodor Thornhill
  2022-12-15 20:33                                     ` Theodor Thornhill
  2022-12-27 15:46                                   ` Lynn Winebarger
  1 sibling, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15 20:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> If this code is plugged into transpose-sexps we get this nice behavior:
>
> It's a bit different from what SMIE would do, but there's a lot of
> overlap and when it's different it's arguably better, so sounds good
> to me.
>

Great!

>> Now forward/backward-sexp can actually work a little differently, as you
>> suggest, or we can let it use the same "move over siblings"-semantic.
>> In that case we don't even need the treesit-sexp-type-regexp variables to
>> control this, I think.
>>
>> What do you think?
>
> I'm not sufficiently familiar with the tree-sitter tree to foresee
> precisely how it would affect `forward/backward-sexp`, but I think you
> have a good enough understanding to make a good judgment at this
> point :-)

Great. I'll prepare a patch for this behavior, and we can discuss the
forward-* commands after that.

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 20:03                                   ` Theodor Thornhill
@ 2022-12-15 20:33                                     ` Theodor Thornhill
  2022-12-15 20:57                                       ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15 20:33 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

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

Theodor Thornhill <theo@thornhill.no> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> If this code is plugged into transpose-sexps we get this nice behavior:
>>
>> It's a bit different from what SMIE would do, but there's a lot of
>> overlap and when it's different it's arguably better, so sounds good
>> to me.
>>
>
> Great!
>
>>> Now forward/backward-sexp can actually work a little differently, as you
>>> suggest, or we can let it use the same "move over siblings"-semantic.
>>> In that case we don't even need the treesit-sexp-type-regexp variables to
>>> control this, I think.
>>>
>>> What do you think?
>>
>> I'm not sufficiently familiar with the tree-sitter tree to foresee
>> precisely how it would affect `forward/backward-sexp`, but I think you
>> have a good enough understanding to make a good judgment at this
>> point :-)
>
> Great. I'll prepare a patch for this behavior, and we can discuss the
> forward-* commands after that.
>

What do you think about this?  Feel free to try it and let me know if
something is completely wrong :-)

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-treesit-transpose-sexps.patch --]
[-- Type: text/x-diff, Size: 5835 bytes --]

From 6b693d9733811d1ecfe8aa0bd0dd52151ba19bee Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Thu, 15 Dec 2022 21:22:13 +0100
Subject: [PATCH] Add treesit-transpose-sexps

We don't really need to rely on forward-sexp to define what to
transpose.  In tree-sitter we can consider siblings as "balanced
expressions", and swap them without doing any movement to calculate
where the siblings in question are.

* lisp/simple.el: Add requires for treesit.
* lisp/simple.el (transpose-sexps): If tree-sitter is available, use
treesit-transpose-sexps as the 'special' function.
(transpose-subr): Just use tree-sitter when available.
* lisp/treesit.el (treesit-transpose-sexps): New function.
---
 lisp/simple.el  | 68 +++++++++++++++++++++++++++----------------------
 lisp/treesit.el | 15 +++++++++++
 2 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 893a43b03f..235f758baa 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -28,10 +28,12 @@
 
 ;;; Code:
 
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'cl-lib)
+                   (require 'treesit))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
 (declare-function shell-mode "shell" ())
+(declare-function treesit-parser-list "treesit.c")
 
 ;;; From compile.el
 (defvar compilation-current-error)
@@ -8453,36 +8455,37 @@ transpose-sexps
           (transpose-sexps arg nil)
         (scan-error (user-error "Not between two complete sexps")))
     (transpose-subr
-     (lambda (arg)
-       ;; Here we should try to simulate the behavior of
-       ;; (cons (progn (forward-sexp x) (point))
-       ;;       (progn (forward-sexp (- x)) (point)))
-       ;; Except that we don't want to rely on the second forward-sexp
-       ;; putting us back to where we want to be, since forward-sexp-function
-       ;; might do funny things like infix-precedence.
-       (if (if (> arg 0)
-	       (looking-at "\\sw\\|\\s_")
-	     (and (not (bobp))
-		  (save-excursion
-                    (forward-char -1)
-                    (looking-at "\\sw\\|\\s_"))))
-	   ;; Jumping over a symbol.  We might be inside it, mind you.
-	   (progn (funcall (if (> arg 0)
-			       'skip-syntax-backward 'skip-syntax-forward)
-			   "w_")
-		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
-         ;; Otherwise, we're between sexps.  Take a step back before jumping
-         ;; to make sure we'll obey the same precedence no matter which
-         ;; direction we're going.
-         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
-                  " .")
-         (cons (save-excursion (forward-sexp arg) (point))
-	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
-			         (not (zerop (funcall (if (> arg 0)
-							  'skip-syntax-forward
-						        'skip-syntax-backward)
-						      ".")))))
-		      (point)))))
+     (if (treesit-parser-list) #'treesit-transpose-sexps
+       (lambda (arg)
+         ;; Here we should try to simulate the behavior of
+         ;; (cons (progn (forward-sexp x) (point))
+         ;;       (progn (forward-sexp (- x)) (point)))
+         ;; Except that we don't want to rely on the second forward-sexp
+         ;; putting us back to where we want to be, since forward-sexp-function
+         ;; might do funny things like infix-precedence.
+         (if (if (> arg 0)
+	         (looking-at "\\sw\\|\\s_")
+	       (and (not (bobp))
+		    (save-excursion
+                      (forward-char -1)
+                      (looking-at "\\sw\\|\\s_"))))
+             ;; Jumping over a symbol.  We might be inside it, mind you.
+	     (progn (funcall (if (> arg 0)
+			         'skip-syntax-backward 'skip-syntax-forward)
+			     "w_")
+		    (cons (save-excursion (forward-sexp arg) (point)) (point)))
+           ;; Otherwise, we're between sexps.  Take a step back before jumping
+           ;; to make sure we'll obey the same precedence no matter which
+           ;; direction we're going.
+           (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
+                    " .")
+           (cons (save-excursion (forward-sexp arg) (point))
+	         (progn (while (or (forward-comment (if (> arg 0) 1 -1))
+			           (not (zerop (funcall (if (> arg 0)
+							    'skip-syntax-forward
+						          'skip-syntax-backward)
+						        ".")))))
+		        (point))))))
      arg 'special)))
 
 (defun transpose-lines (arg)
@@ -8521,6 +8524,9 @@ transpose-subr
 		       (progn (funcall mover (- x)) (point))))))
 	pos1 pos2)
     (cond
+     ((treesit-parser-list)
+      (cl-multiple-value-bind (p1 p2) (funcall aux 1)
+        (transpose-subr-1 p1 p2)))
      ((= arg 0)
       (save-excursion
 	(setq pos1 (funcall aux 1))
diff --git a/lisp/treesit.el b/lisp/treesit.el
index 913a1d8c5b..36f01159f6 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1620,6 +1620,21 @@ treesit--defun-maybe-top-level
                  node)
       finally return node))))
 
+(defun treesit-transpose-sexps (&optional arg)
+  "Tree-sitter `transpose-sexps' function.
+Arg is the same as in `transpose-sexps'."
+  (interactive "*p")
+  (if-let* ((node (treesit-node-at (point)))
+            (prev (treesit-node-prev-sibling node))
+            (next (treesit-node-next-sibling node)))
+
+      (list (cons (treesit-node-start prev)
+                  (treesit-node-end prev))
+            (cons (treesit-node-start next)
+                  (treesit-node-end next)))
+    ;; Hack to trigger the error message in transpose-subr-1 when we
+    ;; don't have siblings to swap.
+    (list (cons 0 1) (cons 0 1))))
 (defun treesit-beginning-of-defun (&optional arg)
   "Tree-sitter `beginning-of-defun' function.
 ARG is the same as in `beginning-of-defun'."
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 20:33                                     ` Theodor Thornhill
@ 2022-12-15 20:57                                       ` Theodor Thornhill
  2022-12-24  7:00                                         ` Eli Zaretskii
  2022-12-24 14:01                                         ` Stefan Monnier
  0 siblings, 2 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15 20:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

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

Theodor Thornhill <theo@thornhill.no> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>
>>>> If this code is plugged into transpose-sexps we get this nice behavior:
>>>
>>> It's a bit different from what SMIE would do, but there's a lot of
>>> overlap and when it's different it's arguably better, so sounds good
>>> to me.
>>>
>>
>> Great!
>>
>>>> Now forward/backward-sexp can actually work a little differently, as you
>>>> suggest, or we can let it use the same "move over siblings"-semantic.
>>>> In that case we don't even need the treesit-sexp-type-regexp variables to
>>>> control this, I think.
>>>>
>>>> What do you think?
>>>
>>> I'm not sufficiently familiar with the tree-sitter tree to foresee
>>> precisely how it would affect `forward/backward-sexp`, but I think you
>>> have a good enough understanding to make a good judgment at this
>>> point :-)
>>
>> Great. I'll prepare a patch for this behavior, and we can discuss the
>> forward-* commands after that.
>>
>
> What do you think about this?  Feel free to try it and let me know if
> something is completely wrong :-)

Now you can use 'arg' as well.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-treesit-transpose-sexps.patch --]
[-- Type: text/x-diff, Size: 5947 bytes --]

From 0ccb449aee15ba1bdd13e5380e5341733a3c9f99 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Thu, 15 Dec 2022 21:22:13 +0100
Subject: [PATCH] Add treesit-transpose-sexps

We don't really need to rely on forward-sexp to define what to
transpose.  In tree-sitter we can consider siblings as "balanced
expressions", and swap them without doing any movement to calculate
where the siblings in question are.

* lisp/simple.el: Add requires for treesit.
* lisp/simple.el (transpose-sexps): If tree-sitter is available, use
treesit-transpose-sexps as the 'special' function.
(transpose-subr): Just use tree-sitter when available.
* lisp/treesit.el (treesit-transpose-sexps): New function.
---
 lisp/simple.el  | 68 +++++++++++++++++++++++++++----------------------
 lisp/treesit.el | 16 ++++++++++++
 2 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 893a43b03f..d6252ea9e6 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -28,10 +28,12 @@
 
 ;;; Code:
 
-(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'cl-lib)
+                   (require 'treesit))
 
 (declare-function widget-convert "wid-edit" (type &rest args))
 (declare-function shell-mode "shell" ())
+(declare-function treesit-parser-list "treesit.c")
 
 ;;; From compile.el
 (defvar compilation-current-error)
@@ -8453,36 +8455,37 @@ transpose-sexps
           (transpose-sexps arg nil)
         (scan-error (user-error "Not between two complete sexps")))
     (transpose-subr
-     (lambda (arg)
-       ;; Here we should try to simulate the behavior of
-       ;; (cons (progn (forward-sexp x) (point))
-       ;;       (progn (forward-sexp (- x)) (point)))
-       ;; Except that we don't want to rely on the second forward-sexp
-       ;; putting us back to where we want to be, since forward-sexp-function
-       ;; might do funny things like infix-precedence.
-       (if (if (> arg 0)
-	       (looking-at "\\sw\\|\\s_")
-	     (and (not (bobp))
-		  (save-excursion
-                    (forward-char -1)
-                    (looking-at "\\sw\\|\\s_"))))
-	   ;; Jumping over a symbol.  We might be inside it, mind you.
-	   (progn (funcall (if (> arg 0)
-			       'skip-syntax-backward 'skip-syntax-forward)
-			   "w_")
-		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
-         ;; Otherwise, we're between sexps.  Take a step back before jumping
-         ;; to make sure we'll obey the same precedence no matter which
-         ;; direction we're going.
-         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
-                  " .")
-         (cons (save-excursion (forward-sexp arg) (point))
-	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
-			         (not (zerop (funcall (if (> arg 0)
-							  'skip-syntax-forward
-						        'skip-syntax-backward)
-						      ".")))))
-		      (point)))))
+     (if (treesit-parser-list) #'treesit-transpose-sexps
+       (lambda (arg)
+         ;; Here we should try to simulate the behavior of
+         ;; (cons (progn (forward-sexp x) (point))
+         ;;       (progn (forward-sexp (- x)) (point)))
+         ;; Except that we don't want to rely on the second forward-sexp
+         ;; putting us back to where we want to be, since forward-sexp-function
+         ;; might do funny things like infix-precedence.
+         (if (if (> arg 0)
+	         (looking-at "\\sw\\|\\s_")
+	       (and (not (bobp))
+		    (save-excursion
+                      (forward-char -1)
+                      (looking-at "\\sw\\|\\s_"))))
+             ;; Jumping over a symbol.  We might be inside it, mind you.
+	     (progn (funcall (if (> arg 0)
+			         'skip-syntax-backward 'skip-syntax-forward)
+			     "w_")
+		    (cons (save-excursion (forward-sexp arg) (point)) (point)))
+           ;; Otherwise, we're between sexps.  Take a step back before jumping
+           ;; to make sure we'll obey the same precedence no matter which
+           ;; direction we're going.
+           (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
+                    " .")
+           (cons (save-excursion (forward-sexp arg) (point))
+	         (progn (while (or (forward-comment (if (> arg 0) 1 -1))
+			           (not (zerop (funcall (if (> arg 0)
+							    'skip-syntax-forward
+						          'skip-syntax-backward)
+						        ".")))))
+		        (point))))))
      arg 'special)))
 
 (defun transpose-lines (arg)
@@ -8521,6 +8524,9 @@ transpose-subr
 		       (progn (funcall mover (- x)) (point))))))
 	pos1 pos2)
     (cond
+     ((treesit-parser-list)
+      (cl-multiple-value-bind (p1 p2) (funcall aux arg)
+        (transpose-subr-1 p1 p2)))
      ((= arg 0)
       (save-excursion
 	(setq pos1 (funcall aux 1))
diff --git a/lisp/treesit.el b/lisp/treesit.el
index 913a1d8c5b..016f6d19eb 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1620,6 +1620,22 @@ treesit--defun-maybe-top-level
                  node)
       finally return node))))
 
+(defun treesit-transpose-sexps (&optional arg)
+  "Tree-sitter `transpose-sexps' function.
+Arg is the same as in `transpose-sexps'."
+  (interactive "*p")
+  (if-let* ((node (treesit-node-at (point)))
+            (parent (treesit-node-parent node))
+            (index (treesit-node-index node))
+            (prev (treesit-node-child parent (1- index)))
+            (next (treesit-node-child parent (+ arg index))))
+      (list (cons (treesit-node-start prev)
+                  (treesit-node-end prev))
+            (cons (treesit-node-start next)
+                  (treesit-node-end next)))
+    ;; Hack to trigger the error message in transpose-subr-1 when we
+    ;; don't have siblings to swap.
+    (list (cons 0 1) (cons 0 1))))
 (defun treesit-beginning-of-defun (&optional arg)
   "Tree-sitter `beginning-of-defun' function.
 ARG is the same as in `beginning-of-defun'."
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15  5:59                 ` Theodor Thornhill
@ 2022-12-15 21:23                   ` Yuan Fu
  2022-12-15 21:28                     ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Yuan Fu @ 2022-12-15 21:23 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Stefan Monnier, emacs-devel, eliz



> On Dec 14, 2022, at 9:59 PM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> 
> 
> On 15 December 2022 00:31:20 CET, Yuan Fu <casouri@gmail.com> wrote:
>> 
>> 
>>> On Dec 14, 2022, at 6:01 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> 
>>>> I would argue that the purpose of forward-sexp is to move over items in
>>>> a list.
>>> 
>>> There are different ways to look at it.  In the Lisp context where it
>>> emerged, we only have "identifiers" and "parenthesized thingies", so
>>> that doesn't give much guidance about what to do in-between.
>> 
>> I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.
>> 
>>> 
>>> The semantics I chose for SMIE is what I found to be closest to
>>> past practice.
>> 
>> That’ great! You’ve done all the experiments and thinking, and all I need to do is to understand it ;-)
>> 
> 
> Are you working on this, yuan? If so I'll get out of your hair.

No I’m working on something else, please go ahead!

Yuan


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 21:23                   ` Yuan Fu
@ 2022-12-15 21:28                     ` Theodor Thornhill
  0 siblings, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-15 21:28 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Monnier, emacs-devel, eliz



On 15 December 2022 22:23:45 CET, Yuan Fu <casouri@gmail.com> wrote:
>
>
>> On Dec 14, 2022, at 9:59 PM, Theodor Thornhill <theo@thornhill.no> wrote:
>> 
>> 
>> 
>> On 15 December 2022 00:31:20 CET, Yuan Fu <casouri@gmail.com> wrote:
>>> 
>>> 
>>>> On Dec 14, 2022, at 6:01 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>>> 
>>>>> I would argue that the purpose of forward-sexp is to move over items in
>>>>> a list.
>>>> 
>>>> There are different ways to look at it.  In the Lisp context where it
>>>> emerged, we only have "identifiers" and "parenthesized thingies", so
>>>> that doesn't give much guidance about what to do in-between.
>>> 
>>> I see, so maybe sexp means the general, flexible AST entity. And thinking of it, my idea is just forward-list :-) And we definitely should include forward-list into the list of navigation commands we want to support, among the ones that are already brought up.
>>> 
>>>> 
>>>> The semantics I chose for SMIE is what I found to be closest to
>>>> past practice.
>>> 
>>> That’ great! You’ve done all the experiments and thinking, and all I need to do is to understand it ;-)
>>> 
>> 
>> Are you working on this, yuan? If so I'll get out of your hair.
>
>No I’m working on something else, please go ahead!
>
>Yuan

Ok!



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-14 23:32   ` Stephen Leake
@ 2022-12-16 10:02     ` Kévin Le Gouguec
  2022-12-16 11:54       ` [SPAM UNSURE] " Stephen Leake
  0 siblings, 1 reply; 54+ messages in thread
From: Kévin Le Gouguec @ 2022-12-16 10:02 UTC (permalink / raw)
  To: Stephen Leake
  Cc: Stefan Monnier, Theodor Thornhill, emacs-devel, eliz, casouri

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> Cool, thanks, a few comments (based on my experience with adding
>> similar things based on `smie`):
>>
>>> ** Forward-sexp:
>>> Executing C-M-f repeatedly will go from:
>>> ```
>>> public void foo(|String bar, String baz) {}
>>> ```
>>> to
>>> ```
>>> public void foo(String bar|, String baz) {}
>>> ```
>>
>> That looks wrong.  `String` is a valid AST node.  Whether it gets a node
>> in tree-sitter or not, I don't know, but here there are several "sexps"
>> that start at point and I think `forward-sexp` should be conservative
>> and keep advancing by the smallest option.
>
> ada-mode has something similar; forward-sexp goes to the next labeled
> keyword. That's more useful than stopping at every little AST node. It
> relies on markup in the grammar to label the keywords; tree-sitter would
> need another markup similar to the current indent markup.

Chiming in since I got bitten by this in ada-mode right now: as someone
used to mark long_identifiers_with_underscores with C-M-SPC in other
modes, it's tripping me up that mark-sexp (and sexp movement in general)
"overshoots" and goes over much bigger expressions in ada-mode.

Identifiers are a unit I manipulate much more often than other kinds of
expressions, so it's useful for me to have C-M-* recognize them.  I find
it tolerable to "spam" C-M-* when needed to move over bigger units; YMMV
🤷



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

* Re: [SPAM UNSURE] Re: Plug treesit.el into other emacs constructs
  2022-12-16 10:02     ` Kévin Le Gouguec
@ 2022-12-16 11:54       ` Stephen Leake
  2022-12-17 15:30         ` Kévin Le Gouguec
  0 siblings, 1 reply; 54+ messages in thread
From: Stephen Leake @ 2022-12-16 11:54 UTC (permalink / raw)
  To: Kévin Le Gouguec
  Cc: Stefan Monnier, Theodor Thornhill, emacs-devel, eliz, casouri

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>
>> ada-mode has something similar; forward-sexp goes to the next labeled
>> keyword. That's more useful than stopping at every little AST node. It
>> relies on markup in the grammar to label the keywords; tree-sitter would
>> need another markup similar to the current indent markup.
>
> Chiming in since I got bitten by this in ada-mode right now: as someone
> used to mark long_identifiers_with_underscores with C-M-SPC in other
> modes, it's tripping me up that mark-sexp (and sexp movement in general)
> "overshoots" and goes over much bigger expressions in ada-mode.

Sounds like we need an option for this in ada-mode?

My solution to this is to bind C-<right> to (forward-symbol 1), and
C-<left> to (forward-symbol -1), so these keys move over whole
identifiers.

-- 
-- Stephe



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

* Re: [SPAM UNSURE] Re: Plug treesit.el into other emacs constructs
  2022-12-16 11:54       ` [SPAM UNSURE] " Stephen Leake
@ 2022-12-17 15:30         ` Kévin Le Gouguec
  0 siblings, 0 replies; 54+ messages in thread
From: Kévin Le Gouguec @ 2022-12-17 15:30 UTC (permalink / raw)
  To: Stephen Leake
  Cc: Stefan Monnier, Theodor Thornhill, emacs-devel, eliz, casouri

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
>
>> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>>
>>> ada-mode has something similar; forward-sexp goes to the next labeled
>>> keyword. That's more useful than stopping at every little AST node. It
>>> relies on markup in the grammar to label the keywords; tree-sitter would
>>> need another markup similar to the current indent markup.
>>
>> Chiming in since I got bitten by this in ada-mode right now: as someone
>> used to mark long_identifiers_with_underscores with C-M-SPC in other
>> modes, it's tripping me up that mark-sexp (and sexp movement in general)
>> "overshoots" and goes over much bigger expressions in ada-mode.
>
> Sounds like we need an option for this in ada-mode?

That'd be very welcome indeed, as far as I am concerned (no rush though;
I can take a stab at it on a rainy day if no-one else is clamoring for
it).

> My solution to this is to bind C-<right> to (forward-symbol 1), and
> C-<left> to (forward-symbol -1), so these keys move over whole
> identifiers.

Interesting; I don't think the *-symbol commands were on my radar (no
mark-symbol though AFAICT).

(I see now we also have {forward,backward}-list, bound to C-M-{n,p};
that's another pair I don't often use; I do use C-M-{d,u} a lot, but for
"horizontal" traversal I usually rely on C-M-{f,b})



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 20:57                                       ` Theodor Thornhill
@ 2022-12-24  7:00                                         ` Eli Zaretskii
  2022-12-24  8:44                                           ` Yuan Fu
  2022-12-24 14:01                                         ` Stefan Monnier
  1 sibling, 1 reply; 54+ messages in thread
From: Eli Zaretskii @ 2022-12-24  7:00 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: monnier, casouri, emacs-devel

> From: Theodor Thornhill <theo@thornhill.no>
> Cc: Yuan Fu <casouri@gmail.com>, emacs-devel@gnu.org, eliz@gnu.org
> Date: Thu, 15 Dec 2022 21:57:31 +0100
> 
> Theodor Thornhill <theo@thornhill.no> writes:
> 
> > Theodor Thornhill <theo@thornhill.no> writes:
> >
> >> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> >>
> >>>> If this code is plugged into transpose-sexps we get this nice behavior:
> >>>
> >>> It's a bit different from what SMIE would do, but there's a lot of
> >>> overlap and when it's different it's arguably better, so sounds good
> >>> to me.
> >>>
> >>
> >> Great!
> >>
> >>>> Now forward/backward-sexp can actually work a little differently, as you
> >>>> suggest, or we can let it use the same "move over siblings"-semantic.
> >>>> In that case we don't even need the treesit-sexp-type-regexp variables to
> >>>> control this, I think.
> >>>>
> >>>> What do you think?
> >>>
> >>> I'm not sufficiently familiar with the tree-sitter tree to foresee
> >>> precisely how it would affect `forward/backward-sexp`, but I think you
> >>> have a good enough understanding to make a good judgment at this
> >>> point :-)
> >>
> >> Great. I'll prepare a patch for this behavior, and we can discuss the
> >> forward-* commands after that.
> >>
> >
> > What do you think about this?  Feel free to try it and let me know if
> > something is completely wrong :-)
> 
> Now you can use 'arg' as well.

Stefan, Yuan: any comments or objections to installing this on master?



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-24  7:00                                         ` Eli Zaretskii
@ 2022-12-24  8:44                                           ` Yuan Fu
  0 siblings, 0 replies; 54+ messages in thread
From: Yuan Fu @ 2022-12-24  8:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Theodor Thornhill, monnier, emacs-devel



> On Dec 23, 2022, at 11:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Theodor Thornhill <theo@thornhill.no>
>> Cc: Yuan Fu <casouri@gmail.com>, emacs-devel@gnu.org, eliz@gnu.org
>> Date: Thu, 15 Dec 2022 21:57:31 +0100
>> 
>> Theodor Thornhill <theo@thornhill.no> writes:
>> 
>>> Theodor Thornhill <theo@thornhill.no> writes:
>>> 
>>>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>>> 
>>>>>> If this code is plugged into transpose-sexps we get this nice behavior:
>>>>> 
>>>>> It's a bit different from what SMIE would do, but there's a lot of
>>>>> overlap and when it's different it's arguably better, so sounds good
>>>>> to me.
>>>>> 
>>>> 
>>>> Great!
>>>> 
>>>>>> Now forward/backward-sexp can actually work a little differently, as you
>>>>>> suggest, or we can let it use the same "move over siblings"-semantic.
>>>>>> In that case we don't even need the treesit-sexp-type-regexp variables to
>>>>>> control this, I think.
>>>>>> 
>>>>>> What do you think?
>>>>> 
>>>>> I'm not sufficiently familiar with the tree-sitter tree to foresee
>>>>> precisely how it would affect `forward/backward-sexp`, but I think you
>>>>> have a good enough understanding to make a good judgment at this
>>>>> point :-)
>>>> 
>>>> Great. I'll prepare a patch for this behavior, and we can discuss the
>>>> forward-* commands after that.
>>>> 
>>> 
>>> What do you think about this?  Feel free to try it and let me know if
>>> something is completely wrong :-)
>> 
>> Now you can use 'arg' as well.
> 
> Stefan, Yuan: any comments or objections to installing this on master?

This is the same as the patch in bug#60128, right? I don’t have much to comment about it. It looks pretty nice to me :-)

Yuan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 20:57                                       ` Theodor Thornhill
  2022-12-24  7:00                                         ` Eli Zaretskii
@ 2022-12-24 14:01                                         ` Stefan Monnier
  2022-12-24 14:15                                           ` Theodor Thornhill
  1 sibling, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-24 14:01 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> -(eval-when-compile (require 'cl-lib))
> +(eval-when-compile (require 'cl-lib)
> +                   (require 'treesit))

I don't see any part of `treesit` used during compilation.
Is it a left over, maybe?

> @@ -8453,36 +8455,37 @@ transpose-sexps
>            (transpose-sexps arg nil)
>          (scan-error (user-error "Not between two complete sexps")))
>      (transpose-subr
> -     (lambda (arg)
> -       ;; Here we should try to simulate the behavior of
> -       ;; (cons (progn (forward-sexp x) (point))
> -       ;;       (progn (forward-sexp (- x)) (point)))
> -       ;; Except that we don't want to rely on the second forward-sexp
> -       ;; putting us back to where we want to be, since forward-sexp-function
> -       ;; might do funny things like infix-precedence.
> -       (if (if (> arg 0)
> -	       (looking-at "\\sw\\|\\s_")
> -	     (and (not (bobp))
> -		  (save-excursion
> -                    (forward-char -1)
> -                    (looking-at "\\sw\\|\\s_"))))
> -	   ;; Jumping over a symbol.  We might be inside it, mind you.
> -	   (progn (funcall (if (> arg 0)
> -			       'skip-syntax-backward 'skip-syntax-forward)
> -			   "w_")
> -		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
> -         ;; Otherwise, we're between sexps.  Take a step back before jumping
> -         ;; to make sure we'll obey the same precedence no matter which
> -         ;; direction we're going.
> -         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
> -                  " .")
> -         (cons (save-excursion (forward-sexp arg) (point))
> -	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
> -			         (not (zerop (funcall (if (> arg 0)
> -							  'skip-syntax-forward
> -						        'skip-syntax-backward)
> -						      ".")))))
> -		      (point)))))
> +     (if (treesit-parser-list) #'treesit-transpose-sexps
> +       (lambda (arg)
> +         ;; Here we should try to simulate the behavior of
> +         ;; (cons (progn (forward-sexp x) (point))
> +         ;;       (progn (forward-sexp (- x)) (point)))
> +         ;; Except that we don't want to rely on the second forward-sexp
> +         ;; putting us back to where we want to be, since forward-sexp-function
> +         ;; might do funny things like infix-precedence.
> +         (if (if (> arg 0)
> +	         (looking-at "\\sw\\|\\s_")
> +	       (and (not (bobp))
> +		    (save-excursion
> +                      (forward-char -1)
> +                      (looking-at "\\sw\\|\\s_"))))
> +             ;; Jumping over a symbol.  We might be inside it, mind you.
> +	     (progn (funcall (if (> arg 0)
> +			         'skip-syntax-backward 'skip-syntax-forward)
> +			     "w_")
> +		    (cons (save-excursion (forward-sexp arg) (point)) (point)))
> +           ;; Otherwise, we're between sexps.  Take a step back before jumping
> +           ;; to make sure we'll obey the same precedence no matter which
> +           ;; direction we're going.
> +           (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
> +                    " .")
> +           (cons (save-excursion (forward-sexp arg) (point))
> +	         (progn (while (or (forward-comment (if (> arg 0) 1 -1))
> +			           (not (zerop (funcall (if (> arg 0)
> +							    'skip-syntax-forward
> +						          'skip-syntax-backward)
> +						        ".")))))
> +		        (point))))))
>       arg 'special)))

Could we use a `transpose-sexp-function` variable, which `treesit` can
then set, so `simple.el` doesn't need to know about `treesit` at all?

>  (defun transpose-lines (arg)
> @@ -8521,6 +8524,9 @@ transpose-subr
>  		       (progn (funcall mover (- x)) (point))))))
>  	pos1 pos2)
>      (cond
> +     ((treesit-parser-list)
> +      (cl-multiple-value-bind (p1 p2) (funcall aux arg)
> +        (transpose-subr-1 p1 p2)))
>       ((= arg 0)
>        (save-excursion
>  	(setq pos1 (funcall aux 1))

Please use `pcase-let` instead of `cl-multiple-value-bind` (especially
since you use it to decompose something built with `list` rather than
with `cl-values`).

Also to avid re-testing `treesit-parser-list`, I'd recommend you extend
the semantics of `mover` so it can either return a position (the old
protocol) or directly return a pair of positions.  You could even add
a 3rd kind of return value to explicitly trigger the error message
instead of relying on the (cons 0 1) hack.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-24 14:01                                         ` Stefan Monnier
@ 2022-12-24 14:15                                           ` Theodor Thornhill
  2022-12-26 19:11                                             ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-24 14:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz



On 24 December 2022 15:01:36 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> -(eval-when-compile (require 'cl-lib))
>> +(eval-when-compile (require 'cl-lib)
>> +                   (require 'treesit))
>
>I don't see any part of `treesit` used during compilation.
>Is it a left over, maybe?
>
>> @@ -8453,36 +8455,37 @@ transpose-sexps
>>            (transpose-sexps arg nil)
>>          (scan-error (user-error "Not between two complete sexps")))
>>      (transpose-subr
>> -     (lambda (arg)
>> -       ;; Here we should try to simulate the behavior of
>> -       ;; (cons (progn (forward-sexp x) (point))
>> -       ;;       (progn (forward-sexp (- x)) (point)))
>> -       ;; Except that we don't want to rely on the second forward-sexp
>> -       ;; putting us back to where we want to be, since forward-sexp-function
>> -       ;; might do funny things like infix-precedence.
>> -       (if (if (> arg 0)
>> -	       (looking-at "\\sw\\|\\s_")
>> -	     (and (not (bobp))
>> -		  (save-excursion
>> -                    (forward-char -1)
>> -                    (looking-at "\\sw\\|\\s_"))))
>> -	   ;; Jumping over a symbol.  We might be inside it, mind you.
>> -	   (progn (funcall (if (> arg 0)
>> -			       'skip-syntax-backward 'skip-syntax-forward)
>> -			   "w_")
>> -		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
>> -         ;; Otherwise, we're between sexps.  Take a step back before jumping
>> -         ;; to make sure we'll obey the same precedence no matter which
>> -         ;; direction we're going.
>> -         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
>> -                  " .")
>> -         (cons (save-excursion (forward-sexp arg) (point))
>> -	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
>> -			         (not (zerop (funcall (if (> arg 0)
>> -							  'skip-syntax-forward
>> -						        'skip-syntax-backward)
>> -						      ".")))))
>> -		      (point)))))
>> +     (if (treesit-parser-list) #'treesit-transpose-sexps
>> +       (lambda (arg)
>> +         ;; Here we should try to simulate the behavior of
>> +         ;; (cons (progn (forward-sexp x) (point))
>> +         ;;       (progn (forward-sexp (- x)) (point)))
>> +         ;; Except that we don't want to rely on the second forward-sexp
>> +         ;; putting us back to where we want to be, since forward-sexp-function
>> +         ;; might do funny things like infix-precedence.
>> +         (if (if (> arg 0)
>> +	         (looking-at "\\sw\\|\\s_")
>> +	       (and (not (bobp))
>> +		    (save-excursion
>> +                      (forward-char -1)
>> +                      (looking-at "\\sw\\|\\s_"))))
>> +             ;; Jumping over a symbol.  We might be inside it, mind you.
>> +	     (progn (funcall (if (> arg 0)
>> +			         'skip-syntax-backward 'skip-syntax-forward)
>> +			     "w_")
>> +		    (cons (save-excursion (forward-sexp arg) (point)) (point)))
>> +           ;; Otherwise, we're between sexps.  Take a step back before jumping
>> +           ;; to make sure we'll obey the same precedence no matter which
>> +           ;; direction we're going.
>> +           (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
>> +                    " .")
>> +           (cons (save-excursion (forward-sexp arg) (point))
>> +	         (progn (while (or (forward-comment (if (> arg 0) 1 -1))
>> +			           (not (zerop (funcall (if (> arg 0)
>> +							    'skip-syntax-forward
>> +						          'skip-syntax-backward)
>> +						        ".")))))
>> +		        (point))))))
>>       arg 'special)))
>
>Could we use a `transpose-sexp-function` variable, which `treesit` can
>then set, so `simple.el` doesn't need to know about `treesit` at all?
>

Yes absolutely! I'll make that change. It makes sense, because we need not really rely on forward-foo anyways:)

>>  (defun transpose-lines (arg)
>> @@ -8521,6 +8524,9 @@ transpose-subr
>>  		       (progn (funcall mover (- x)) (point))))))
>>  	pos1 pos2)
>>      (cond
>> +     ((treesit-parser-list)
>> +      (cl-multiple-value-bind (p1 p2) (funcall aux arg)
>> +        (transpose-subr-1 p1 p2)))
>>       ((= arg 0)
>>        (save-excursion
>>  	(setq pos1 (funcall aux 1))
>
>Please use `pcase-let` instead of `cl-multiple-value-bind` (especially
>since you use it to decompose something built with `list` rather than
>with `cl-values`).
>
>Also to avid re-testing `treesit-parser-list`, I'd recommend you extend
>the semantics of `mover` so it can either return a position (the old
>protocol) or directly return a pair of positions.  You could even add
>a 3rd kind of return value to explicitly trigger the error message
>instead of relying on the (cons 0 1) hack.
>
>
>        Stefan
>

Yeah! I believe this either wasn't the latest patch, or i forgot to send it. I'll see what lies around my system and wrap things up.



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-24 14:15                                           ` Theodor Thornhill
@ 2022-12-26 19:11                                             ` Theodor Thornhill
  2022-12-26 22:46                                               ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-26 19:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

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

uld we use a `transpose-sexp-function` variable, which `treesit` can
>>then set, so `simple.el` doesn't need to know about `treesit` at all?
>>
>
> Yes absolutely! I'll make that change. It makes sense, because we need not really rely on forward-foo anyways:)
>
>>>  (defun transpose-lines (arg)
>>> @@ -8521,6 +8524,9 @@ transpose-subr
>>>  		       (progn (funcall mover (- x)) (point))))))
>>>  	pos1 pos2)
>>>      (cond
>>> +     ((treesit-parser-list)
>>> +      (cl-multiple-value-bind (p1 p2) (funcall aux arg)
>>> +        (transpose-subr-1 p1 p2)))
>>>       ((= arg 0)
>>>        (save-excursion
>>>  	(setq pos1 (funcall aux 1))
>>
>>Please use `pcase-let` instead of `cl-multiple-value-bind` (especially
>>since you use it to decompose something built with `list` rather than
>>with `cl-values`).
>>
>>Also to avid re-testing `treesit-parser-list`, I'd recommend you extend
>>the semantics of `mover` so it can either return a position (the old
>>protocol) or directly return a pair of positions.  You could even add
>>a 3rd kind of return value to explicitly trigger the error message
>>instead of relying on the (cons 0 1) hack.
>>
>>
>>        Stefan
>>
>
> Yeah! I believe this either wasn't the latest patch, or i forgot to
> send it. I'll see what lies around my system and wrap things up.


What do you think about something like this?

It feels a little iffy how to handle the separate return values, but it
works.  I'd be super happy for some feedback on how to best solve that,
though :)

Also, I made the treesit-transpose-sexps a little better imo, in that we
only find named nodes to swap, but use every available node for the
entry. We rarely, if ever want to swap the unnamed nodes.

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-treesit-transpose-sexps-bug-60128.patch --]
[-- Type: text/x-diff, Size: 8114 bytes --]

From 0dc412eaf16123dcb65381970fb82c0741809753 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sun, 25 Dec 2022 20:11:59 +0100
Subject: [PATCH] Add treesit-transpose-sexps (bug#60128)

We don't really need to rely on forward-sexp to define what to
transpose.  In tree-sitter we can consider siblings as "balanced
expressions", and swap them without doing any movement to calculate
where the siblings in question are.

* lisp/simple.el (transpose-sexps-function): New defvar-local.
(transpose-sexps): Use the new defvar-local if available.
(transpose-subr): Check whether the mover function returns a cons of
conses, then run transpose-subr-1 on the position-pairs.
* lisp/treesit.el (treesit-transpose-sexps): New function.
---
 lisp/simple.el  | 97 ++++++++++++++++++++++++++++---------------------
 lisp/treesit.el | 24 +++++++++++-
 2 files changed, 78 insertions(+), 43 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 4551b749d5..591b659c62 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8438,6 +8438,14 @@ transpose-words
   (interactive "*p")
   (transpose-subr 'forward-word arg))
 
+(defvar-local transpose-sexps-function nil
+  "If non-nil, `transpose-sexps' delegates to this function.
+
+The return value of this function is expected to be a cons of two
+conses, denoting the positions in the current buffer to be
+transposed.  If no such pair of positions is available, signal
+USER-ERROR.")
+
 (defun transpose-sexps (arg &optional interactive)
   "Like \\[transpose-chars] (`transpose-chars'), but applies to sexps.
 Unlike `transpose-words', point must be between the two sexps and not
@@ -8454,36 +8462,37 @@ transpose-sexps
           (transpose-sexps arg nil)
         (scan-error (user-error "Not between two complete sexps")))
     (transpose-subr
-     (lambda (arg)
-       ;; Here we should try to simulate the behavior of
-       ;; (cons (progn (forward-sexp x) (point))
-       ;;       (progn (forward-sexp (- x)) (point)))
-       ;; Except that we don't want to rely on the second forward-sexp
-       ;; putting us back to where we want to be, since forward-sexp-function
-       ;; might do funny things like infix-precedence.
-       (if (if (> arg 0)
-	       (looking-at "\\sw\\|\\s_")
-	     (and (not (bobp))
-		  (save-excursion
-                    (forward-char -1)
-                    (looking-at "\\sw\\|\\s_"))))
-	   ;; Jumping over a symbol.  We might be inside it, mind you.
-	   (progn (funcall (if (> arg 0)
-			       'skip-syntax-backward 'skip-syntax-forward)
-			   "w_")
-		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
-         ;; Otherwise, we're between sexps.  Take a step back before jumping
-         ;; to make sure we'll obey the same precedence no matter which
-         ;; direction we're going.
-         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
-                  " .")
-         (cons (save-excursion (forward-sexp arg) (point))
-	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
-			         (not (zerop (funcall (if (> arg 0)
-							  'skip-syntax-forward
-						        'skip-syntax-backward)
-						      ".")))))
-		      (point)))))
+     (if transpose-sexps-function transpose-sexps-function
+       (lambda (arg)
+         ;; Here we should try to simulate the behavior of
+         ;; (cons (progn (forward-sexp x) (point))
+         ;;       (progn (forward-sexp (- x)) (point)))
+         ;; Except that we don't want to rely on the second forward-sexp
+         ;; putting us back to where we want to be, since forward-sexp-function
+         ;; might do funny things like infix-precedence.
+         (if (if (> arg 0)
+	         (looking-at "\\sw\\|\\s_")
+	       (and (not (bobp))
+	            (save-excursion
+                      (forward-char -1)
+                      (looking-at "\\sw\\|\\s_"))))
+             ;; Jumping over a symbol.  We might be inside it, mind you.
+	     (progn (funcall (if (> arg 0)
+			         #'skip-syntax-backward #'skip-syntax-forward)
+			     "w_")
+	            (cons (save-excursion (forward-sexp arg) (point)) (point)))
+           ;; Otherwise, we're between sexps.  Take a step back before jumping
+           ;; to make sure we'll obey the same precedence no matter which
+           ;; direction we're going.
+           (funcall (if (> arg 0) #'skip-syntax-backward #'skip-syntax-forward)
+                    " .")
+           (cons (save-excursion (forward-sexp arg) (point))
+	         (progn (while (or (forward-comment (if (> arg 0) 1 -1))
+			           (not (zerop (funcall (if (> arg 0)
+						            #'skip-syntax-forward
+						          #'skip-syntax-backward)
+						        ".")))))
+		        (point))))))
      arg 'special)))
 
 (defun transpose-lines (arg)
@@ -8509,19 +8518,23 @@ transpose-lines
 ;; FIXME document SPECIAL.
 (defun transpose-subr (mover arg &optional special)
   "Subroutine to do the work of transposing objects.
-Works for lines, sentences, paragraphs, etc.  MOVER is a function that
-moves forward by units of the given object (e.g. `forward-sentence',
-`forward-paragraph').  If ARG is zero, exchanges the current object
-with the one containing mark.  If ARG is an integer, moves the
-current object past ARG following (if ARG is positive) or
-preceding (if ARG is negative) objects, leaving point after the
-current object."
-  (let ((aux (if special mover
-	       (lambda (x)
-		 (cons (progn (funcall mover x) (point))
-		       (progn (funcall mover (- x)) (point))))))
-	pos1 pos2)
+Works for lines, sentences, paragraphs, etc.  MOVER is either a
+function that moves forward by units of the given
+object (e.g. `forward-sentence', `forward-paragraph'), or a
+function that calculates a cons of two position-pairs.  If ARG is
+zero, exchanges the current object with the one containing mark.
+If ARG is an integer, moves the current object past ARG
+following (if ARG is positive) or preceding (if ARG is negative)
+objects, leaving point after the current object."
+  (let* ((aux (if special mover
+	        (lambda (x)
+		  (cons (progn (funcall mover x) (point))
+		        (progn (funcall mover (- x)) (point))))))
+	 (pos1 (save-excursion (funcall aux arg)))
+         pos2)
     (cond
+     ((and (consp (car pos1)) (consp (cdr pos1)))
+      (transpose-subr-1 (car pos1) (cdr pos1)))
      ((= arg 0)
       (save-excursion
 	(setq pos1 (funcall aux 1))
diff --git a/lisp/treesit.el b/lisp/treesit.el
index cefbed1a16..9f0965ac68 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1582,6 +1582,27 @@ treesit-search-forward-goto
       (goto-char current-pos)))
     node))
 
+(defun treesit-transpose-sexps (&optional arg)
+  "Tree-sitter `transpose-sexps' function.
+Arg is the same as in `transpose-sexps'.
+
+Return a pair of positions describing the regions to transpose
+for use in `transpose-subr' and friends."
+  (let* ((parent (treesit-node-parent (treesit-node-at (point))))
+         (child (treesit-node-child parent 0 t)))
+    (named-let loop ((prev child)
+                     (next (treesit-node-child
+                            parent (+ arg (treesit-node-index child t))
+                            t)))
+      (if (< (point) (or (treesit-node-end next)
+                         (user-error "Don't have two things to transpose")))
+          (cons (cons (treesit-node-start prev)
+                      (treesit-node-end prev))
+                (cons (treesit-node-start next)
+                      (treesit-node-end next)))
+        (loop (treesit-node-next-sibling prev t)
+              (treesit-node-next-sibling next t))))))
+
 ;;; Navigation, defun, things
 ;;
 ;; Emacs lets you define "things" by a regexp that matches the type of
@@ -2111,7 +2132,8 @@ treesit-major-mode-setup
   ;; Defun name.
   (when treesit-defun-name-function
     (setq-local add-log-current-defun-function
-                #'treesit-add-log-current-defun)))
+                #'treesit-add-log-current-defun))
+  (setq-local transpose-sexps-function #'treesit-transpose-sexps))
 
 ;;; Debugging
 
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-26 19:11                                             ` Theodor Thornhill
@ 2022-12-26 22:46                                               ` Stefan Monnier
  2022-12-26 22:51                                                 ` Stefan Monnier
  2022-12-26 22:56                                                 ` Theodor Thornhill
  0 siblings, 2 replies; 54+ messages in thread
From: Stefan Monnier @ 2022-12-26 22:46 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> +(defvar-local transpose-sexps-function nil
> +  "If non-nil, `transpose-sexps' delegates to this function.
> +
> +The return value of this function is expected to be a cons of two
> +conses, denoting the positions in the current buffer to be
> +transposed.  If no such pair of positions is available, signal
> +USER-ERROR.")

This docstring needs to tell what args are passed to the function.

I see you make it return a pair of pairs, so it has to handle all the
semantics of `transpose-sexps`.  My intuition told me to go with
a function that returns a pair of positions (i.e. it takes an ARG and
returns the BEG..END of the ARGth sibling).  I suspect it would fit
within `transpose-subr` a bit better.

> +     (if transpose-sexps-function transpose-sexps-function
> +       (lambda (arg)

Aka (or transpose-sexps-function (lambda (arg) ...))
But even better is to put the `lambda` in the default value of the
variable, so you just use `transpose-sexps-function` unconditionally.

> +  (let* ((aux (if special mover
> +	        (lambda (x)
> +		  (cons (progn (funcall mover x) (point))
> +		        (progn (funcall mover (- x)) (point))))))

If `mover` is changed to return a pair of positions, than the above can
just be:

> +  (let* ((aux (if special mover
> +	        (lambda (x)
> +		  (cons (progn (funcall mover x) (point))
> +  		        (progn (funcall mover (- x)) (point))))))


> +	 (pos1 (save-excursion (funcall aux arg)))
> +         pos2)
>      (cond
> +     ((and (consp (car pos1)) (consp (cdr pos1)))
> +      (transpose-subr-1 (car pos1) (cdr pos1)))
>       ((= arg 0)
>        (save-excursion
>  	(setq pos1 (funcall aux 1))
> diff --git a/lisp/treesit.el b/lisp/treesit.el
> index cefbed1a16..9f0965ac68 100644
> --- a/lisp/treesit.el
> +++ b/lisp/treesit.el
> @@ -1582,6 +1582,27 @@ treesit-search-forward-goto
>        (goto-char current-pos)))
>      node))
>  
> +(defun treesit-transpose-sexps (&optional arg)
> +  "Tree-sitter `transpose-sexps' function.
> +Arg is the same as in `transpose-sexps'.
> +
> +Return a pair of positions describing the regions to transpose
> +for use in `transpose-subr' and friends."
> +  (let* ((parent (treesit-node-parent (treesit-node-at (point))))
> +         (child (treesit-node-child parent 0 t)))
> +    (named-let loop ((prev child)
> +                     (next (treesit-node-child
> +                            parent (+ arg (treesit-node-index child t))
> +                            t)))
> +      (if (< (point) (or (treesit-node-end next)
> +                         (user-error "Don't have two things to transpose")))
> +          (cons (cons (treesit-node-start prev)
> +                      (treesit-node-end prev))
> +                (cons (treesit-node-start next)
> +                      (treesit-node-end next)))
> +        (loop (treesit-node-next-sibling prev t)
> +              (treesit-node-next-sibling next t))))))
> +
>  ;;; Navigation, defun, things
>  ;;
>  ;; Emacs lets you define "things" by a regexp that matches the type of
> @@ -2111,7 +2132,8 @@ treesit-major-mode-setup
>    ;; Defun name.
>    (when treesit-defun-name-function
>      (setq-local add-log-current-defun-function
> -                #'treesit-add-log-current-defun)))
> +                #'treesit-add-log-current-defun))
> +  (setq-local transpose-sexps-function #'treesit-transpose-sexps))
>  
>  ;;; Debugging




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-26 22:46                                               ` Stefan Monnier
@ 2022-12-26 22:51                                                 ` Stefan Monnier
  2022-12-27 22:15                                                   ` Theodor Thornhill via Emacs development discussions.
  2022-12-26 22:56                                                 ` Theodor Thornhill
  1 sibling, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-26 22:51 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

>> +  (let* ((aux (if special mover
>> +	        (lambda (x)
>> +		  (cons (progn (funcall mover x) (point))
>> +		        (progn (funcall mover (- x)) (point))))))
>
> If `mover` is changed to return a pair of positions, than the above can
> just be:

[ ... and here I hit `C-c C-c` i.s.o `C-x C-x`, sorry.  ]

    (let* ((aux (if special mover
                  (lambda (x)
      	            (let ((pos (progn (funcall mover x) (point))))
      	              (if (consp pos) pos
                        (cons pos
    	                      (progn (funcall mover (- x)) (point))))))))

-- Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-26 22:46                                               ` Stefan Monnier
  2022-12-26 22:51                                                 ` Stefan Monnier
@ 2022-12-26 22:56                                                 ` Theodor Thornhill
  1 sibling, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-26 22:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +(defvar-local transpose-sexps-function nil
>> +  "If non-nil, `transpose-sexps' delegates to this function.
>> +
>> +The return value of this function is expected to be a cons of two
>> +conses, denoting the positions in the current buffer to be
>> +transposed.  If no such pair of positions is available, signal
>> +USER-ERROR.")
>
> This docstring needs to tell what args are passed to the function.
>
> I see you make it return a pair of pairs, so it has to handle all the
> semantics of `transpose-sexps`.  My intuition told me to go with
> a function that returns a pair of positions (i.e. it takes an ARG and
> returns the BEG..END of the ARGth sibling).  I suspect it would fit
> within `transpose-subr` a bit better.
>

Yeah, I could do that.  It may be simpler than to try to surgically add
another behavior inside that function.


>> +     (if transpose-sexps-function transpose-sexps-function
>> +       (lambda (arg)
>
> Aka (or transpose-sexps-function (lambda (arg) ...))
> But even better is to put the `lambda` in the default value of the
> variable, so you just use `transpose-sexps-function` unconditionally.
>

Yeah, I actually did that in an experiment earlier, but decided against
it in favor of smaller changes. I'll make that change now, though.

>> +  (let* ((aux (if special mover
>> +	        (lambda (x)
>> +		  (cons (progn (funcall mover x) (point))
>> +		        (progn (funcall mover (- x)) (point))))))
>
> If `mover` is changed to return a pair of positions, than the above can
> just be:
>
>> +  (let* ((aux (if special mover
>> +	        (lambda (x)
>> +		  (cons (progn (funcall mover x) (point))
>> +  		        (progn (funcall mover (- x)) (point))))))
>
>

I'll try to make it work without changing the protocol.  I'll add the
patch to the bugreport, and let's continue this particular discussion
there? :-)

Theo



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-15 19:56                                 ` Stefan Monnier
  2022-12-15 20:03                                   ` Theodor Thornhill
@ 2022-12-27 15:46                                   ` Lynn Winebarger
  1 sibling, 0 replies; 54+ messages in thread
From: Lynn Winebarger @ 2022-12-27 15:46 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Theodor Thornhill, Yuan Fu, emacs-devel, eliz

On Thu, Dec 15, 2022 at 2:57 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > If this code is plugged into transpose-sexps we get this nice behavior:
>
> It's a bit different from what SMIE would do, but there's a lot of
> overlap and when it's different it's arguably better, so sounds good
> to me.
>
> > Now forward/backward-sexp can actually work a little differently, as you
> > suggest, or we can let it use the same "move over siblings"-semantic.
> > In that case we don't even need the treesit-sexp-type-regexp variables to
> > control this, I think.
> >
> > What do you think?
>
> I'm not sufficiently familiar with the tree-sitter tree to foresee
> precisely how it would affect `forward/backward-sexp`, but I think you
> have a good enough understanding to make a good judgment at this
> point :-)

A while ago, I created a LALR(1) grammar with "generic" symbols that
encapsulated essentially all "standard" syntactic constructs I've seen
in programming languages:  top-level forms, statements, blocks,
sequences, constructors, keyword forms (including ones with op,
l-values, r-values, prefix/postfix/infix & unary/binary operators,
calls, destructuring l-values, quoted data, interpolating quotation
and other interpolating forms, etc.  It was pretty neat.  If I can
ensure it (or some equivalent grammar) is unencumbered I'd like to
document it and extend emacs's syntax tables to be able to analyze a
buffer using these constructs instead of just s-expressions, with
modes just providing a map of language-specific tokens to the generic
symbols.

Lynn



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

* Re: Plug treesit.el into other emacs constructs
  2022-12-26 22:51                                                 ` Stefan Monnier
@ 2022-12-27 22:15                                                   ` Theodor Thornhill via Emacs development discussions.
  2022-12-28  0:12                                                     ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill via Emacs development discussions. @ 2022-12-27 22:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> +  (let* ((aux (if special mover
>>> +	        (lambda (x)
>>> +		  (cons (progn (funcall mover x) (point))
>>> +		        (progn (funcall mover (- x)) (point))))))
>>
>> If `mover` is changed to return a pair of positions, than the above can
>> just be:
>
> [ ... and here I hit `C-c C-c` i.s.o `C-x C-x`, sorry.  ]
>
>     (let* ((aux (if special mover
>                   (lambda (x)
>       	            (let ((pos (progn (funcall mover x) (point))))
>       	              (if (consp pos) pos
>                         (cons pos
>     	                      (progn (funcall mover (- x)) (point))))))))
>

I think I managed to keep the semantics as they are now.  What do you
think?  Does this seem like a sane approach?

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-treesit-transpose-sexps-bug-60128.patch --]
[-- Type: text/x-diff, Size: 7690 bytes --]

From 805199c03d6cfee4bec828295a6105d51b96867f Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sun, 25 Dec 2022 20:11:59 +0100
Subject: [PATCH] Add treesit-transpose-sexps (bug#60128)

We don't really need to rely on forward-sexp to define what to
transpose.  In tree-sitter we can consider siblings as "balanced
expressions", and swap them without doing any movement to calculate
where the siblings in question are.

* lisp/simple.el (transpose-sexps-function): New defvar-local.
(transpose-sexps): Use the new defvar-local if available.
(transpose-subr): Check whether the mover function returns a cons of
conses, then run transpose-subr-1 on the position-pairs.
* lisp/treesit.el (treesit-transpose-sexps): New function.
---
 etc/NEWS        |  9 ++++++
 lisp/simple.el  | 73 +++++++++++++++++++++++++++----------------------
 lisp/treesit.el | 29 +++++++++++++++++++-
 3 files changed, 78 insertions(+), 33 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d17e1f1f89..5997d7a19c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -44,6 +44,15 @@ example, as part of preview for iconified frames.
 \f
 * Editing Changes in Emacs 30.1
 
+** New helper 'transpose-sexps-function'
+Emacs now can set this defvar buffer-locally to customize the behavior
+of the 'transpose-sexps' function.
+
+** New function 'treesit-trnspose-sexps'
+treesit.el now unconditionally sets 'transpose-sexps-function' for all
+Tree-sitter modes.  This functionality utilizes the new
+'transpose-sexps-function'.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 30.1
 ---
diff --git a/lisp/simple.el b/lisp/simple.el
index 4551b749d5..03d3fda451 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8438,6 +8438,44 @@ transpose-words
   (interactive "*p")
   (transpose-subr 'forward-word arg))
 
+(defvar-local transpose-sexps-function
+    (lambda (arg)
+      ;; Here we should try to simulate the behavior of
+      ;; (cons (progn (forward-sexp x) (point))
+      ;;       (progn (forward-sexp (- x)) (point)))
+      ;; Except that we don't want to rely on the second forward-sexp
+      ;; putting us back to where we want to be, since forward-sexp-function
+      ;; might do funny things like infix-precedence.
+      (if (if (> arg 0)
+	      (looking-at "\\sw\\|\\s_")
+	    (and (not (bobp))
+	         (save-excursion
+                   (forward-char -1)
+                   (looking-at "\\sw\\|\\s_"))))
+          ;; Jumping over a symbol.  We might be inside it, mind you.
+	  (progn (funcall (if (> arg 0)
+			      #'skip-syntax-backward #'skip-syntax-forward)
+			  "w_")
+	         (cons (save-excursion (forward-sexp arg) (point)) (point)))
+        ;; Otherwise, we're between sexps.  Take a step back before jumping
+        ;; to make sure we'll obey the same precedence no matter which
+        ;; direction we're going.
+        (funcall (if (> arg 0) #'skip-syntax-backward #'skip-syntax-forward)
+                 " .")
+        (cons (save-excursion (forward-sexp arg) (point))
+	      (progn (while (or (forward-comment (if (> arg 0) 1 -1))
+			        (not (zerop (funcall (if (> arg 0)
+						         #'skip-syntax-forward
+						       #'skip-syntax-backward)
+						     ".")))))
+		     (point)))))
+  "If non-nil, `transpose-sexps' delegates to this function.
+
+This function takes one argument ARG, a number as provided
+through running `transpose-sexps'.  Its expected return value is
+a position pair, which is a cons (BEG . END), where BEG and END
+are buffer positions.")
+
 (defun transpose-sexps (arg &optional interactive)
   "Like \\[transpose-chars] (`transpose-chars'), but applies to sexps.
 Unlike `transpose-words', point must be between the two sexps and not
@@ -8453,38 +8491,7 @@ transpose-sexps
       (condition-case nil
           (transpose-sexps arg nil)
         (scan-error (user-error "Not between two complete sexps")))
-    (transpose-subr
-     (lambda (arg)
-       ;; Here we should try to simulate the behavior of
-       ;; (cons (progn (forward-sexp x) (point))
-       ;;       (progn (forward-sexp (- x)) (point)))
-       ;; Except that we don't want to rely on the second forward-sexp
-       ;; putting us back to where we want to be, since forward-sexp-function
-       ;; might do funny things like infix-precedence.
-       (if (if (> arg 0)
-	       (looking-at "\\sw\\|\\s_")
-	     (and (not (bobp))
-		  (save-excursion
-                    (forward-char -1)
-                    (looking-at "\\sw\\|\\s_"))))
-	   ;; Jumping over a symbol.  We might be inside it, mind you.
-	   (progn (funcall (if (> arg 0)
-			       'skip-syntax-backward 'skip-syntax-forward)
-			   "w_")
-		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
-         ;; Otherwise, we're between sexps.  Take a step back before jumping
-         ;; to make sure we'll obey the same precedence no matter which
-         ;; direction we're going.
-         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
-                  " .")
-         (cons (save-excursion (forward-sexp arg) (point))
-	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
-			         (not (zerop (funcall (if (> arg 0)
-							  'skip-syntax-forward
-						        'skip-syntax-backward)
-						      ".")))))
-		      (point)))))
-     arg 'special)))
+    (transpose-subr transpose-sexps-function arg 'special)))
 
 (defun transpose-lines (arg)
   "Exchange current line and previous line, leaving point after both.
@@ -8542,6 +8549,8 @@ transpose-subr
       (goto-char (+ (car pos2) (- (cdr pos1) (car pos1))))))))
 
 (defun transpose-subr-1 (pos1 pos2)
+  (unless (and pos1 pos2)
+    (error "Don't have two things to transpose"))
   (when (> (car pos1) (cdr pos1)) (setq pos1 (cons (cdr pos1) (car pos1))))
   (when (> (car pos2) (cdr pos2)) (setq pos2 (cons (cdr pos2) (car pos2))))
   (when (> (car pos1) (car pos2))
diff --git a/lisp/treesit.el b/lisp/treesit.el
index cefbed1a16..203a724fe7 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1582,6 +1582,32 @@ treesit-search-forward-goto
       (goto-char current-pos)))
     node))
 
+(defun treesit-transpose-sexps (&optional arg)
+  "Tree-sitter `transpose-sexps' function.
+Arg is the same as in `transpose-sexps'.
+
+Locate the node closest to POINT, and transpose that node with
+its sibling node ARG nodes away.
+
+Return a pair of positions as described by
+`transpose-sexps-function' for use in `transpose-subr' and
+friends."
+  (let* ((parent (treesit-node-parent (treesit-node-at (point))))
+         (child (treesit-node-child parent 0 t)))
+    (named-let loop ((prev child)
+                     (next (treesit-node-next-sibling child t)))
+      (when (and prev next)
+        (if (< (point) (treesit-node-end next))
+            (if (= arg -1)
+                (cons (treesit-node-start prev)
+                      (treesit-node-end prev))
+              (when-let ((n (treesit-node-child
+                             parent (+ arg (treesit-node-index prev t)) t)))
+                (cons (treesit-node-end n)
+                      (treesit-node-start n))))
+          (loop (treesit-node-next-sibling prev t)
+                (treesit-node-next-sibling next t)))))))
+
 ;;; Navigation, defun, things
 ;;
 ;; Emacs lets you define "things" by a regexp that matches the type of
@@ -2111,7 +2137,8 @@ treesit-major-mode-setup
   ;; Defun name.
   (when treesit-defun-name-function
     (setq-local add-log-current-defun-function
-                #'treesit-add-log-current-defun)))
+                #'treesit-add-log-current-defun))
+  (setq-local transpose-sexps-function #'treesit-transpose-sexps))
 
 ;;; Debugging
 
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-27 22:15                                                   ` Theodor Thornhill via Emacs development discussions.
@ 2022-12-28  0:12                                                     ` Stefan Monnier
  2022-12-28  9:26                                                       ` Theodor Thornhill via Emacs development discussions.
  0 siblings, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-28  0:12 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> I think I managed to keep the semantics as they are now.  What do you
> think?  Does this seem like a sane approach?

Yes, LGTM.  See nitpicks below.

> +** New function 'treesit-trnspose-sexps'
                             ^^
                             a

> +(defvar-local transpose-sexps-function

I'd keep it a plain `defvar`.

> +  "If non-nil, `transpose-sexps' delegates to this function.
> +
> +This function takes one argument ARG, a number as provided
> +through running `transpose-sexps'.  Its expected return value is
> +a position pair, which is a cons (BEG . END), where BEG and END
> +are buffer positions.")

The ARG is not quite the same as the one passed to `transpose-sexps`.
I think we should say something like ".. ARG, a number.  Its expected
return value is a pair of positions (BEG . END) delimiting the ARGth
sibling".

The rest looks great, thanks.


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-28  0:12                                                     ` Stefan Monnier
@ 2022-12-28  9:26                                                       ` Theodor Thornhill via Emacs development discussions.
  2022-12-28 18:01                                                         ` Stefan Monnier
  0 siblings, 1 reply; 54+ messages in thread
From: Theodor Thornhill via Emacs development discussions. @ 2022-12-28  9:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I think I managed to keep the semantics as they are now.  What do you
>> think?  Does this seem like a sane approach?
>
> Yes, LGTM.  See nitpicks below.
>
>> +** New function 'treesit-trnspose-sexps'
>                              ^^
>                              a
>
>> +(defvar-local transpose-sexps-function
>
> I'd keep it a plain `defvar`.
>
>> +  "If non-nil, `transpose-sexps' delegates to this function.
>> +
>> +This function takes one argument ARG, a number as provided
>> +through running `transpose-sexps'.  Its expected return value is
>> +a position pair, which is a cons (BEG . END), where BEG and END
>> +are buffer positions.")
>
> The ARG is not quite the same as the one passed to `transpose-sexps`.
> I think we should say something like ".. ARG, a number.  Its expected
> return value is a pair of positions (BEG . END) delimiting the ARGth
> sibling".
>
> The rest looks great, thanks.
>
>

Something like this?

If you're ok with this, maybe you can install for me, as I can't? Then I
can close the relevant bugreport.


Oh, and one more thing.  There is a bug in in 'transpose-subr' in the
case where (> arg 0).  Transpose-subr-1 makes an effort to swap the
conses around so that we have ranges that makes sense.  However, the
case in question then unconditionally jumps to (car pos2), which would
be the wrong position.  Either we need to change that, or add something
like this to the docstring for the MOVER function:

```
The MOVER function is expected to return its conses in different
orders.  if `forward-word' is used for the mover function the
conses will be like this, where the number is the order the
position is calculated ('|' is where point is initially):

                       first|  second
                       ^    ^  ^     ^
                      (2 .  1) (4  . 3)
```

What do you think?  I'll open a bug report for it if the docstring isn't
enough.

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-treesit-transpose-sexps-bug-60128.patch --]
[-- Type: text/x-diff, Size: 8722 bytes --]

From c773c854c1648786cc26cd2ade734ebce016d66a Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sun, 25 Dec 2022 20:11:59 +0100
Subject: [PATCH] Add treesit-transpose-sexps (bug#60128)

We don't really need to rely on forward-sexp to define what to
transpose.  In tree-sitter we can consider siblings as "balanced
expressions", and swap them without doing any movement to calculate
where the siblings in question are.

* lisp/simple.el (transpose-sexps-function): New defvar-local.
(transpose-sexps): Use the new defvar-local if available.
(transpose-subr): Check whether the mover function returns a cons of
conses, then run transpose-subr-1 on the position-pairs.
* lisp/treesit.el (treesit-transpose-sexps): New function.
---
 etc/NEWS        |  9 +++++
 lisp/simple.el  | 88 +++++++++++++++++++++++++++----------------------
 lisp/treesit.el | 29 +++++++++++++++-
 3 files changed, 86 insertions(+), 40 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d17e1f1f89..83aa81eb4b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -44,6 +44,15 @@ example, as part of preview for iconified frames.
 \f
 * Editing Changes in Emacs 30.1
 
+** New helper 'transpose-sexps-function'
+Emacs now can set this defvar to customize the behavior of the
+'transpose-sexps' function.
+
+** New function 'treesit-transpose-sexps'
+treesit.el now unconditionally sets 'transpose-sexps-function' for all
+Tree-sitter modes.  This functionality utilizes the new
+'transpose-sexps-function'.
+
 \f
 * Changes in Specialized Modes and Packages in Emacs 30.1
 ---
diff --git a/lisp/simple.el b/lisp/simple.el
index 4551b749d5..cf0845853a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8438,6 +8438,43 @@ transpose-words
   (interactive "*p")
   (transpose-subr 'forward-word arg))
 
+(defvar transpose-sexps-function
+  (lambda (arg)
+    ;; Here we should try to simulate the behavior of
+    ;; (cons (progn (forward-sexp x) (point))
+    ;;       (progn (forward-sexp (- x)) (point)))
+    ;; Except that we don't want to rely on the second forward-sexp
+    ;; putting us back to where we want to be, since forward-sexp-function
+    ;; might do funny things like infix-precedence.
+    (if (if (> arg 0)
+	    (looking-at "\\sw\\|\\s_")
+	  (and (not (bobp))
+	       (save-excursion
+                 (forward-char -1)
+                 (looking-at "\\sw\\|\\s_"))))
+        ;; Jumping over a symbol.  We might be inside it, mind you.
+	(progn (funcall (if (> arg 0)
+			    #'skip-syntax-backward #'skip-syntax-forward)
+			"w_")
+	       (cons (save-excursion (forward-sexp arg) (point)) (point)))
+      ;; Otherwise, we're between sexps.  Take a step back before jumping
+      ;; to make sure we'll obey the same precedence no matter which
+      ;; direction we're going.
+      (funcall (if (> arg 0) #'skip-syntax-backward #'skip-syntax-forward)
+               " .")
+      (cons (save-excursion (forward-sexp arg) (point))
+	    (progn (while (or (forward-comment (if (> arg 0) 1 -1))
+			      (not (zerop (funcall (if (> arg 0)
+						       #'skip-syntax-forward
+						     #'skip-syntax-backward)
+						   ".")))))
+		   (point)))))
+  "If non-nil, `transpose-sexps' delegates to this function.
+
+This function takes one argument ARG, a number.  Its expected
+return value is a position pair, which is a cons (BEG . END),
+where BEG and END are buffer positions.")
+
 (defun transpose-sexps (arg &optional interactive)
   "Like \\[transpose-chars] (`transpose-chars'), but applies to sexps.
 Unlike `transpose-words', point must be between the two sexps and not
@@ -8453,38 +8490,7 @@ transpose-sexps
       (condition-case nil
           (transpose-sexps arg nil)
         (scan-error (user-error "Not between two complete sexps")))
-    (transpose-subr
-     (lambda (arg)
-       ;; Here we should try to simulate the behavior of
-       ;; (cons (progn (forward-sexp x) (point))
-       ;;       (progn (forward-sexp (- x)) (point)))
-       ;; Except that we don't want to rely on the second forward-sexp
-       ;; putting us back to where we want to be, since forward-sexp-function
-       ;; might do funny things like infix-precedence.
-       (if (if (> arg 0)
-	       (looking-at "\\sw\\|\\s_")
-	     (and (not (bobp))
-		  (save-excursion
-                    (forward-char -1)
-                    (looking-at "\\sw\\|\\s_"))))
-	   ;; Jumping over a symbol.  We might be inside it, mind you.
-	   (progn (funcall (if (> arg 0)
-			       'skip-syntax-backward 'skip-syntax-forward)
-			   "w_")
-		  (cons (save-excursion (forward-sexp arg) (point)) (point)))
-         ;; Otherwise, we're between sexps.  Take a step back before jumping
-         ;; to make sure we'll obey the same precedence no matter which
-         ;; direction we're going.
-         (funcall (if (> arg 0) 'skip-syntax-backward 'skip-syntax-forward)
-                  " .")
-         (cons (save-excursion (forward-sexp arg) (point))
-	       (progn (while (or (forward-comment (if (> arg 0) 1 -1))
-			         (not (zerop (funcall (if (> arg 0)
-							  'skip-syntax-forward
-						        'skip-syntax-backward)
-						      ".")))))
-		      (point)))))
-     arg 'special)))
+    (transpose-subr transpose-sexps-function arg 'special)))
 
 (defun transpose-lines (arg)
   "Exchange current line and previous line, leaving point after both.
@@ -8509,13 +8515,15 @@ transpose-lines
 ;; FIXME document SPECIAL.
 (defun transpose-subr (mover arg &optional special)
   "Subroutine to do the work of transposing objects.
-Works for lines, sentences, paragraphs, etc.  MOVER is a function that
-moves forward by units of the given object (e.g. `forward-sentence',
-`forward-paragraph').  If ARG is zero, exchanges the current object
-with the one containing mark.  If ARG is an integer, moves the
-current object past ARG following (if ARG is positive) or
-preceding (if ARG is negative) objects, leaving point after the
-current object."
+Works for lines, sentences, paragraphs, etc.  MOVER is a function
+that moves forward by units of the given
+object (e.g. `forward-sentence', `forward-paragraph'), or a
+function calculating a cons of buffer positions.
+
+  If ARG is zero, exchanges the current object with the one
+containing mark.  If ARG is an integer, moves the current object
+past ARG following (if ARG is positive) or preceding (if ARG is
+negative) objects, leaving point after the current object."
   (let ((aux (if special mover
 	       (lambda (x)
 		 (cons (progn (funcall mover x) (point))
@@ -8542,6 +8550,8 @@ transpose-subr
       (goto-char (+ (car pos2) (- (cdr pos1) (car pos1))))))))
 
 (defun transpose-subr-1 (pos1 pos2)
+  (unless (and pos1 pos2)
+    (error "Don't have two things to transpose"))
   (when (> (car pos1) (cdr pos1)) (setq pos1 (cons (cdr pos1) (car pos1))))
   (when (> (car pos2) (cdr pos2)) (setq pos2 (cons (cdr pos2) (car pos2))))
   (when (> (car pos1) (car pos2))
diff --git a/lisp/treesit.el b/lisp/treesit.el
index cefbed1a16..203a724fe7 100644
--- a/lisp/treesit.el
+++ b/lisp/treesit.el
@@ -1582,6 +1582,32 @@ treesit-search-forward-goto
       (goto-char current-pos)))
     node))
 
+(defun treesit-transpose-sexps (&optional arg)
+  "Tree-sitter `transpose-sexps' function.
+Arg is the same as in `transpose-sexps'.
+
+Locate the node closest to POINT, and transpose that node with
+its sibling node ARG nodes away.
+
+Return a pair of positions as described by
+`transpose-sexps-function' for use in `transpose-subr' and
+friends."
+  (let* ((parent (treesit-node-parent (treesit-node-at (point))))
+         (child (treesit-node-child parent 0 t)))
+    (named-let loop ((prev child)
+                     (next (treesit-node-next-sibling child t)))
+      (when (and prev next)
+        (if (< (point) (treesit-node-end next))
+            (if (= arg -1)
+                (cons (treesit-node-start prev)
+                      (treesit-node-end prev))
+              (when-let ((n (treesit-node-child
+                             parent (+ arg (treesit-node-index prev t)) t)))
+                (cons (treesit-node-end n)
+                      (treesit-node-start n))))
+          (loop (treesit-node-next-sibling prev t)
+                (treesit-node-next-sibling next t)))))))
+
 ;;; Navigation, defun, things
 ;;
 ;; Emacs lets you define "things" by a regexp that matches the type of
@@ -2111,7 +2137,8 @@ treesit-major-mode-setup
   ;; Defun name.
   (when treesit-defun-name-function
     (setq-local add-log-current-defun-function
-                #'treesit-add-log-current-defun)))
+                #'treesit-add-log-current-defun))
+  (setq-local transpose-sexps-function #'treesit-transpose-sexps))
 
 ;;; Debugging
 
-- 
2.34.1


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

* Re: Plug treesit.el into other emacs constructs
  2022-12-28  9:26                                                       ` Theodor Thornhill via Emacs development discussions.
@ 2022-12-28 18:01                                                         ` Stefan Monnier
  2022-12-28 18:27                                                           ` Theodor Thornhill
  0 siblings, 1 reply; 54+ messages in thread
From: Stefan Monnier @ 2022-12-28 18:01 UTC (permalink / raw)
  To: Theodor Thornhill; +Cc: Yuan Fu, emacs-devel, eliz

> Something like this?
>
> If you're ok with this, maybe you can install for me, as I can't? Then I
> can close the relevant bugreport.

Sorry, I forgot that you don't have write access yet.
Pushed, thanks,


        Stefan




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

* Re: Plug treesit.el into other emacs constructs
  2022-12-28 18:01                                                         ` Stefan Monnier
@ 2022-12-28 18:27                                                           ` Theodor Thornhill
  0 siblings, 0 replies; 54+ messages in thread
From: Theodor Thornhill @ 2022-12-28 18:27 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Yuan Fu, emacs-devel, eliz



On 28 December 2022 19:01:23 CET, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> Something like this?
>>
>> If you're ok with this, maybe you can install for me, as I can't? Then I
>> can close the relevant bugreport.
>
>Sorry, I forgot that you don't have write access yet.
>Pushed, thanks,
>
>
>        Stefan
>

No worries, and thanks for all the feedback!

Theo



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

end of thread, other threads:[~2022-12-28 18:27 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-12 14:33 Plug treesit.el into other emacs constructs Theodor Thornhill
2022-12-12 14:45 ` Eli Zaretskii
2022-12-13 18:17   ` Theodor Thornhill
2022-12-12 15:46 ` Stefan Monnier
2022-12-13 18:27   ` Theodor Thornhill
2022-12-13 19:37     ` Stefan Monnier
2022-12-13 19:53       ` Yuan Fu
2022-12-13 20:06         ` Perry Smith
2022-12-13 23:19         ` Stefan Monnier
2022-12-14  8:14           ` Yuan Fu
2022-12-14  8:42             ` Theodor Thornhill
2022-12-14 14:01             ` Stefan Monnier
2022-12-14 16:24               ` Theodor Thornhill
2022-12-14 17:46                 ` Stefan Monnier
2022-12-14 18:07                   ` Theodor Thornhill
2022-12-14 19:25                     ` Stefan Monnier
2022-12-14 19:35                       ` Stefan Monnier
2022-12-14 20:04                       ` Theodor Thornhill
2022-12-14 20:50                         ` Stefan Monnier
2022-12-14 21:15                           ` Theodor Thornhill
2022-12-14 21:34                             ` Stefan Monnier
2022-12-15 19:37                               ` Theodor Thornhill
2022-12-15 19:56                                 ` Stefan Monnier
2022-12-15 20:03                                   ` Theodor Thornhill
2022-12-15 20:33                                     ` Theodor Thornhill
2022-12-15 20:57                                       ` Theodor Thornhill
2022-12-24  7:00                                         ` Eli Zaretskii
2022-12-24  8:44                                           ` Yuan Fu
2022-12-24 14:01                                         ` Stefan Monnier
2022-12-24 14:15                                           ` Theodor Thornhill
2022-12-26 19:11                                             ` Theodor Thornhill
2022-12-26 22:46                                               ` Stefan Monnier
2022-12-26 22:51                                                 ` Stefan Monnier
2022-12-27 22:15                                                   ` Theodor Thornhill via Emacs development discussions.
2022-12-28  0:12                                                     ` Stefan Monnier
2022-12-28  9:26                                                       ` Theodor Thornhill via Emacs development discussions.
2022-12-28 18:01                                                         ` Stefan Monnier
2022-12-28 18:27                                                           ` Theodor Thornhill
2022-12-26 22:56                                                 ` Theodor Thornhill
2022-12-27 15:46                                   ` Lynn Winebarger
2022-12-14 23:31               ` Yuan Fu
2022-12-15  0:05                 ` Yuan Fu
2022-12-15  7:09                   ` Eli Zaretskii
2022-12-15  7:14                     ` Theodor Thornhill
2022-12-15  4:37                 ` Stefan Monnier
2022-12-15  5:59                 ` Theodor Thornhill
2022-12-15 21:23                   ` Yuan Fu
2022-12-15 21:28                     ` Theodor Thornhill
2022-12-13 20:02       ` Theodor Thornhill
2022-12-13 23:10         ` Stefan Monnier
2022-12-14 23:32   ` Stephen Leake
2022-12-16 10:02     ` Kévin Le Gouguec
2022-12-16 11:54       ` [SPAM UNSURE] " Stephen Leake
2022-12-17 15:30         ` Kévin Le Gouguec

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