GNU bug report logs - #61617
M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions

Previous Next

To reply to this bug, email your comments to 61617 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Evgeni Kolev <evgenysw <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: M-x mark-defun doesn't work correctly in tree-sitter modes when
 comments exist between functions
Date: Sun, 19 Feb 2023 10:39:07 +0200

M-x mark-defun doesn't work correctly when the function at point is
preceded by a comment.

I've noticed this bug in go-ts-mode, and have also checked it's
reproduced in rust-ts-mode:

To reproduce:
1. with the following code in go-ts-mode, "|" is the point (in function sum2)
```
package main

func sum(a, b int) int {
    return a + b
}

// comment
func sum2(a, b int) int {
    Ireturn a + b
}
```
2. Execute M-x mark-defun
3. The region selected is wrong - the empty line between the two
functions is marked. I expect only function sum2 and the preceding
comment to be marked.

Observations:
- The issue is reproducible with a fresh build of master, and
reproducible with emacs -Q.
- The issue is also reproducible in rust-ts-mode which makes me think
the root cause is in treesit code (treesit.el maybe).
- From what I've checked, (beginning-of-defun-comments) works
correctly, but (end-of-defun) doesn't. These two (and other) functions
are used by mark-defun.
- The issue does not exist when the function is not preceded by a
comment. In other words - the issue is not reproduced when no comments
exist between the functions.

Below is a Rust example which reproduces the issue, again "|" is the
point (in function main2). Steps to reproduce are identical to the
steps for go-ts-mode:
```
fn main() {
    println!("Hello World!");
}

// comment
fn main2() {
    |println!("Hello World!");
}
```

Message #8 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Evgeni Kolev <evgenysw <at> gmail.com>
To: 61617 <at> debbugs.gnu.org
Subject: M-x mark-defun doesn't work correctly in tree-sitter modes when
 comments exist between functions
Date: Mon, 20 Feb 2023 10:30:30 +0200

Sorry, I have a typo in my Go example, the point in the code is "I"
(capital i), instead of "|" (pipe). The Rust example is OK.

Fixed steps to reproduce for go-ts-mode:

To reproduce:
1. with the following code in go-ts-mode, "|" is the point (in function sum2)
```
package main

func sum(a, b int) int {
    return a + b
}

// comment
func sum2(a, b int) int {
    |return a + b
}
```
2. Execute M-x mark-defun
3. The region selected is wrong - the empty line between the two
functions is marked. I expect only function sum2 and the preceding
comment to be marked.

Message #11 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: evgenysw <at> gmail.com
Cc: Theodor Thornhill <theo <at> thornhill.no>, 61617 <at> debbugs.gnu.org
Subject: Re: bug#61617: M-x mark-defun doesn't work correctly in tree-sitter 
 modes when comments exist between functions
Date: Fri, 24 Feb 2023 21:08:29 -0800

Evgeni Kolev <evgenysw <at> gmail.com> writes:

> Sorry, I have a typo in my Go example, the point in the code is "I"
> (capital i), instead of "|" (pipe). The Rust example is OK.
>
> Fixed steps to reproduce for go-ts-mode:
>
> To reproduce:
> 1. with the following code in go-ts-mode, "|" is the point (in function sum2)
> ```
> package main
>
> func sum(a, b int) int {
>     return a + b
> }
>
> // comment
> func sum2(a, b int) int {
>     |return a + b
> }
> ```
> 2. Execute M-x mark-defun
> 3. The region selected is wrong - the empty line between the two
> functions is marked. I expect only function sum2 and the preceding
> comment to be marked.

Huh, with or without comments, mark-defun always includes the empty
lines before the defun for me. I get the same behavior in rust-ts-mode.
This seems intentional, because this is at the end of the definition of
mark-defun:


(skip-chars-backward "[:space:]\n")
(unless (bobp)
  (forward-line 1))

Are you using emacs-29 or emacs-30? Theo might have changed something on master.

Yuan

Message #14 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Evgeni Kolev <evgenysw <at> gmail.com>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Theodor Thornhill <theo <at> thornhill.no>, 61617 <at> debbugs.gnu.org
Subject: Re: bug#61617: M-x mark-defun doesn't work correctly in tree-sitter
 modes when comments exist between functions
Date: Sat, 25 Feb 2023 09:27:17 +0200

[Message part 1 (text/plain, inline)]

On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri <at> gmail.com> wrote:

> Huh, with or without comments, mark-defun always includes the empty
> lines before the defun for me. I get the same behavior in rust-ts-mode.
> This seems intentional, because this is at the end of the definition of
> mark-defun:
>
> (skip-chars-backward "[:space:]\n")
> (unless (bobp)
>   (forward-line 1))

Did you check with my example or another example? With my example, the
issue I get is that only the empty lines are marked, without the
defun, without the comments. I'm attaching two screenshots - before
and after mark-defun.

Note: I get the correct behaviour when there is just one defun in the
file. But if there are more, or the defun at point is not at the top
of the file - mark-defun does not work as expected.

>
> Are you using emacs-29 or emacs-30? Theo might have changed something on master.

I've observed the issue on both. If you can't reproduce it - I'll
re-test on a fresh docker image to make sure the issue is not in my
setup. However, I'm pretty sure it's not in my setup because I used a
freshly built emacs just for this purpose (emacs-30 maybe, I'm not
sure), and ran it with emacs -Q.

[before-mark-defun.png (image/png, attachment)]

[after-mark-defun.png (image/png, attachment)]

Message #17 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Evgeni Kolev <evgenysw <at> gmail.com>
Cc: Theodor Thornhill <theo <at> thornhill.no>, 61617 <at> debbugs.gnu.org
Subject: Re: bug#61617: M-x mark-defun doesn't work correctly in tree-sitter
 modes when comments exist between functions
Date: Sun, 26 Feb 2023 16:42:37 -0800

> On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw <at> gmail.com> wrote:
> 
> On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri <at> gmail.com> wrote:
> 
>> Huh, with or without comments, mark-defun always includes the empty
>> lines before the defun for me. I get the same behavior in rust-ts-mode.
>> This seems intentional, because this is at the end of the definition of
>> mark-defun:
>> 
>> (skip-chars-backward "[:space:]\n")
>> (unless (bobp)
>>  (forward-line 1))
> 
> Did you check with my example or another example? With my example, the
> issue I get is that only the empty lines are marked, without the
> defun, without the comments. I'm attaching two screenshots - before
> and after mark-defun.
> 
> Note: I get the correct behaviour when there is just one defun in the
> file. But if there are more, or the defun at point is not at the top
> of the file - mark-defun does not work as expected.

Sorry, I used wrong go mode when testing your recipe. I used go-mode thinking it is go-ts-mode :-( Using go-ts-mode, I can reproduce what you see, yes.

The problem you discovered revealed some shortcoming in our tree-sitter navigation function, and requires quite a few non-trivial changes, I working on fixing it.

The direct cause is that (treesit-beginning-of-defun -1) is supposed to go to the beginning of next defun, but instead goes to the beginning of next next defun. That was a design decision I made but I can see it causing old functions to misbehave. 

Yuan

Message #20 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Evgeni Kolev <evgenysw <at> gmail.com>
Cc: Theodor Thornhill <theo <at> thornhill.no>, 61617 <at> debbugs.gnu.org
Subject: Re: bug#61617: M-x mark-defun doesn't work correctly in tree-sitter
 modes when comments exist between functions
Date: Mon, 27 Feb 2023 01:24:42 -0800

> On Feb 26, 2023, at 4:42 PM, Yuan Fu <casouri <at> gmail.com> wrote:
> 
> 
> 
>> On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw <at> gmail.com> wrote:
>> 
>> On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri <at> gmail.com> wrote:
>> 
>>> Huh, with or without comments, mark-defun always includes the empty
>>> lines before the defun for me. I get the same behavior in rust-ts-mode.
>>> This seems intentional, because this is at the end of the definition of
>>> mark-defun:
>>> 
>>> (skip-chars-backward "[:space:]\n")
>>> (unless (bobp)
>>> (forward-line 1))
>> 
>> Did you check with my example or another example? With my example, the
>> issue I get is that only the empty lines are marked, without the
>> defun, without the comments. I'm attaching two screenshots - before
>> and after mark-defun.
>> 
>> Note: I get the correct behaviour when there is just one defun in the
>> file. But if there are more, or the defun at point is not at the top
>> of the file - mark-defun does not work as expected.
> 
> Sorry, I used wrong go mode when testing your recipe. I used go-mode thinking it is go-ts-mode :-( Using go-ts-mode, I can reproduce what you see, yes.
> 
> The problem you discovered revealed some shortcoming in our tree-sitter navigation function, and requires quite a few non-trivial changes, I working on fixing it.
> 
> The direct cause is that (treesit-beginning-of-defun -1) is supposed to go to the beginning of next defun, but instead goes to the beginning of next next defun. That was a design decision I made but I can see it causing old functions to misbehave. 
> 
> Yuan

The fix is brain-twisting but don’t embody a lot of code, yay! This now should be fixed.

Yuan

Message #23 received at 61617 <at> debbugs.gnu.org (full text, mbox):

From: Evgeni Kolev <evgenysw <at> gmail.com>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Theodor Thornhill <theo <at> thornhill.no>, 61617 <at> debbugs.gnu.org
Subject: Re: bug#61617: M-x mark-defun doesn't work correctly in tree-sitter
 modes when comments exist between functions
Date: Mon, 27 Feb 2023 18:03:37 +0200

Great, thanks for the fix! I tested a few common (for me) scenarios
and mark-defun works as expected!

On Mon, Feb 27, 2023 at 11:24 AM Yuan Fu <casouri <at> gmail.com> wrote:
>
>
>
> > On Feb 26, 2023, at 4:42 PM, Yuan Fu <casouri <at> gmail.com> wrote:
> >
> >
> >
> >> On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw <at> gmail.com> wrote:
> >>
> >> On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri <at> gmail.com> wrote:
> >>
> >>> Huh, with or without comments, mark-defun always includes the empty
> >>> lines before the defun for me. I get the same behavior in rust-ts-mode.
> >>> This seems intentional, because this is at the end of the definition of
> >>> mark-defun:
> >>>
> >>> (skip-chars-backward "[:space:]\n")
> >>> (unless (bobp)
> >>> (forward-line 1))
> >>
> >> Did you check with my example or another example? With my example, the
> >> issue I get is that only the empty lines are marked, without the
> >> defun, without the comments. I'm attaching two screenshots - before
> >> and after mark-defun.
> >>
> >> Note: I get the correct behaviour when there is just one defun in the
> >> file. But if there are more, or the defun at point is not at the top
> >> of the file - mark-defun does not work as expected.
> >
> > Sorry, I used wrong go mode when testing your recipe. I used go-mode thinking it is go-ts-mode :-( Using go-ts-mode, I can reproduce what you see, yes.
> >
> > The problem you discovered revealed some shortcoming in our tree-sitter navigation function, and requires quite a few non-trivial changes, I working on fixing it.
> >
> > The direct cause is that (treesit-beginning-of-defun -1) is supposed to go to the beginning of next defun, but instead goes to the beginning of next next defun. That was a design decision I made but I can see it causing old functions to misbehave.
> >
> > Yuan
>
> The fix is brain-twisting but don’t embody a lot of code, yay! This now should be fixed.
>
> Yuan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.