* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions @ 2023-02-19 8:39 Evgeni Kolev 2023-02-20 8:30 ` Evgeni Kolev 2023-02-25 5:08 ` Yuan Fu 0 siblings, 2 replies; 7+ messages in thread From: Evgeni Kolev @ 2023-02-19 8:39 UTC (permalink / raw) To: 61617 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!"); } ``` ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-19 8:39 bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions Evgeni Kolev @ 2023-02-20 8:30 ` Evgeni Kolev 2023-02-25 5:08 ` Yuan Fu 1 sibling, 0 replies; 7+ messages in thread From: Evgeni Kolev @ 2023-02-20 8:30 UTC (permalink / raw) To: 61617 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-19 8:39 bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions Evgeni Kolev 2023-02-20 8:30 ` Evgeni Kolev @ 2023-02-25 5:08 ` Yuan Fu 2023-02-25 7:27 ` Evgeni Kolev 1 sibling, 1 reply; 7+ messages in thread From: Yuan Fu @ 2023-02-25 5:08 UTC (permalink / raw) To: evgenysw; +Cc: Theodor Thornhill, 61617 Evgeni Kolev <evgenysw@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-25 5:08 ` Yuan Fu @ 2023-02-25 7:27 ` Evgeni Kolev 2023-02-27 0:42 ` Yuan Fu 0 siblings, 1 reply; 7+ messages in thread From: Evgeni Kolev @ 2023-02-25 7:27 UTC (permalink / raw) To: Yuan Fu; +Cc: Theodor Thornhill, 61617 [-- Attachment #1: Type: text/plain, Size: 1189 bytes --] On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri@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. [-- Attachment #2: before-mark-defun.png --] [-- Type: image/png, Size: 88615 bytes --] [-- Attachment #3: after-mark-defun.png --] [-- Type: image/png, Size: 85871 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-25 7:27 ` Evgeni Kolev @ 2023-02-27 0:42 ` Yuan Fu 2023-02-27 9:24 ` Yuan Fu 0 siblings, 1 reply; 7+ messages in thread From: Yuan Fu @ 2023-02-27 0:42 UTC (permalink / raw) To: Evgeni Kolev; +Cc: Theodor Thornhill, 61617 > On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw@gmail.com> wrote: > > On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-27 0:42 ` Yuan Fu @ 2023-02-27 9:24 ` Yuan Fu 2023-02-27 16:03 ` Evgeni Kolev 0 siblings, 1 reply; 7+ messages in thread From: Yuan Fu @ 2023-02-27 9:24 UTC (permalink / raw) To: Evgeni Kolev; +Cc: Theodor Thornhill, 61617 > On Feb 26, 2023, at 4:42 PM, Yuan Fu <casouri@gmail.com> wrote: > > > >> On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw@gmail.com> wrote: >> >> On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions 2023-02-27 9:24 ` Yuan Fu @ 2023-02-27 16:03 ` Evgeni Kolev 0 siblings, 0 replies; 7+ messages in thread From: Evgeni Kolev @ 2023-02-27 16:03 UTC (permalink / raw) To: Yuan Fu; +Cc: Theodor Thornhill, 61617 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@gmail.com> wrote: > > > > > On Feb 26, 2023, at 4:42 PM, Yuan Fu <casouri@gmail.com> wrote: > > > > > > > >> On Feb 24, 2023, at 11:27 PM, Evgeni Kolev <evgenysw@gmail.com> wrote: > >> > >> On Sat, Feb 25, 2023 at 7:08 AM Yuan Fu <casouri@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-27 16:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-19 8:39 bug#61617: M-x mark-defun doesn't work correctly in tree-sitter modes when comments exist between functions Evgeni Kolev 2023-02-20 8:30 ` Evgeni Kolev 2023-02-25 5:08 ` Yuan Fu 2023-02-25 7:27 ` Evgeni Kolev 2023-02-27 0:42 ` Yuan Fu 2023-02-27 9:24 ` Yuan Fu 2023-02-27 16:03 ` Evgeni Kolev
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.