[imenu.el] Attempt to fix broken imenu--generic-function

[imenu.el] Attempt to fix broken imenu--generic-function

[Filipp Gunbin]

Drew, I'm sorry, your commit 77166e0da2d58f2f6436989b7059d913be5b3439
broke imenu.  Do you have objections for this fix?

diff --git a/lisp/imenu.el b/lisp/imenu.el
index edca51e3ad..7285b10574 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -832,15 +832,14 @@ imenu--generic-function
     (dolist (item index-alist)
       (when (listp item)
 	(setcdr item (sort (cdr item) 'imenu--sort-by-position))))
-    (let ((main-element (assq nil index-alist)))
-      (nconc (delq main-element (delq 'dummy index-alist))
-             (cdr main-element)))
     ;; Remove any empty menus.  That can happen because of skipping
     ;; things inside comments or strings.
-    (when (consp (car index-alist))
-      (setq index-alist  (cl-delete-if-not
-                          (lambda (it) (cdr it))
-                          index-alist)))))
+    (setq index-alist (cl-delete-if
+                       (lambda (it) (and (consp it) (null (cdr it))))
+                       index-alist))
+    (let ((main-element (assq nil index-alist)))
+      (nconc (delq main-element (delq 'dummy index-alist))
+             (cdr main-element)))))
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;

RE: [imenu.el] Attempt to fix broken imenu--generic-function

[Drew Adams]

> Drew, I'm sorry, your commit 77166e0da2d58f2f6436989b7059d913be5b3439
> broke imenu.  Do you have objections for this fix?

Sorry, but I have no idea what you're talking about.
I haven't committed anything.

Is this about some Emacs bug?  If so, what bug #?

Can you please specify what was broken, how/why, etc.?

Thx.

Re: [imenu.el] Attempt to fix broken imenu--generic-function

[Filipp Gunbin]

On 17/07/2018 13:55 -0700, Drew Adams wrote:

>> Drew, I'm sorry, your commit 77166e0da2d58f2f6436989b7059d913be5b3439
>> broke imenu.  Do you have objections for this fix?
>
> Sorry, but I have no idea what you're talking about.
> I haven't committed anything.
>
> Is this about some Emacs bug?  If so, what bug #?
>
> Can you please specify what was broken, how/why, etc.?
>
> Thx.

Yes, it's in emacs master (emacs-devel list is in CC).  I gave commit
hash (77166e0da2d58f2f6436989b7059d913be5b3439).

Below is the relevant diff of your change.

What is broken: result of nconc is no longer returned from the function.
And, it's unclear why test `(consp (car index-alist))' before filtering
empty menus - by this time main-element is already "spliced" into the
index-alist, so index-alist element may be a usual element (INDEX-NAME
. INDEX-POSITION) or nested index alist (INDEX-NAME . INDEX-ALIST).  So
the test appears obscure.

That results in imenu errors.

My fix attempts to preseve the behavior you added, while fixing that.
I'm not attaching the diff again, it's in the previous message.


commit 77166e0da2d58f2f6436989b7059d913be5b3439
Author: Drew Adams <drew.adams@oracle.com>
Date:   Sat Jul 7 19:20:45 2018 +0300

    Fix 2 minor bugs in 'imenu--generic-function'
    
    * lisp/imenu.el (imenu--generic-function): Move point to START
    before checking whether the current item is inside a comment
    or a string.  Remove any empty menus that could have been
    added before returning.  (Bug#32024)

diff --git a/lisp/imenu.el b/lisp/imenu.el
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -828,9 +830,15 @@
 	  (set-syntax-table old-table)))
     ;; Sort each submenu by position.
     ;; This is in case one submenu gets items from two different regexps.
     (dolist (item index-alist)
       (when (listp item)
 	(setcdr item (sort (cdr item) 'imenu--sort-by-position))))
     (let ((main-element (assq nil index-alist)))
       (nconc (delq main-element (delq 'dummy index-alist))
-	     (cdr main-element)))))
+             (cdr main-element)))
+    ;; Remove any empty menus.  That can happen because of skipping
+    ;; things inside comments or strings.
+    (when (consp (car index-alist))
+      (setq index-alist  (cl-delete-if-not
+                          (lambda (it) (cdr it))
+                          index-alist)))))

RE: [imenu.el] Attempt to fix broken imenu--generic-function

[Drew Adams]

> > Is this about some Emacs bug?  If so, what bug #?
> > Can you please specify what was broken, how/why, etc.?
> 
I didn't commit it, but yes, it's for bug #32024.  (Shouldn't
the commit log say that?)

> What is broken: result of nconc is no longer returned from the function.

Right you are.  The fix looks good; thx.

Re: [imenu.el] Attempt to fix broken imenu--generic-function

[Eli Zaretskii]

> Date: Tue, 17 Jul 2018 16:18:29 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: emacs-devel@gnu.org
> 
> > > Is this about some Emacs bug?  If so, what bug #?
> > > Can you please specify what was broken, how/why, etc.?
> > 
> I didn't commit it, but yes, it's for bug #32024.  (Shouldn't
> the commit log say that?)

It does, doesn't it?

  commit 77166e0da2d58f2f6436989b7059d913be5b3439
  Author: Drew Adams <drew.adams@oracle.com>
  Date:   Sat Jul 7 19:20:45 2018 +0300

      Fix 2 minor bugs in 'imenu--generic-function'

      * lisp/imenu.el (imenu--generic-function): Move point to START
      before checking whether the current item is inside a comment
      or a string.  Remove any empty menus that could have been
      added before returning.  (Bug#32024)
				^^^^^^^^^

RE: [imenu.el] Attempt to fix broken imenu--generic-function

[Drew Adams]

> > > > > your commit
> > > > > 77166e0da2d58f2f6436989b7059d913be5b3439
> > > >
> > > > Sorry, but I have no idea what you're talking
> > > > about. I haven't committed anything.  Is this
> > > > about some Emacs bug? If so, what bug #?
> > >
> > > commit hash
> > > (77166e0da2d58f2f6436989b7059d913be5b3439).
> >
> > [searches mail messages...]   I didn't commit
> > it, but yes, it's for bug #32024. (Shouldn't
> > the commit log say that?)
> 
> It does, doesn't it?  [shows bug # in log entry]

That was my question. Good.

I'm reading this in emacs-devel.  Nothing was
said here about the bug.  Only a hex commit
number was presented, saying it was my commit.
I know nothing about that. I didn't commit the
patch. (But thank you for committing it.)

> commit 77166e0da2d58f2f6436989b7059d913be5b3439

Since the commit log does mention the bug, the
bug # could have been cited here. Or the log
entry could have been shown here, as you've
done now.

Anyway, mystery, and problem, solved - thanks
to all.

RE: [imenu.el] Attempt to fix broken imenu--generic-function

[Eli Zaretskii]

On July 18, 2018 7:36:44 AM GMT+03:00, Drew Adams <drew.adams@oracle.com> wrote:
> > > [searches mail messages...]   I didn't commit
> > > it, but yes, it's for bug #32024. (Shouldn't
> > > the commit log say that?)
> > 
> > It does, doesn't it?  [shows bug # in log entry]
> 
> That was my question. Good.
> 
> I'm reading this in emacs-devel.  Nothing was
> said here about the bug.  Only a hex commit
> number was presented, saying it was my commit.

I'm also reading emacs-devel.  Filipp has shown the entire
commit log message, including the bug number, in the
message to which you were responding, asking where
is the bug number.

Re: [imenu.el] Attempt to fix broken imenu--generic-function

[Filipp Gunbin]

On 17/07/2018 16:18 -0700, Drew Adams wrote:

>> What is broken: result of nconc is no longer returned from the function.
>
> Right you are.  The fix looks good; thx.

Thanks, installed.