[PATCH] Fix imenu bug with shared list structure

[PATCH] Fix imenu bug with shared list structure

[Daniel Colascione]

Fixes a subtle bug caused by insufficient care being taken with shared list 
structure. It involves nested menu items being mysteriously deleted.


--- /dev/stdin	2008-09-25 16:51:54.840409301 -0400
+++ imenu.el	2008-09-25 16:51:36.000000000 -0400
@@ -483,8 +483,10 @@
 	(/ (1- pos) (max (/ total 100) 1))
       (/ (* 100 (1- pos)) (max total 1)))))
 
-;; Split LIST into sublists of max length N.
-;; Example (imenu--split '(1 2 3 4 5 6 7 8) 3)-> '((1 2 3) (4 5 6) (7 8))
+;;; Split LIST into sublists of max length N.
+;;; Example (imenu--split '(1 2 3 4 5 6 7 8) 3)-> '((1 2 3) (4 5 6) (7 8))
+;;;
+;;; The returned list DOES NOT share structure with LIST
 (defun imenu--split (list n)
   (let ((remain list)
 	(result '())
@@ -504,12 +506,17 @@
 	 (push (nreverse sublist) result))
     (nreverse result)))
 
-;;; Split the alist MENULIST into a nested alist, if it is long enough.
-;;; In any case, add TITLE to the front of the alist.
+;;; Split the alist MENULIST into a nested alist, if it is long
+;;; enough. In any case, add TITLE to the front of the alist.  If
+;;; IMENU--RESCAN-ITEM is present in MENULIST, it is moved to the
+;;; beginning of the returned alist.
+;;;
+;;; The returned alist DOES NOT share structure with MENULIST.
 (defun imenu--split-menu (menulist title)
-  (let (keep-at-top tail)
+  (let ((menulist (copy-sequence menulist))
+        keep-at-top tail)
     (if (memq imenu--rescan-item menulist)
-	(setq keep-at-top (cons imenu--rescan-item nil)
+	(setq keep-at-top (list imenu--rescan-item)
 	      menulist (delq imenu--rescan-item menulist)))
     (setq tail menulist)
     (dolist (item tail)
@@ -517,18 +524,22 @@
 	(push item keep-at-top)
 	(setq menulist (delq item menulist))))
     (if imenu-sort-function
-	(setq menulist (sort (copy-sequence menulist) imenu-sort-function)))
+	(setq menulist (sort menulist imenu-sort-function)))
     (if (> (length menulist) imenu-max-items)
 	(setq menulist
 	      (mapcar
 	       (lambda (menu)
 		 (cons (format "From: %s" (caar menu)) menu))
 	       (imenu--split menulist imenu-max-items))))
+
     (cons title
 	  (nconc (nreverse keep-at-top) menulist))))
 
 ;;; Split up each long alist that are nested within ALIST
 ;;; into nested alists.
+;;;
+;;; Return a split and sorted copy of ALIST. The returned alist DOES
+;;; NOT share structure with ALIST.
 (defun imenu--split-submenus (alist)
   (mapcar (function
 	   (lambda (elt)

Re: [PATCH] Fix imenu bug with shared list structure

[Stefan Monnier]

> Fixes a subtle bug caused by insufficient care being taken with shared list 
> structure. It involves nested menu items being mysteriously deleted.

Just a note about top-level comments: they should use ";;" rather than
";;;" because ";;;" is used for sectioning.  Lots of places use ";;;"
for historical reasons, but when you come across them, please change
them rather than imitate them.  Users of outline-minor-mode will be
thankful,


        Stefan


> --- /dev/stdin	2008-09-25 16:51:54.840409301 -0400
> +++ imenu.el	2008-09-25 16:51:36.000000000 -0400
> @@ -483,8 +483,10 @@
>  	(/ (1- pos) (max (/ total 100) 1))
>        (/ (* 100 (1- pos)) (max total 1)))))
 
> -;; Split LIST into sublists of max length N.
> -;; Example (imenu--split '(1 2 3 4 5 6 7 8) 3)-> '((1 2 3) (4 5 6) (7 8))
> +;;; Split LIST into sublists of max length N.
> +;;; Example (imenu--split '(1 2 3 4 5 6 7 8) 3)-> '((1 2 3) (4 5 6) (7 8))
> +;;;
> +;;; The returned list DOES NOT share structure with LIST
>  (defun imenu--split (list n)
>    (let ((remain list)
>  	(result '())
> @@ -504,12 +506,17 @@
>  	 (push (nreverse sublist) result))
>      (nreverse result)))
 
> -;;; Split the alist MENULIST into a nested alist, if it is long enough.
> -;;; In any case, add TITLE to the front of the alist.
> +;;; Split the alist MENULIST into a nested alist, if it is long
> +;;; enough. In any case, add TITLE to the front of the alist.  If
> +;;; IMENU--RESCAN-ITEM is present in MENULIST, it is moved to the
> +;;; beginning of the returned alist.
> +;;;
> +;;; The returned alist DOES NOT share structure with MENULIST.
>  (defun imenu--split-menu (menulist title)
> -  (let (keep-at-top tail)
> +  (let ((menulist (copy-sequence menulist))
> +        keep-at-top tail)
>      (if (memq imenu--rescan-item menulist)
> -	(setq keep-at-top (cons imenu--rescan-item nil)
> +	(setq keep-at-top (list imenu--rescan-item)
>  	      menulist (delq imenu--rescan-item menulist)))
>      (setq tail menulist)
>      (dolist (item tail)
> @@ -517,18 +524,22 @@
>  	(push item keep-at-top)
>  	(setq menulist (delq item menulist))))
>      (if imenu-sort-function
> -	(setq menulist (sort (copy-sequence menulist) imenu-sort-function)))
> +	(setq menulist (sort menulist imenu-sort-function)))
>      (if (> (length menulist) imenu-max-items)
>  	(setq menulist
>  	      (mapcar
>  	       (lambda (menu)
>  		 (cons (format "From: %s" (caar menu)) menu))
>  	       (imenu--split menulist imenu-max-items))))
> +
>      (cons title
>  	  (nconc (nreverse keep-at-top) menulist))))
 
>  ;;; Split up each long alist that are nested within ALIST
>  ;;; into nested alists.
> +;;;
> +;;; Return a split and sorted copy of ALIST. The returned alist DOES
> +;;; NOT share structure with ALIST.
>  (defun imenu--split-submenus (alist)
>    (mapcar (function
>  	   (lambda (elt)

Re: [PATCH] Fix imenu bug with shared list structure

[Daniel Colascione]

On Thursday 25 September 2008, Stefan Monnier wrote:
> Just a note about top-level comments: they should use ";;" rather than
> ";;;" because ";;;" is used for sectioning.  Lots of places use ";;;"
> for historical reasons, but when you come across them, please change
> them rather than imitate them.  Users of outline-minor-mode will be
> thankful,

Should I fix all of imenu.el and which-func.el?

Re: [PATCH] Fix imenu bug with shared list structure

[Glenn Morris]

Daniel Colascione wrote:

> Fixes a subtle bug caused by insufficient care being taken with shared list 
> structure. It involves nested menu items being mysteriously deleted.

Thanks; installed. An example is always nice, but I'll take your word
for it. Please also supply a ChangeLog entry in future.