bug#70614: Wrong simple imenu definition for java

bug#70614: Wrong simple imenu definition for java

[Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

I noticed the treesitter imenu definitions for java in java-ts-mode.el have the following:

  ;; Imenu.
  (setq-local treesit-simple-imenu-settings
              '(("Class" \\`class_declaration\\'</%60class_declaration/'> nil nil)
                ("Interface" \\`interface_declaration\\'</%60interface_declaration/'> nil nil)
                ("Enum" \\`record_declaration\\'</%60record_declaration/'> nil nil)
                ("Method" \\`method_declaration\\'</%60method_declaration/'> nil nil)))
  (treesit-major-mode-setup))

  The 3rd definition for enums should either be for “Record” if that was the intention or match against \\`enum_declaration</%60enum_declaration>   if it really was meant for enums.

Ben

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

bug#70614: Wrong simple imenu definition for java

[Yuan Fu]

> On Apr 27, 2024, at 11:45 AM, Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
> 
> I noticed the treesitter imenu definitions for java in java-ts-mode.el have the following:
>    ;; Imenu.
>   (setq-local treesit-simple-imenu-settings
>               '(("Class" \\`class_declaration\\' nil nil)
>                 ("Interface" \\`interface_declaration\\' nil nil)
>                 ("Enum" \\`record_declaration\\' nil nil)
>                 ("Method" \\`method_declaration\\' nil nil)))
>   (treesit-major-mode-setup))
>    The 3rd definition for enums should either be for “Record” if that was the intention or match against \\`enum_declaration   if it really was meant for enums.
>  Ben

Thanks for reporting that! Is that you who posted this post on reddit?

https://www.reddit.com/r/emacs/comments/1cej9fr/hierarchical_treesitter_based_imenulist_with_java/

It looks pretty nice, do you want to just put it in Emacs? Also, there’s a chance that your code can be simplified by using treesit-induce-sparse-tree. You can look at treesit-simple-imenu for now it’s used. Basically we use treesit-induce-sparse-tree to find all the interesting nodes in the buffer, and go through the returned tree to convert nodes into imenu labels.

Also CCing Theo; Theo, WDYT?

Yuan

bug#70614: Wrong simple imenu definition for java

[Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi Yuan
Yes it was me that posted on reddit. I’ll look into the sparse tree method – that looks promising.  I’d be happy to add into the tree. What would the process be for that?
Ben

From: Yuan Fu <casouri@gmail.com>
Date: Monday, April 29, 2024 at 10:06 PM
To: Benjamin Leis <benjamin.leis@servicenow.com>
Cc: 70614@debbugs.gnu.org <70614@debbugs.gnu.org>, Theodor Thornhill <theo@thornhill.no>
Subject: Re: bug#70614: Wrong simple imenu definition for java
[External Email]



> On Apr 27, 2024, at 11:45 AM, Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
>
> I noticed the treesitter imenu definitions for java in java-ts-mode.el have the following:
> ;; Imenu.
> (setq-local treesit-simple-imenu-settings
> '(("Class" \\`class_declaration\\' nil nil)
> ("Interface" \\`interface_declaration\\' nil nil)
> ("Enum" \\`record_declaration\\' nil nil)
> ("Method" \\`method_declaration\\' nil nil)))
> (treesit-major-mode-setup))
> The 3rd definition for enums should either be for “Record” if that was the intention or match against \\`enum_declaration if it really was meant for enums.
> Ben

Thanks for reporting that! Is that you who posted this post on reddit?

https://www.reddit.com/r/emacs/comments/1cej9fr/hierarchical_treesitter_based_imenulist_with_java/<https://www.reddit.com/r/emacs/comments/1cej9fr/hierarchical_treesitter_based_imenulist_with_java>

It looks pretty nice, do you want to just put it in Emacs? Also, there’s a chance that your code can be simplified by using treesit-induce-sparse-tree. You can look at treesit-simple-imenu for now it’s used. Basically we use treesit-induce-sparse-tree to find all the interesting nodes in the buffer, and go through the returned tree to convert nodes into imenu labels.

Also CCing Theo; Theo, WDYT?

Yuan

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

bug#70614: Wrong simple imenu definition for java

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Benjamin Leis <benjamin.leis@servicenow.com> writes:

> Hi Yuan
>
> Yes it was me that posted on reddit. I’ll look into the sparse tree
> method – that looks promising.  I’d be happy to add into the tree.
> What would the process be for that?
>
> Ben

If you just extend the current implementation and wrap it nicely in a
patch and send it as a reply to this email, I'd be happy to review and
merge. Remember that for some "significant" contribution an assignment
to the FSF is needed. You can ask for a form to start that process here,
and someone will send it to you off list.

That can take some time, so if you're willing to do that I'd suggest you
get started on that sooner rather than later :-)

Theo

bug#70614: Wrong simple imenu definition for java

[Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

I don’t know if our legal department will clear this or not but I’m reaching out to them. I can prepare a patch for the basic changes in the meantime.  You can see the basic code changes from that reddit post. I’ll rename the functions to match in java-ts-mode.el , look into the sparse tree function and if there is anything else stylistically, you’d like I could do that as well.

Smaller point, ,y preference is to expose records rather than enums and/or make the set configurable in a list.
Ben

From: Theodor Thornhill <theo@thornhill.no>
Date: Tuesday, April 30, 2024 at 11:30 AM
To: Benjamin Leis <benjamin.leis@servicenow.com>
Cc: Yuan Fu <casouri@gmail.com>, 70614@debbugs.gnu.org <70614@debbugs.gnu.org>
Subject: Re: bug#70614: Wrong simple imenu definition for java
[External Email]

Benjamin Leis <benjamin.leis@servicenow.com> writes:

> Hi Yuan
>
> Yes it was me that posted on reddit. I’ll look into the sparse tree
> method – that looks promising.  I’d be happy to add into the tree.
> What would the process be for that?
>
> Ben

If you just extend the current implementation and wrap it nicely in a
patch and send it as a reply to this email, I'd be happy to review and
merge. Remember that for some "significant" contribution an assignment
to the FSF is needed. You can ask for a form to start that process here,
and someone will send it to you off list.

That can take some time, so if you're willing to do that I'd suggest you
get started on that sooner rather than later :-)

Theo

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

bug#70614: Wrong simple imenu definition for java

[Yuan Fu]

> On Apr 30, 2024, at 11:30 AM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> Benjamin Leis <benjamin.leis@servicenow.com> writes:
> 
>> Hi Yuan
>> 
>> Yes it was me that posted on reddit. I’ll look into the sparse tree
>> method – that looks promising.  I’d be happy to add into the tree.
>> What would the process be for that?
>> 
>> Ben
> 
> If you just extend the current implementation and wrap it nicely in a
> patch and send it as a reply to this email, I'd be happy to review and
> merge. Remember that for some "significant" contribution an assignment
> to the FSF is needed. You can ask for a form to start that process here,
> and someone will send it to you off list.
> 
> That can take some time, so if you're willing to do that I'd suggest you
> get started on that sooner rather than later :-)
> 
> Theo

Eli, could you send Ben the copyright assignment form?

Yuan

bug#70614: Wrong simple imenu definition for java

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Tue, 30 Apr 2024 18:10:44 -0700
> Cc: Benjamin Leis <benjamin.leis@servicenow.com>,
>  "70614@debbugs.gnu.org" <70614@debbugs.gnu.org>,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Eli, could you send Ben the copyright assignment form?

Form sent off-list.

bug#70614: Wrong simple imenu definition for java

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Yuan Fu <casouri@gmail.com> writes:

>> On Apr 27, 2024, at 11:45 AM, Benjamin Leis via Bug reports for GNU Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
>> 
>> I noticed the treesitter imenu definitions for java in java-ts-mode.el have the following:
>>    ;; Imenu.
>>   (setq-local treesit-simple-imenu-settings
>>               '(("Class" \\`class_declaration\\' nil nil)
>>                 ("Interface" \\`interface_declaration\\' nil nil)
>>                 ("Enum" \\`record_declaration\\' nil nil)
>>                 ("Method" \\`method_declaration\\' nil nil)))
>>   (treesit-major-mode-setup))
>>    The 3rd definition for enums should either be for “Record” if that was the intention or match against \\`enum_declaration   if it really was meant for enums.
>>  Ben
>
> Thanks for reporting that! Is that you who posted this post on reddit?
>
> https://www.reddit.com/r/emacs/comments/1cej9fr/hierarchical_treesitter_based_imenulist_with_java/
>
> It looks pretty nice, do you want to just put it in Emacs? Also, there’s a
> chance that your code can be simplified by using treesit-induce-sparse-tree. You
> can look at treesit-simple-imenu for now it’s used. Basically we use
> treesit-induce-sparse-tree to find all the interesting nodes in the buffer, and
> go through the returned tree to convert nodes into imenu labels.
>
> Also CCing Theo; Theo, WDYT?

I like it - let's get it in :)

Theo