Regression in `imenu--mouse-menu'

Regression in `imenu--mouse-menu'

[David PONCE]

Hi,

As indicated in its doc string the function `imenu--mouse-menu' should
"Returns t for rescan and otherwise an element or subelement of
INDEX-ALIST.".

I checked that it works as expected in both Emacs 20.7 and 21.3.

Using the CVS HEAD version of Emacs, `imenu--mouse-menu' always
returns nil, and unconditionally moves the cursor to the position in
the cdr of the selected item.

The following simple test illustrates that:

emacs -q --no-site-file

(require 'imenu)
(imenu--mouse-menu
 '(
   ("a" . a)
   ("b" . b)
   ("c" . c)
   ("d" . 1)
   ("e" . e)
   ("f" . f)
   )
 t)

In both Emacs 20.7 and 21.3, when I execute the above code and select,
for example, the menu item "c", `imenu--mouse-menu' correctly returns
the element ("c" . c).

The same code run in Emacs HEAD fails with the following error:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p c)
  imenu-default-goto-function("c" c)
  apply(imenu-default-goto-function "c" c nil)
  imenu(("c" . c))
  imenu--menubar-select(("c" . c))

When I select the menu item "d", Emacs 20.7 and 21.3 return ("d" . 1),
Emacs HEAD returns nil and moves the point to 1.

That new behavior is quite annoying and breaks existing code (for
example, in the JDEE and Semantic, that use `imenu--menubar-select'
to display (possibly large) completion menus, to benefit of the nice
automatic menu split done by imenu).

In case it helps, After replacing the code of `imenu--menubar-select'
in HEAD by the 21.3 version, the function works again as expected.

Sincerely,
David

Re: Regression in `imenu--mouse-menu'

[Stefan Monnier]

> As indicated in its doc string the function `imenu--mouse-menu' should
> "Returns t for rescan and otherwise an element or subelement of
> INDEX-ALIST.".

Alright, you got me: I failed to update the docstring.

> That new behavior is quite annoying and breaks existing code (for
> example, in the JDEE and Semantic, that use `imenu--menubar-select'
> to display (possibly large) completion menus, to benefit of the nice
> automatic menu split done by imenu).

The `--' in the name is intended to make it clear that it's
an internal function.

Anyway, the patch below should fix it.  Will be applied shortly,


	Stefan


--- imenu.el.~1.93.~	Tue May  6 11:13:49 2003
+++ imenu.el	Fri May 16 16:50:32 2003
@@ -634,7 +634,7 @@
 	alist)
        t))
 
-(defun imenu--create-keymap-1 (title alist)
+(defun* imenu--create-keymap (title alist &optional (cmd 'identity))
   (let ((counter 0))
     (list* 'keymap title
 	   (mapcar
@@ -642,10 +642,9 @@
 	      (list* (car item) (car item)
 		     (cond
 		      ((imenu--subalist-p item)
-		       (imenu--create-keymap-1 (car item) (cdr item)))
+		       (imenu--create-keymap (car item) (cdr item) cmd))
 		      (t
-		       `(lambda () (interactive)
-			  (imenu--menubar-select ',item))))))
+		       `(lambda () (interactive) (,cmd ',item))))))
 	    alist))))
 
 (defun imenu--in-alist (str alist)
@@ -905,10 +904,10 @@
 Returns t for rescan and otherwise an element or subelement of INDEX-ALIST."
   (setq index-alist (imenu--split-submenus index-alist))
   (let* ((menu (imenu--split-menu index-alist (or title (buffer-name))))
-	 (map (imenu--create-keymap-1 (car menu)
-				      (if (< 1 (length (cdr menu)))
-					  (cdr menu)
-					(cdr (car (cdr menu)))))))
+	 (map (imenu--create-keymap (car menu)
+				    (cdr (if (< 1 (length (cdr menu)))
+					     menu
+					   (car (cdr menu)))))))
     (popup-menu map event)))
 
 (defun imenu-choose-buffer-index (&optional prompt alist)
@@ -1001,10 +1000,11 @@
 	  (setq index-alist (imenu--split-submenus index-alist))
 	  (setq menu (imenu--split-menu index-alist
 					(buffer-name)))
-	  (setq menu1 (imenu--create-keymap-1 (car menu)
-					      (if (< 1 (length (cdr menu)))
-						  (cdr menu)
-						(cdr (car (cdr menu))))))
+	  (setq menu1 (imenu--create-keymap (car menu)
+					    (cdr (if (< 1 (length (cdr menu)))
+						     menu
+						   (car (cdr menu))))
+					    'imenu--menubar-select))
 	  (setq old (lookup-key (current-local-map) [menu-bar index]))
 	  (setcdr old (cdr menu1)))))))

Re: Regression in `imenu--mouse-menu'

[David Ponce]

Hi Stefan,

>>That new behavior is quite annoying and breaks existing code (for
>>example, in the JDEE and Semantic, that use `imenu--menubar-select'
                                                ^^^^^^^^^^^^^^^^^^^^^^
                                  Oops! I meant imenu--mouse-menu
>>to display (possibly large) completion menus, to benefit of the nice
>>automatic menu split done by imenu).
> 
> 
> The `--' in the name is intended to make it clear that it's
> an internal function.

However it would be nice to make public a such useful function, to popup
a menu from an alist, and return the selected element.  And that
automagically splits a large menu into sub-menus ;-)

> Anyway, the patch below should fix it.  Will be applied shortly,

Thanks for the quick reply!   Your patch fixed the problem :-)

David