bug#59853: 30.0.50; tree-sitter modes have unexpected beginning-of-defun behavior

[Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Brian Leung <leungbk@posteo.net> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> Not sure I can reproduce this.
>
> Sorry, I made a mistake in my description. I additionally noticed 
> additional odd results in java-ts-mode. Let me try again:
>
>> p[u]blic abstract class Class {
>>     public static interface Interface {
>>         void someMethod();
>> [*]
>>         void otherMethod();
>>     [}]
>> }

Right - thanks!  Now I understand what you describe.  I'll try to answer
them case by case.

>
> 1. When point is at the "u" in "public", narrow-to-defun cuts off 
> "pu" and moves point to "b".

Yes, this definitely looks like a bug.  This is probably because the
whole 'public' keyword maps to the declaration, and we don't move to the
actual beginning.  I can look into this.

> 2. When point is anywhere in the first line of the class 
> declaration, mark-defun highlights "void otherMethod()", instead 
> of the entire class declaration.

Yeah, I think I've fixed this in a patch I just submitted.

> 3a. When point is at the [*] in between someMethod and 
> otherMethod, narrow-to-defun captures "void otherMethod()". I feel 
> that since the methods inside the interface declaration have no 
> bodies, it makes more sense to capture the entire interface 
> definition if point is at [*].

Maybe, but I don't believe this is wrong either.

> 3b. Arguably, even if point were on the method declarations, we 
> might still want to (as plain java-mode does) capture the entire 
> interface definition, since body-less method declarations don't 
> feel especially defun-like.

Maybe.  Can you try applying the below patch and see if this changes
anything for you?

> 4. When point is on the closing bracket of the interface 
> definition, narrow-to-defun captures
>
>>         void otherMethod();
>>     }
>> }
>
> which looks quite unusual to me.

Yes, I agree with this one.  My guess is that this is the same as number
one.

What do you think?

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-more-granular-defun-type-regexp.patch --]
[-- Type: text/x-diff, Size: 1459 bytes --]

From ef79549e793773b50faea9bad2cd35509e5714e1 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Wed, 7 Dec 2022 10:07:37 +0100
Subject: [PATCH] Make more granular defun-type-regexp

We don't want to match local_variable_declaration and others to hit on
beginning-of-defun.  The fix is just to make the regexp a bit more
granular.

* lisp/progmodes/java-ts-mode.el (java-ts-mode): Use regexp-opt to
distinguish more granularly.
---
 lisp/progmodes/java-ts-mode.el | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/java-ts-mode.el b/lisp/progmodes/java-ts-mode.el
index 2c42505ac9..db2566d655 100644
--- a/lisp/progmodes/java-ts-mode.el
+++ b/lisp/progmodes/java-ts-mode.el
@@ -317,7 +317,15 @@ java-ts-mode
               (append "{}():;," electric-indent-chars))
 
   ;; Navigation.
-  (setq-local treesit-defun-type-regexp "declaration")
+  (setq-local treesit-defun-type-regexp
+              (regexp-opt '("method_declaration"
+                            "class_declaration"
+                            "record_declaration"
+                            "interface_declaration"
+                            "enum_declaration"
+                            "import_declaration"
+                            "package_declaration"
+                            "module_declaration")))
 
   ;; Font-lock.
   (setq-local treesit-font-lock-settings java-ts-mode--font-lock-settings)
-- 
2.34.1