bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

The current js--treesit-imenu, used by the JavaScript, TypeScript and
TSX tree-sitter modes, would return empty pairs in the imenu alist if
there were none of that type of symbol.

This would break both the built in imenu and also packages like
consult-imenu.

See https://github.com/minad/consult/issues/697 for the discussion there.
---
 lisp/progmodes/js.el | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 45dfef372cd..6ca260ad8ad 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3732,10 +3732,17 @@ js--treesit-imenu
          (func-tree (treesit-induce-sparse-tree
                      node "function_declaration" nil 1000))
          (var-tree (treesit-induce-sparse-tree
-                    node "lexical_declaration" nil 1000)))
-    `(("Class" . ,(js--treesit-imenu-1 class-tree))
-      ("Variable" . ,(js--treesit-imenu-1 var-tree))
-      ("Function" . ,(js--treesit-imenu-1 func-tree)))))
+                    node "lexical_declaration" nil 1000))
+         (imenu-alist nil))
+    ;; when a sub-tree is empty, we should not return that pair at all
+    ;; https://github.com/minad/consult/issues/697#issuecomment-1345302734
+    (when func-tree
+      (setq imenu-alist (cons `("Function" . ,(js--treesit-imenu-1 func-tree)) imenu-alist)))
+    (when var-tree
+      (setq imenu-alist (cons `("Variable" . ,(js--treesit-imenu-1 var-tree)) imenu-alist)))
+    (when class-tree
+      (setq imenu-alist (cons `("Class" . ,(js--treesit-imenu-1 class-tree)) imenu-alist)))
+    imenu-alist))

 ;;; Main Function

--
2.25.1

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Yuan Fu]

"Charl P. Botha" <cpbotha@vxlabs.com> writes:

> The current js--treesit-imenu, used by the JavaScript, TypeScript and
> TSX tree-sitter modes, would return empty pairs in the imenu alist if
> there were none of that type of symbol.
>
> This would break both the built in imenu and also packages like
> consult-imenu.
>
> See https://github.com/minad/consult/issues/697 for the discussion
> there.

Thank you very much! Sorry for the inconvenience it caused. Have you
signed the copyright assignment? I’m asking because this change is on
the verge of 15 lines. Eli, WDYT?

Yuan

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

Dear Yuan Fu, dear Eli,

On Tue, Dec 13, 2022, at 00:28, Yuan Fu wrote:
> "Charl P. Botha" <cpbotha@vxlabs.com> writes:
>
>> The current js--treesit-imenu, used by the JavaScript, TypeScript and
>> TSX tree-sitter modes, would return empty pairs in the imenu alist if
>> there were none of that type of symbol.
>>
>> This would break both the built in imenu and also packages like
>> consult-imenu.
>>
>> See https://github.com/minad/consult/issues/697 for the discussion
>> there.
>
> Thank you very much! Sorry for the inconvenience it caused. Have you
> signed the copyright assignment? I’m asking because this change is on
> the verge of 15 lines. Eli, WDYT?

Emacs is (and always has been) a positive force in my life, thank you for all the energy that you pour into it!

I have not yet signed the copyright assignment, I would gladly do so.

Hopefully we can do this digitally? I live in South Africa, but I'm a Dutch citizen, in case that matters. I also have a PGP key [1] that's fairly well connected and can be verified on keybase (FWIW) [2]

Kind regards,
Charl

[1] http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x384435C7E77A4564
[2] https://keybase.io/cpbotha

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 12 Dec 2022 14:28:19 -0800
> Cc: 59945@debbugs.gnu.org,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Thank you very much! Sorry for the inconvenience it caused. Have you
> signed the copyright assignment? I’m asking because this change is on
> the verge of 15 lines. Eli, WDYT?

Yes, it's fine to install this, but please remember to add the
Copyright-paperwork-exempt thingy to the log message.

Thanks.

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Eli Zaretskii]

> Date: Tue, 13 Dec 2022 09:48:02 +0200
> From: "Charl P. Botha" <cpbotha@vxlabs.com>
> Cc: 59945@debbugs.gnu.org, "Eli Zaretskii" <eliz@gnu.org>
> 
> Emacs is (and always has been) a positive force in my life, thank you for all the energy that you pour into it!
> 
> I have not yet signed the copyright assignment, I would gladly do so.

Thanks, form sent off-list.

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

Dear Yuan,

On Tue, Dec 13, 2022, at 14:03, Eli Zaretskii wrote:
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 12 Dec 2022 14:28:19 -0800
>> Cc: 59945@debbugs.gnu.org,
>>  Eli Zaretskii <eliz@gnu.org>
>> 
>> Thank you very much! Sorry for the inconvenience it caused. Have you
>> signed the copyright assignment? I’m asking because this change is on
>> the verge of 15 lines. Eli, WDYT?
>
> Yes, it's fine to install this, but please remember to add the
> Copyright-paperwork-exempt thingy to the log message.

In the meantime, I've sent my signed copyright assignment, and my employer's docusigned disclaimer of rights, to the FSF.

I would be super grateful if you could add the commit when you can make the time.

Kind regards,
Charl

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Eli Zaretskii]

> Date: Thu, 15 Dec 2022 11:13:41 +0200
> From: "Charl P. Botha" <cpbotha@vxlabs.com>
> Cc: 59945@debbugs.gnu.org, "Eli Zaretskii" <eliz@gnu.org>
> 
> Dear Yuan,
> 
> On Tue, Dec 13, 2022, at 14:03, Eli Zaretskii wrote:
> >> From: Yuan Fu <casouri@gmail.com>
> >> Date: Mon, 12 Dec 2022 14:28:19 -0800
> >> Cc: 59945@debbugs.gnu.org,
> >>  Eli Zaretskii <eliz@gnu.org>
> >> 
> >> Thank you very much! Sorry for the inconvenience it caused. Have you
> >> signed the copyright assignment? I’m asking because this change is on
> >> the verge of 15 lines. Eli, WDYT?
> >
> > Yes, it's fine to install this, but please remember to add the
> > Copyright-paperwork-exempt thingy to the log message.
> 
> In the meantime, I've sent my signed copyright assignment, and my employer's docusigned disclaimer of rights, to the FSF.

Thanks.

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Yuan Fu]

"Charl P. Botha" <cpbotha@vxlabs.com> writes:

> Dear Yuan,
>
> On Tue, Dec 13, 2022, at 14:03, Eli Zaretskii wrote:
>>> From: Yuan Fu <casouri@gmail.com>
>>> Date: Mon, 12 Dec 2022 14:28:19 -0800
>>> Cc: 59945@debbugs.gnu.org,
>>>  Eli Zaretskii <eliz@gnu.org>
>>> 
>>> Thank you very much! Sorry for the inconvenience it caused. Have you
>>> signed the copyright assignment? I’m asking because this change is on
>>> the verge of 15 lines. Eli, WDYT?
>>
>> Yes, it's fine to install this, but please remember to add the
>> Copyright-paperwork-exempt thingy to the log message.
>
> In the meantime, I've sent my signed copyright assignment, and my employer's docusigned disclaimer of rights, to the FSF.
>
> I would be super grateful if you could add the commit when you can make the time.

Thanks. Unless I’m missing something, your patch is a diff file rather
than a patch file. You can generate one with

git format-patch -1 --stdout >~/mypatch.patch

Don’t worry about commit message this time, I’ll reword your commit and
add it. But if you want to submit more patches in the future I recommend
you to read CONTRIBUTE file under repo root, which describes the commit
message format Emacs uses.

Yuan

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

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

Dear Yuan,

On Fri, Dec 16, 2022, at 03:40, Yuan Fu wrote:
> "Charl P. Botha" <cpbotha@vxlabs.com> writes:
>> In the meantime, I've sent my signed copyright assignment, and my employer's docusigned disclaimer of rights, to the FSF.
>>
>> I would be super grateful if you could add the commit when you can make the time.
>
> Thanks. Unless I’m missing something, your patch is a diff file rather
> than a patch file. You can generate one with
>
> git format-patch -1 --stdout >~/mypatch.patch
>
> Don’t worry about commit message this time, I’ll reword your commit and
> add it. But if you want to submit more patches in the future I recommend
> you to read CONTRIBUTE file under repo root, which describes the commit
> message format Emacs uses.

I followed the instructions at https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html which said to use "git format-patch master" and include either inline (which I did) or as a mime attachment.

I have now re-done it using your advice above, and included the patch file as attachment to this email.

Next time (would be great if it happens), I'll attach by default.

Kind regards,
Charl

[-- Attachment #2: cpb-fix-empty-pairs-js-ts.patch --]
[-- Type: application/octet-stream, Size: 1859 bytes --]

From 01c4f3ade5c61b96718bbf36b2ed7b4b1ed84aa0 Mon Sep 17 00:00:00 2001
From: "Charl P. Botha" <cpbotha@vxlabs.com>
Date: Sat, 10 Dec 2022 19:09:38 +0200
Subject: [PATCH] Fix empty pairs in js tree-sitter imenu alist

The current js--treesit-imenu, used by the JavaScript, TypeScript and
TSX tree-sitter modes, would return empty pairs in the imenu alist if
there were none of that type of symbol.

This would break both the built in imenu and also packages like
consult-imenu.

See https://github.com/minad/consult/issues/697 for the discussion there.
---
 lisp/progmodes/js.el | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e0625951b65..8fd0386e6ae 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3738,10 +3738,17 @@ js--treesit-imenu
          (func-tree (treesit-induce-sparse-tree
                      node "function_declaration" nil 1000))
          (var-tree (treesit-induce-sparse-tree
-                    node "lexical_declaration" nil 1000)))
-    `(("Class" . ,(js--treesit-imenu-1 class-tree))
-      ("Variable" . ,(js--treesit-imenu-1 var-tree))
-      ("Function" . ,(js--treesit-imenu-1 func-tree)))))
+                    node "lexical_declaration" nil 1000))
+         (imenu-alist nil))
+    ;; when a sub-tree is empty, we should not return that pair at all
+    ;; https://github.com/minad/consult/issues/697#issuecomment-1345302734
+    (when func-tree
+      (setq imenu-alist (cons `("Function" . ,(js--treesit-imenu-1 func-tree)) imenu-alist)))
+    (when var-tree
+      (setq imenu-alist (cons `("Variable" . ,(js--treesit-imenu-1 var-tree)) imenu-alist)))
+    (when class-tree
+      (setq imenu-alist (cons `("Class" . ,(js--treesit-imenu-1 class-tree)) imenu-alist)))
+    imenu-alist))
 
 ;;; Main Function
 
-- 
2.25.1

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Robert Pluim]

>>>>> On Fri, 16 Dec 2022 07:58:46 +0200, "Charl P. Botha" <cpbotha@vxlabs.com> said:

    Charl> I followed the instructions at
    Charl> https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html
    Charl> which said to use "git format-patch master" and include either inline
    Charl> (which I did) or as a mime attachment.

Not quite. You sent the result of 'git format-patch master' as an
email, not inline in an email (presumably with 'git send-email'). That
shouldnʼt matter, because 'git apply' should work in either case, but
attachments are sometimes easier to work with.

Robert
-- 

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

On Fri, Dec 16, 2022, at 16:11, Robert Pluim wrote:
>>>>>> On Fri, 16 Dec 2022 07:58:46 +0200, "Charl P. Botha" <cpbotha@vxlabs.com> said:
>     Charl> I followed the instructions at
>     Charl> 
> https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html
>     Charl> which said to use "git format-patch master" and include 
> either inline
>     Charl> (which I did) or as a mime attachment.
>
> Not quite. You sent the result of 'git format-patch master' as an
> email, not inline in an email (presumably with 'git send-email'). That
> shouldnʼt matter, because 'git apply' should work in either case, but
> attachments are sometimes easier to work with.

Thank you for the correction, Robert! I thought that sending the output of `git format-patch master` as a plain-text email was what was meant by "inline". I'm sorry about this, I'll stick to attachments in the future.

Yuan, can you confirm that the patch I eventually attached is OK?

Kind regards,
Charl

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Yuan Fu]

"Charl P. Botha" <cpbotha@vxlabs.com> writes:

> On Fri, Dec 16, 2022, at 16:11, Robert Pluim wrote:
>>>>>>> On Fri, 16 Dec 2022 07:58:46 +0200, "Charl P. Botha" <cpbotha@vxlabs.com> said:
>>     Charl> I followed the instructions at
>>     Charl> 
>> https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html
>>     Charl> which said to use "git format-patch master" and include 
>> either inline
>>     Charl> (which I did) or as a mime attachment.
>>
>> Not quite. You sent the result of 'git format-patch master' as an
>> email, not inline in an email (presumably with 'git send-email'). That
>> shouldnʼt matter, because 'git apply' should work in either case, but
>> attachments are sometimes easier to work with.
>
> Thank you for the correction, Robert! I thought that sending the
> output of `git format-patch master` as a plain-text email was what was
> meant by "inline". I'm sorry about this, I'll stick to attachments in
> the future.

Not really about attachment vs inline. The first patch doesn’t have
author, commit message, etc. I’m fine with either attachment or inline
:-)

> Yuan, can you confirm that the patch I eventually attached is OK?

Thanks! I applied your patch. I made a little change to it so it’s more
idiomatic.

Elisp tip of the day: if you want to make a list of possibly nil objects
and don’t want nil’s in the list, you can use append. Basically change

(list a b c)

to

(append (and xxx (list a)) (and yyy (list b)) (and zzz (list c)))

If the condition aren’t met, eg, (and xxx (list a)) returns nil, append
just appends an empty list.

Yuan

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Charl P. Botha]

Dear Yuan,

On Wed, Dec 21, 2022, at 06:48, Yuan Fu wrote:
> Thanks! I applied your patch. I made a little change to it so it’s more
> idiomatic.
>
> Elisp tip of the day: if you want to make a list of possibly nil objects
> and don’t want nil’s in the list, you can use append. Basically change
>
> (list a b c)
>
> to
>
> (append (and xxx (list a)) (and yyy (list b)) (and zzz (list c)))
>
> If the condition aren’t met, eg, (and xxx (list a)) returns nil, append
> just appends an empty list.

Thank you very much for taking care of this, and thank you for your emacs-lisp improvement and tip. On that day, I searched for but was not able to find a better formulation than my klunky one.

Kind regards,
Charl

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Robert Pluim]

>>>>> On Tue, 20 Dec 2022 20:48:58 -0800, Yuan Fu <casouri@gmail.com> said:

    Yuan> "Charl P. Botha" <cpbotha@vxlabs.com> writes:

    >> On Fri, Dec 16, 2022, at 16:11, Robert Pluim wrote:
    >>>>>>>> On Fri, 16 Dec 2022 07:58:46 +0200, "Charl P. Botha" <cpbotha@vxlabs.com> said:
    Charl> I followed the instructions at
    Charl> 
    >>> https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html
    Charl> which said to use "git format-patch master" and include 
    >>> either inline
    Charl> (which I did) or as a mime attachment.
    >>> 
    >>> Not quite. You sent the result of 'git format-patch master' as an
    >>> email, not inline in an email (presumably with 'git send-email'). That
    >>> shouldnʼt matter, because 'git apply' should work in either case, but
    >>> attachments are sometimes easier to work with.
    >> 
    >> Thank you for the correction, Robert! I thought that sending the
    >> output of `git format-patch master` as a plain-text email was what was
    >> meant by "inline". I'm sorry about this, I'll stick to attachments in
    >> the future.

    Yuan> Not really about attachment vs inline. The first patch doesn’t have
    Yuan> author, commit message, etc. I’m fine with either attachment or inline
    Yuan> :-)

It did, itʼs just that the Author was the From of the email itself,
and the commit message was everything before '---' :-)

Itʼs the style used on linux-kernel and related lists.

Robert
-- 

bug#59945: [PATCH] Fix empty pairs in js tree-sitter imenu alist

[Yuan Fu]

> On Dec 21, 2022, at 9:25 AM, Robert Pluim <rpluim@gmail.com> wrote:
> 
>>>>>> On Tue, 20 Dec 2022 20:48:58 -0800, Yuan Fu <casouri@gmail.com> said:
> 
>    Yuan> "Charl P. Botha" <cpbotha@vxlabs.com> writes:
> 
>>> On Fri, Dec 16, 2022, at 16:11, Robert Pluim wrote:
>>>>>>>>> On Fri, 16 Dec 2022 07:58:46 +0200, "Charl P. Botha" <cpbotha@vxlabs.com> said:
>    Charl> I followed the instructions at
>    Charl> 
>>>> https://www.gnu.org/software/emacs/manual/html_node/emacs/Sending-Patches.html
>    Charl> which said to use "git format-patch master" and include 
>>>> either inline
>    Charl> (which I did) or as a mime attachment.
>>>> 
>>>> Not quite. You sent the result of 'git format-patch master' as an
>>>> email, not inline in an email (presumably with 'git send-email'). That
>>>> shouldnʼt matter, because 'git apply' should work in either case, but
>>>> attachments are sometimes easier to work with.
>>> 
>>> Thank you for the correction, Robert! I thought that sending the
>>> output of `git format-patch master` as a plain-text email was what was
>>> meant by "inline". I'm sorry about this, I'll stick to attachments in
>>> the future.
> 
>    Yuan> Not really about attachment vs inline. The first patch doesn’t have
>    Yuan> author, commit message, etc. I’m fine with either attachment or inline
>    Yuan> :-)
> 
> It did, itʼs just that the Author was the From of the email itself,
> and the commit message was everything before '---' :-)
> 
> Itʼs the style used on linux-kernel and related lists.

Ahhhh, TIL.

Yuan