Error in C++ mode with Emacs 27.0.90

Error in C++ mode with Emacs 27.0.90

[Angelo Graziosi]

I found an error with C++ mode which I can reproduce with this init.el:

-------------------------------------------
$ cat init.el

;; C/C++ modes
(defun my-c-mode ()
  "My customization for `c-mode' and `c++-mode'."
  (interactive)

  ;; No indent for open bracket
  (c-set-offset 'substatement-open 0)

  ;; Add index of func. to menu bar
  (imenu-add-to-menubar "Functions")
  )

;; c++-mode
(add-hook 'c++-mode-hook 'my-c-mode)

(setq imenu-auto-rescan t)

;; The default is 60000
(setq imenu-auto-rescan-maxout 500000)

;; Show in which function is the cursor
(which-function-mode 1)
-------------------------------------------

(maybe it can be reduced...) and this test case:

-----------------------------------
$ cat foobar.cpp
int main()
{
  return 0;
}
-----------------------------------

When I visit it with C-x C-f, I get this error in minibuffer:

Error in menu-bar-update-hook (imenu-update-menubar): (wrong-type-argument sequencep #<marker at 1 in foobar.cpp>)

The error disappears if I add a space before 'int main()', i.e. with ' int main()'

I have seen that both on GNU/Linux and Windows builds of 27.0.70.

Ciao, 
  Angelo.

Re: Error in C++ mode with Emacs 27.0.90

[Angelo Graziosi]

> Il 24 marzo 2020 alle 21.50 Angelo Graziosi ha scritto:
> 
> 
> I found an error with C++ mode which I can reproduce with this init.el:
> 
> -------------------------------------------
> $ cat init.el
> 
> ;; C/C++ modes
> (defun my-c-mode ()
>   "My customization for `c-mode' and `c++-mode'."
>   (interactive)
> 
>   ;; No indent for open bracket
>   (c-set-offset 'substatement-open 0)
> 
>   ;; Add index of func. to menu bar
>   (imenu-add-to-menubar "Functions")
>   )
> 
> ;; c++-mode
> (add-hook 'c++-mode-hook 'my-c-mode)
> 
> (setq imenu-auto-rescan t)
> 
> ;; The default is 60000
> (setq imenu-auto-rescan-maxout 500000)
> 
> ;; Show in which function is the cursor
> (which-function-mode 1)
> -------------------------------------------
> 
> (maybe it can be reduced...) and this test case:
> 
> -----------------------------------
> $ cat foobar.cpp
> int main()
> {
>   return 0;
> }
> -----------------------------------
> 
> When I visit it with C-x C-f, I get this error in minibuffer:
> 
> Error in menu-bar-update-hook (imenu-update-menubar): (wrong-type-argument sequencep #<marker at 1 in foobar.cpp>)
> 
> The error disappears if I add a space before 'int main()', i.e. with ' int main()'
> 
> I have seen that both on GNU/Linux and Windows builds of 27.0.90.
>

Both Emacs 27 branch and master are affected by this issue.

> Ciao, 
>   Angelo.

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Angelo.

On Fri, Mar 27, 2020 at 16:56:02 +0100, Angelo Graziosi wrote:
> > Il 24 marzo 2020 alle 21.50 Angelo Graziosi ha scritto:


> > I found an error with C++ mode which I can reproduce with this init.el:

> > -------------------------------------------
> > $ cat init.el

> > ;; C/C++ modes
> > (defun my-c-mode ()
> >   "My customization for `c-mode' and `c++-mode'."
> >   (interactive)

> >   ;; No indent for open bracket
> >   (c-set-offset 'substatement-open 0)

> >   ;; Add index of func. to menu bar
> >   (imenu-add-to-menubar "Functions")
> >   )

> > ;; c++-mode
> > (add-hook 'c++-mode-hook 'my-c-mode)

> > (setq imenu-auto-rescan t)

> > ;; The default is 60000
> > (setq imenu-auto-rescan-maxout 500000)

> > ;; Show in which function is the cursor
> > (which-function-mode 1)
> > -------------------------------------------

> > (maybe it can be reduced...) and this test case:

> > -----------------------------------
> > $ cat foobar.cpp
> > int main()
> > {
> >   return 0;
> > }
> > -----------------------------------

> > When I visit it with C-x C-f, I get this error in minibuffer:

> > Error in menu-bar-update-hook (imenu-update-menubar): (wrong-type-argument sequencep #<marker at 1 in foobar.cpp>)

> > The error disappears if I add a space before 'int main()', i.e. with ' int main()'

> > I have seen that both on GNU/Linux and Windows builds of 27.0.90.


> Both Emacs 27 branch and master are affected by this issue.

I've had a look into this, and it seems that imenu and CC Mode disagree
about the correct format for an imenu alist when there's only one element
in it.

The function where things go wrong is imenu-update-menubar, in the "else"
branch of the single `if' form in the function.

I hope to have time soon to look into this more thoroughly, assuming
nobody else does first.  ;-)

> > Ciao, 
> >   Angelo.

-- 
Alan Mackenzie (Nuremberg, Germany)

Re: Error in C++ mode with Emacs 27.0.90

[Angelo Graziosi]

> Il 28 marzo 2020 alle 16.19 Alan Mackenzie ha scritto:
> 
> 
> The function where things go wrong is imenu-update-menubar, in the "else"
> branch of the single `if' form in the function.
> 
> I hope to have time soon to look into this more thoroughly, assuming
> nobody else does first.  ;-)

Thanks!

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello again, Angelo.

On Sat, Mar 28, 2020 at 18:34:50 +0100, Angelo Graziosi wrote:

> > Il 28 marzo 2020 alle 16.19 Alan Mackenzie ha scritto:


> > The function where things go wrong is imenu-update-menubar, in the "else"
> > branch of the single `if' form in the function.

> > I hope to have time soon to look into this more thoroughly, assuming
> > nobody else does first.  ;-)

OK, the problem was imenu-update-menubar's handling of nested imenu
element lists.  (Somewhat simplified) the function was inadequately
recognising a nested list by testing the list's length being 1.  With
such a list, it then attempted to strip the enclosing (superfluous)
verbiage.

However when the list had a single elemental element, such as the
function "main", it threw an exception on attempting to strip the
non-existent enclosing level.

I think the following patch fixes the trouble.  Please try it out and
report on this list how well it works.  Thanks:



diff --git a/lisp/imenu.el b/lisp/imenu.el
index fb8b3de662..7d25c2bf91 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -911,11 +911,21 @@ imenu-update-menubar
         (setq index-alist (imenu--split-submenus index-alist))
 	(let* ((menu (imenu--split-menu index-alist
                                         (buffer-name)))
-               (menu1 (imenu--create-keymap (car menu)
-					    (cdr (if (< 1 (length (cdr menu)))
-						     menu
-						   (car (cdr menu))))
-					    'imenu--menubar-select)))
+               (menu1 (imenu--create-keymap
+                       (car menu)
+		       (cdr
+                        (cond
+                         ((cddr menu)
+                          ;; (cdr menu) is a list of len > 1
+                          menu)
+                         ((and (consp (cdadr menu))
+                               ;; POSITION of a "Special element" is an atom:
+                               (consp (cadadr menu)))
+                          ;; Discard the enclosing level of the nested list.
+                          (cadr menu))
+                         (t             ; Non-nested list of length 1
+                          menu)))
+		       'imenu--menubar-select)))
 	  (setcdr imenu--menubar-keymap (cdr menu1)))))))
 
 (defun imenu--menubar-select (item)



> Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Angelo Graziosi]

> Il 28 marzo 2020 alle 21.10 Alan Mackenzie ha scritto:
> 
> 
> Hello again, Angelo.
> 
> OK, the problem was imenu-update-menubar's handling of nested imenu
> element lists.  (Somewhat simplified) the function was inadequately
>
> I think the following patch fixes the trouble.  Please try it out and
> report on this list how well it works.  Thanks:

Yes, it works! I tested it on master so I think it should be backported to Emacs-27

Thank you,

  Angelo.

> 
> 
> 
> diff --git a/lisp/imenu.el b/lisp/imenu.el
> index fb8b3de662..7d25c2bf91 100644
> --- a/lisp/imenu.el
> +++ b/lisp/imenu.el
> @@ -911,11 +911,21 @@ imenu-update-menubar
>          (setq index-alist (imenu--split-submenus index-alist))
>  	(let* ((menu (imenu--split-menu index-alist
>                                          (buffer-name)))
> -               (menu1 (imenu--create-keymap (car menu)
> -					    (cdr (if (< 1 (length (cdr menu)))
> -						     menu
> -						   (car (cdr menu))))
> -					    'imenu--menubar-select)))
> +               (menu1 (imenu--create-keymap
> +                       (car menu)
> +		       (cdr
> +                        (cond
> +                         ((cddr menu)
> +                          ;; (cdr menu) is a list of len > 1
> +                          menu)
> +                         ((and (consp (cdadr menu))
> +                               ;; POSITION of a "Special element" is an atom:
> +                               (consp (cadadr menu)))
> +                          ;; Discard the enclosing level of the nested list.
> +                          (cadr menu))
> +                         (t             ; Non-nested list of length 1
> +                          menu)))
> +		       'imenu--menubar-select)))
>  	  (setcdr imenu--menubar-keymap (cdr menu1)))))))
>  
>  (defun imenu--menubar-select (item)
> 
> 
> 
> > Thanks!
> 
> -- 
> Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Dmitry Gutov]

Hi Alan,

On 28.03.2020 22:10, Alan Mackenzie wrote:
> I think the following patch fixes the trouble.  Please try it out and
> report on this list how well it works

I wonder if we could rewrite the related code such as not to depend on 
whether a list has one element or several. That sounds ugly. Though the 
change would likely take a fair amount of rework.

In any case, imenu--create-keymap is also called from imenu--mouse-menu, 
which probably need the same kind of fix.

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Dmitry.

On Sun, Mar 29, 2020 at 01:18:13 +0200, Dmitry Gutov wrote:
> Hi Alan,

> On 28.03.2020 22:10, Alan Mackenzie wrote:
> > I think the following patch fixes the trouble.  Please try it out and
> > report on this list how well it works

> I wonder if we could rewrite the related code such as not to depend on 
> whether a list has one element or several. That sounds ugly. Though the 
> change would likely take a fair amount of rework.

It seems unlikely that we can get a nested list (in imenu--index-alist)
with only one element.  I have tried creating a C++ buffer with 25
functions in it.  This gives a simple list of 25 elements.  With 26
functions, I get a nested alist with two elements.

However, in an emacs-lisp-mode buffer, we get things like "Variables"
and "*Rescan*", but I've not yet succeeded in causing a nested list with
just one element.  I'm not convinced enough that it couldn't happen,
though.

The latter part of the function looks like this (before my patch):

        (let* ((menu (imenu--split-menu index-alist
                                        (buffer-name)))
               (menu1 (imenu--create-keymap (car menu)
========>                                   (cdr (if (< 1 (length (cdr menu)))
                                                     menu
                                                   (car (cdr menu))))
                                            'imenu--menubar-select)))
          (setcdr imenu--menubar-keymap (cdr menu1)))))))

That `if' form has been there since imenu-update-menubar was first
written by Karl Heuer in 1997 (commit 0a8e8bc63e3).  Presumably, it
really was needed back then.

> In any case, imenu--create-keymap is also called from imenu--mouse-menu, 
> which probably needs the same kind of fix.

Possibly.  I've not managed to create the same error in
imenu--mouse-menu, but perhaps it could do with the same correction,
just in case.  What do you think?

Incidentally, that patch from last night was a bit untidy.  I think it
needs cleaning up a bit.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Angelo.

On Sat, Mar 28, 2020 at 23:31:04 +0100, Angelo Graziosi wrote:

> > Il 28 marzo 2020 alle 21.10 Alan Mackenzie ha scritto:

[ .... ]

> > I think the following patch fixes the trouble.  Please try it out and
> > report on this list how well it works.  Thanks:

> Yes, it works! I tested it on master so I think it should be backported
> to Emacs-27

Thanks!

Is it really important enough for Emacs 27?  It only happens in a file
with one single function, and even then, if I understand it right, it
doesn't prevent the user adding another function and carrying on with his
work.

Anyhow, Eli would need to give his permission before we could add it to
Emacs 27.

> Thank you,

>   Angelo.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Eli Zaretskii]

> Date: Sun, 29 Mar 2020 11:58:26 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: emacs-devel@gnu.org
> 
> > Yes, it works! I tested it on master so I think it should be backported
> > to Emacs-27
> 
> Thanks!
> 
> Is it really important enough for Emacs 27?  It only happens in a file
> with one single function, and even then, if I understand it right, it
> doesn't prevent the user adding another function and carrying on with his
> work.
> 
> Anyhow, Eli would need to give his permission before we could add it to
> Emacs 27.

I might, if you make the change less different from the current code
(the 'length > 1' vs cddr).  If the change is just an addition, then
I'm probably okay with it.

Thanks.

Re: Error in C++ mode with Emacs 27.0.90

[Dmitry Gutov]

On 29.03.2020 14:50, Alan Mackenzie wrote:

> It seems unlikely that we can get a nested list (in imenu--index-alist)
> with only one element.  I have tried creating a C++ buffer with 25
> functions in it.  This gives a simple list of 25 elements.  With 26
> functions, I get a nested alist with two elements.

What's the worst that can happen in this case? We get a nested submenu 
with just one element?

> However, in an emacs-lisp-mode buffer, we get things like "Variables"
> and "*Rescan*", but I've not yet succeeded in causing a nested list with
> just one element.  I'm not convinced enough that it couldn't happen,
> though.
> 
> The latter part of the function looks like this (before my patch):
> 
>          (let* ((menu (imenu--split-menu index-alist
>                                          (buffer-name)))
>                 (menu1 (imenu--create-keymap (car menu)
> ========>                                   (cdr (if (< 1 (length (cdr menu)))
>                                                       menu
>                                                     (car (cdr menu))))
>                                              'imenu--menubar-select)))
>            (setcdr imenu--menubar-keymap (cdr menu1)))))))
> 
> That `if' form has been there since imenu-update-menubar was first
> written by Karl Heuer in 1997 (commit 0a8e8bc63e3).  Presumably, it
> really was needed back then.

If you're implying we can get rid of the check and always return (cdr 
menu), I say let's try.

>> In any case, imenu--create-keymap is also called from imenu--mouse-menu,
>> which probably needs the same kind of fix.
> 
> Possibly.  I've not managed to create the same error in
> imenu--mouse-menu,

   (setq imenu-use-popup-menu t)

followed by

   M-x imenu

did that for me in the example buffer.

> but perhaps it could do with the same correction,
> just in case.  What do you think?

Yup.

> Incidentally, that patch from last night was a bit untidy.  I think it
> needs cleaning up a bit.

So how about this one? In my very brief testing, it seems to work, both 
in the example buffer, and for imenu indices in several bigger files:

diff --git a/lisp/imenu.el b/lisp/imenu.el
index fb8b3de662..809ddd7ed1 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -814,9 +814,7 @@ imenu--mouse-menu
    (setq index-alist (imenu--split-submenus index-alist))
    (let* ((menu (imenu--split-menu index-alist (or title (buffer-name))))
  	 (map (imenu--create-keymap (car menu)
-				    (cdr (if (< 1 (length (cdr menu)))
-					     menu
-					   (car (cdr menu)))))))
+				    (cdr menu))))
      (popup-menu map event)))

  (defun imenu-choose-buffer-index (&optional prompt alist)
@@ -912,9 +910,7 @@ imenu-update-menubar
  	(let* ((menu (imenu--split-menu index-alist
                                          (buffer-name)))
                 (menu1 (imenu--create-keymap (car menu)
-					    (cdr (if (< 1 (length (cdr menu)))
-						     menu
-						   (car (cdr menu))))
+					    (cdr menu)
  					    'imenu--menubar-select)))
  	  (setcdr imenu--menubar-keymap (cdr menu1)))))))

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Eli.

On Sun, Mar 29, 2020 at 16:56:00 +0300, Eli Zaretskii wrote:
> > Date: Sun, 29 Mar 2020 11:58:26 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: emacs-devel@gnu.org

> > > Yes, it works! I tested it on master so I think it should be backported
> > > to Emacs-27

> > Thanks!

> > Is it really important enough for Emacs 27?  It only happens in a file
> > with one single function, and even then, if I understand it right, it
> > doesn't prevent the user adding another function and carrying on with his
> > work.

> > Anyhow, Eli would need to give his permission before we could add it to
> > Emacs 27.

> I might, if you make the change less different from the current code
> (the 'length > 1' vs cddr).  If the change is just an addition, then
> I'm probably okay with it.

OK, let's see!  The following patch, in place of the original patch, is
a minimal patch, although fundamentally the same as the one I first
posted here:



diff --git a/lisp/imenu.el b/lisp/imenu.el
index fb8b3de662..1949f2f48f 100644
--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -911,11 +911,15 @@ imenu-update-menubar
         (setq index-alist (imenu--split-submenus index-alist))
 	(let* ((menu (imenu--split-menu index-alist
                                         (buffer-name)))
-               (menu1 (imenu--create-keymap (car menu)
-					    (cdr (if (< 1 (length (cdr menu)))
-						     menu
-						   (car (cdr menu))))
-					    'imenu--menubar-select)))
+               (menu1 (imenu--create-keymap
+                       (car menu)
+		       (cdr (if (or (< 1 (length (cdr menu)))
+                                    ;; Have we a non-nested single entry?
+                                    (atom (cdadr menu))
+                                    (atom (cadadr menu)))
+				menu
+			      (car (cdr menu))))
+		       'imenu--menubar-select)))
 	  (setcdr imenu--menubar-keymap (cdr menu1)))))))
 
 (defun imenu--menubar-select (item)


Should I commit this to the emacs-27 release branch?

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Eli Zaretskii]

> Date: Mon, 30 Mar 2020 17:49:38 +0000
> Cc: angelo.g0@libero.it, emacs-devel@gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> From: Alan Mackenzie <acm@muc.de>
> 
> diff --git a/lisp/imenu.el b/lisp/imenu.el
> index fb8b3de662..1949f2f48f 100644
> --- a/lisp/imenu.el
> +++ b/lisp/imenu.el
> @@ -911,11 +911,15 @@ imenu-update-menubar
>          (setq index-alist (imenu--split-submenus index-alist))
>  	(let* ((menu (imenu--split-menu index-alist
>                                          (buffer-name)))
> -               (menu1 (imenu--create-keymap (car menu)
> -					    (cdr (if (< 1 (length (cdr menu)))
> -						     menu
> -						   (car (cdr menu))))
> -					    'imenu--menubar-select)))
> +               (menu1 (imenu--create-keymap
> +                       (car menu)
> +		       (cdr (if (or (< 1 (length (cdr menu)))
> +                                    ;; Have we a non-nested single entry?
> +                                    (atom (cdadr menu))
> +                                    (atom (cadadr menu)))
> +				menu
> +			      (car (cdr menu))))
> +		       'imenu--menubar-select)))
>  	  (setcdr imenu--menubar-keymap (cdr menu1)))))))
>  
>  (defun imenu--menubar-select (item)
> 
> 
> Should I commit this to the emacs-27 release branch?

Yes, please.

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Eli.

On Mon, Mar 30, 2020 at 21:36:14 +0300, Eli Zaretskii wrote:
> > Date: Mon, 30 Mar 2020 17:49:38 +0000
> > Cc: angelo.g0@libero.it, emacs-devel@gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> > From: Alan Mackenzie <acm@muc.de>

> > diff --git a/lisp/imenu.el b/lisp/imenu.el
> > index fb8b3de662..1949f2f48f 100644
> > --- a/lisp/imenu.el
> > +++ b/lisp/imenu.el
> > @@ -911,11 +911,15 @@ imenu-update-menubar
> >          (setq index-alist (imenu--split-submenus index-alist))
> >  	(let* ((menu (imenu--split-menu index-alist
> >                                          (buffer-name)))
> > -               (menu1 (imenu--create-keymap (car menu)
> > -					    (cdr (if (< 1 (length (cdr menu)))
> > -						     menu
> > -						   (car (cdr menu))))
> > -					    'imenu--menubar-select)))
> > +               (menu1 (imenu--create-keymap
> > +                       (car menu)
> > +		       (cdr (if (or (< 1 (length (cdr menu)))
> > +                                    ;; Have we a non-nested single entry?
> > +                                    (atom (cdadr menu))
> > +                                    (atom (cadadr menu)))
> > +				menu
> > +			      (car (cdr menu))))
> > +		       'imenu--menubar-select)))
> >  	  (setcdr imenu--menubar-keymap (cdr menu1)))))))

> >  (defun imenu--menubar-select (item)


> > Should I commit this to the emacs-27 release branch?

> Yes, please.

Done.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Dmitry.

On Mon, Mar 30, 2020 at 01:32:48 +0300, Dmitry Gutov wrote:
> On 29.03.2020 14:50, Alan Mackenzie wrote:

> > It seems unlikely that we can get a nested list (in imenu--index-alist)
> > with only one element.  I have tried creating a C++ buffer with 25
> > functions in it.  This gives a simple list of 25 elements.  With 26
> > functions, I get a nested alist with two elements.

> What's the worst that can happen in this case? We get a nested submenu 
> with just one element?

Probably.

> > However, in an emacs-lisp-mode buffer, we get things like "Variables"
> > and "*Rescan*", but I've not yet succeeded in causing a nested list with
> > just one element.  I'm not convinced enough that it couldn't happen,
> > though.

> > The latter part of the function looks like this (before my patch):

> >          (let* ((menu (imenu--split-menu index-alist
> >                                          (buffer-name)))
> >                 (menu1 (imenu--create-keymap (car menu)
> > ========>                                   (cdr (if (< 1 (length (cdr menu)))
> >                                                       menu
> >                                                     (car (cdr menu))))
> >                                              'imenu--menubar-select)))
> >            (setcdr imenu--menubar-keymap (cdr menu1)))))))

> > That `if' form has been there since imenu-update-menubar was first
> > written by Karl Heuer in 1997 (commit 0a8e8bc63e3).  Presumably, it
> > really was needed back then.

> If you're implying we can get rid of the check and always return (cdr 
> menu), I say let's try.

I think we can get rid of the check and just use (cdr menu), yes.  In
master, that is.

> >> In any case, imenu--create-keymap is also called from imenu--mouse-menu,
> >> which probably needs the same kind of fix.

> > Possibly.  I've not managed to create the same error in
> > imenu--mouse-menu,

>    (setq imenu-use-popup-menu t)

> followed by

>    M-x imenu

> did that for me in the example buffer.

Yes, thanks.  For me, too.

> > but perhaps it could do with the same correction,
> > just in case.  What do you think?

> Yup.

Sorry it didn't get into my patch for emacs-27.

> > Incidentally, that patch from last night was a bit untidy.  I think it
> > needs cleaning up a bit.

> So how about this one? In my very brief testing, it seems to work, both 
> in the example buffer, and for imenu indices in several bigger files:

> diff --git a/lisp/imenu.el b/lisp/imenu.el
> index fb8b3de662..809ddd7ed1 100644
> --- a/lisp/imenu.el
> +++ b/lisp/imenu.el
> @@ -814,9 +814,7 @@ imenu--mouse-menu
>     (setq index-alist (imenu--split-submenus index-alist))
>     (let* ((menu (imenu--split-menu index-alist (or title (buffer-name))))
>   	 (map (imenu--create-keymap (car menu)
> -				    (cdr (if (< 1 (length (cdr menu)))
> -					     menu
> -					   (car (cdr menu)))))))
> +				    (cdr menu))))
>       (popup-menu map event)))

>   (defun imenu-choose-buffer-index (&optional prompt alist)
> @@ -912,9 +910,7 @@ imenu-update-menubar
>   	(let* ((menu (imenu--split-menu index-alist
>                                           (buffer-name)))
>                  (menu1 (imenu--create-keymap (car menu)
> -					    (cdr (if (< 1 (length (cdr menu)))
> -						     menu
> -						   (car (cdr menu))))
> +					    (cdr menu)
>   					    'imenu--menubar-select)))
>   	  (setcdr imenu--menubar-keymap (cdr menu1)))))))

I think this would be a good patch for master.  I don't think Eli would
let it into the release branch, though I may be wrong.

Maybe for the release branch he would let in a patch like the one I've
already applied.

imenu is _complicated_.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Error in C++ mode with Emacs 27.0.90

[Dmitry Gutov]

On 04.04.2020 14:46, Alan Mackenzie wrote:

>>> It seems unlikely that we can get a nested list (in imenu--index-alist)
>>> with only one element.  I have tried creating a C++ buffer with 25
>>> functions in it.  This gives a simple list of 25 elements.  With 26
>>> functions, I get a nested alist with two elements.
> 
>> What's the worst that can happen in this case? We get a nested submenu
>> with just one element?
> 
> Probably.

Doesn't sound too scary.

>> If you're implying we can get rid of the check and always return (cdr
>> menu), I say let's try.
> 
> I think we can get rid of the check and just use (cdr menu), yes.  In
> master, that is.

Ok.

> Maybe for the release branch he would let in a patch like the one I've
> already applied.

Would you like to propose one? Given that we'd be copying your code anyway.

> imenu is _complicated_.

I think it's just overdoing with its list-of-lists approach, instead of 
proper structs. And it seems like we're dealing with extraneous code 
anyway (maybe even a premature optimization of sorts).

Re: Error in C++ mode with Emacs 27.0.90

[Alan Mackenzie]

Hello, Dmitry.

Forgive me for top-posting, but it's nearly three years ago since you
wrote this last post, so I'm not going to chop it up before you've got
the context back again.

The issue is in imenu, there are certain special cases for calling
imenu--create-keymap, and my committed patch fixed one (of two) of the
places where this special casing happened.

You proposed a patch that would just use (cdr menu) in both places, and
reported that that seems to work.  I've tried this patch out just now,
and doing this omits the entry *Rescan*.  I haven't yet worked out
precisely why, but it seems the patch needs some refinement.

The recipe you gave to throw an error ((setq imenu-use-popup-menu t)
etc.) still throws an error.

Anyhow, I think it's high time this bug was put to bed, so I intend
working on it in the next few days.  Clearly this will be a fix for the
master branch.

-- 
Alan Mackenzie (Nuremberg, Germany).


On Mon, Mar 30, 2020 at 01:32:48 +0300, Dmitry Gutov wrote:
> On 29.03.2020 14:50, Alan Mackenzie wrote:

> > It seems unlikely that we can get a nested list (in imenu--index-alist)
> > with only one element.  I have tried creating a C++ buffer with 25
> > functions in it.  This gives a simple list of 25 elements.  With 26
> > functions, I get a nested alist with two elements.

> What's the worst that can happen in this case? We get a nested submenu 
> with just one element?

> > However, in an emacs-lisp-mode buffer, we get things like "Variables"
> > and "*Rescan*", but I've not yet succeeded in causing a nested list with
> > just one element.  I'm not convinced enough that it couldn't happen,
> > though.
> > 
> > The latter part of the function looks like this (before my patch):
> > 
> >          (let* ((menu (imenu--split-menu index-alist
> >                                          (buffer-name)))
> >                 (menu1 (imenu--create-keymap (car menu)
> > ========>                                   (cdr (if (< 1 (length (cdr menu)))
> >                                                       menu
> >                                                     (car (cdr menu))))
> >                                              'imenu--menubar-select)))
> >            (setcdr imenu--menubar-keymap (cdr menu1)))))))
> > 
> > That `if' form has been there since imenu-update-menubar was first
> > written by Karl Heuer in 1997 (commit 0a8e8bc63e3).  Presumably, it
> > really was needed back then.

> If you're implying we can get rid of the check and always return (cdr 
> menu), I say let's try.

> >> In any case, imenu--create-keymap is also called from imenu--mouse-menu,
> >> which probably needs the same kind of fix.
> > 
> > Possibly.  I've not managed to create the same error in
> > imenu--mouse-menu,

>    (setq imenu-use-popup-menu t)

> followed by

>    M-x imenu

> did that for me in the example buffer.

> > but perhaps it could do with the same correction,
> > just in case.  What do you think?

> Yup.

> > Incidentally, that patch from last night was a bit untidy.  I think it
> > needs cleaning up a bit.

> So how about this one? In my very brief testing, it seems to work, both 
> in the example buffer, and for imenu indices in several bigger files:

> diff --git a/lisp/imenu.el b/lisp/imenu.el
> index fb8b3de662..809ddd7ed1 100644
> --- a/lisp/imenu.el
> +++ b/lisp/imenu.el
> @@ -814,9 +814,7 @@ imenu--mouse-menu
>     (setq index-alist (imenu--split-submenus index-alist))
>     (let* ((menu (imenu--split-menu index-alist (or title (buffer-name))))
>   	 (map (imenu--create-keymap (car menu)
> -				    (cdr (if (< 1 (length (cdr menu)))
> -					     menu
> -					   (car (cdr menu)))))))
> +				    (cdr menu))))
>       (popup-menu map event)))

>   (defun imenu-choose-buffer-index (&optional prompt alist)
> @@ -912,9 +910,7 @@ imenu-update-menubar
>   	(let* ((menu (imenu--split-menu index-alist
>                                           (buffer-name)))
>                  (menu1 (imenu--create-keymap (car menu)
> -					    (cdr (if (< 1 (length (cdr menu)))
> -						     menu
> -						   (car (cdr menu))))
> +					    (cdr menu)
>   					    'imenu--menubar-select)))
>   	  (setcdr imenu--menubar-keymap (cdr menu1)))))))