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).