* bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus
@ 2012-10-24 0:07 Drew Adams
2012-10-30 3:05 ` Stefan Monnier
0 siblings, 1 reply; 5+ messages in thread
From: Drew Adams @ 2012-10-24 0:07 UTC (permalink / raw)
To: 12717
Imenu allows the use of something it calls "special" menu elements,
which have this form: (INDEX-NAME POSITION FUNCTION ARGUMENTS...)
`imenu--split-submenus' needs to distinguish submenus from ordinary menu
elements, including from special menu elements. Currently this is
bugged.
The current code for `imenu--split-submenus' does this:
(defun imenu--split-submenus (alist)
"..."
(mapcar (lambda (elt)
(if (and (consp elt) (stringp (car elt)) (listp (cdr elt)))
(imenu--split-menu (cdr elt) (car elt))
elt))
alist))
The `if' condition should instead test whether the element is a submenu.
We have a function that does that: `imenu--subalist-p'. I believe this
is the correct code:
(defun imenu--split-submenus (alist)
"..."
(mapcar (lambda (elt)
(if (imenu--subalist-p elt)
(imenu--split-menu (cdr elt) (car elt))
elt))
alist))
In GNU Emacs 24.2.50.1 (i386-mingw-nt5.1.2600)
of 2012-10-22 on DANI-PC
Bzr revision: 110618 monnier@iro.umontreal.ca-20121022132928-232zm0fecassmhfb
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
`configure --with-gcc (4.7) --no-opt --enable-checking --cflags
-I../../libs/libxpm-3.5.8/include -I../../libs/libxpm-3.5.8/src
-I../../libs/libpng-1.4.10 -I../../libs/zlib-1.2.6
-I../../libs/giflib-4.1.4-1/include -I../../libs/jpeg-6b-4/include
-I../../libs/tiff-3.8.2-1/include
-I../../libs/libxml2-2.7.8-w32-bin/include/libxml2
-I../../libs/gnutls-3.0.16/include
-I../../libs/libiconv-1.14-2-mingw32-dev/include'
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus
2012-10-24 0:07 bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus Drew Adams
@ 2012-10-30 3:05 ` Stefan Monnier
2012-10-30 6:18 ` Drew Adams
2012-11-15 2:03 ` Stefan Monnier
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Monnier @ 2012-10-30 3:05 UTC (permalink / raw)
To: Drew Adams; +Cc: 12717
> (defun imenu--split-submenus (alist)
> "..."
> (mapcar (lambda (elt)
> (if (and (consp elt) (stringp (car elt)) (listp (cdr elt)))
> (imenu--split-menu (cdr elt) (car elt))
> elt))
> alist))
> The `if' condition should instead test whether the element is a submenu.
> We have a function that does that: `imenu--subalist-p'. I believe this
> is the correct code:
> (defun imenu--split-submenus (alist)
> "..."
> (mapcar (lambda (elt)
> (if (imenu--subalist-p elt)
> (imenu--split-menu (cdr elt) (car elt))
> elt))
> alist))
This looks eminently reasonable, except that I don't understand the code
enough to be sure (e.g. the current test includes (consp elt) whereas
imenu--split-menu starts right off by calling cdr). Do you happen to
have some kind of test case?
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus
2012-10-30 3:05 ` Stefan Monnier
@ 2012-10-30 6:18 ` Drew Adams
2012-10-30 6:33 ` Drew Adams
2012-11-15 2:03 ` Stefan Monnier
1 sibling, 1 reply; 5+ messages in thread
From: Drew Adams @ 2012-10-30 6:18 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 12717
> This looks eminently reasonable, except that I don't
> understand the code enough to be sure (e.g. the current test
> includes (consp elt) whereas imenu--split-menu starts right
> off by calling cdr). Do you happen to have some kind of test
> case?
I ran into the bug when trying to make use of so-called "special" menu items.
I add two general toggle items to the menus, along with the general item
*Rescan*. In doing that I discovered the bug.
This is the redefinition of `imenu--make-index-alist' that I use. It is here
that I add two menu items: one to toggle sorting on/off, and one to toggle
case-sensitivity of sorting when sorting by name.
;; REPLACE ORIGINAL in `imenu.el'.
;;
;; 1. Respect `ignore-comments-flag', if defined:
;; use `with-comments-hidden'.
;; 2. Add Imenu+ toggle commands to menu.
;;
(defun imenu--make-index-alist (&optional noerror)
"Create an index alist for the definitions in the current buffer.
This works by using the hook function `imenu-create-index-function'.
Report an error if the list is empty unless NOERROR is supplied and
non-nil.
If `ignore-comments-flag' is defined and non-nil, then respect it,
ignoring hidden comments.
See `imenu--index-alist' for the format of the index alist."
(or (and imenu--index-alist
(or (not imenu-auto-rescan)
(and imenu-auto-rescan
(> (buffer-size) imenu-auto-rescan-maxout))))
;; Get the index; truncate if necessary
(progn (setq imenu--index-alist
(save-excursion
(save-restriction
(widen)
(if (and (> emacs-major-version 20)
(require 'hide-comnt nil t))
(let ((search-invisible nil))
(with-comments-hidden
(point-min) (point-max)
(funcall imenu-create-index-function)))
(funcall imenu-create-index-function)))))
(imenu--truncate-items imenu--index-alist)))
(or imenu--index-alist
noerror
(user-error
"No items suitable for an index found in this buffer"))
(or imenu--index-alist (setq imenu--index-alist (list nil)))
(cons imenu--rescan-item ; `*Rescan*'.
(cons '("Toggle Case-Sensitive Name-Sort"
IGNORE
(lambda (&rest _ignore)
(imenup-toggle-case-sensitive-sorting)))
(cons '("Toggle Sorting"
IGNORE
(lambda (&rest _ignore)
(imenup-toggle-sort)))
imenu--index-alist))))
Clearly, adding general menu items this way is a make-do hack. I don't use the
POSITION, and the functions invoked don't use any of the args.
And I don't know how, using this kind of menu creation, to, for example, make
the case-sensitivity toggle item inactive when the sort method is not by name.
But I didn't find a better way to add ordinary, non-indexing menu items.
Anyway, that's how I ran into the bug.
The full code is here, FWIW:
http://www.emacswiki.org/emacs-en/download/imenu%2b.el
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus
2012-10-30 6:18 ` Drew Adams
@ 2012-10-30 6:33 ` Drew Adams
0 siblings, 0 replies; 5+ messages in thread
From: Drew Adams @ 2012-10-30 6:33 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 12717
Sorry, I was confused in my reply. This wasn't directly about
`imenu--make-index-alist', but about `imenu--split-submenus'. I made the fix
for that at the same time, for the same reason (to add the toggle menu items).
`imenu--subalist-p' does this:
(defun imenu--subalist-p (item)
(and (consp (cdr item)) (listp (cadr item))
(not (eq (car (cadr item)) 'lambda))))
I don't think it is necessary to check that the elements of the ALIST arg to
`imenu--split-submenus' are conses. But if you want to be sure that the `cdr'
in `imenu--subalist-p' doesn't cause a problem, then either add a `consp' test
before `imenu--sublist-p' or change the `cdr' to `cdr-safe'.
I'm pretty sure that the ALIST arg will have only cons elements, but I guess
there is no guarantee.
My guess is that the original test (and (consp elt) (stringp (car elt)) (listp
(cdr elt))) was just a feeble attempt to check for a sublmenu.
Anyway, now you know as much as I about the code, and can decide how you want to
fix it.
The point of the bug report is that the current definition ends up trying to
handle a "special" menu item as if it were a submenu: it is a consp with a
string car and listp cdr, but it is not a submenu. It ends up choking.
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus
2012-10-30 3:05 ` Stefan Monnier
2012-10-30 6:18 ` Drew Adams
@ 2012-11-15 2:03 ` Stefan Monnier
1 sibling, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2012-11-15 2:03 UTC (permalink / raw)
To: Drew Adams; +Cc: 12717-done
> This looks eminently reasonable, except that I don't understand the code
> enough to be sure (e.g. the current test includes (consp elt) whereas
> imenu--split-menu starts right off by calling cdr). Do you happen to
> have some kind of test case?
Well, I installed it in trunk, it looks safe enough there,
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-15 2:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-24 0:07 bug#12717: 24.2.50; [PATCH] `imenu--split-submenus' incorrectly distinguishes submenus Drew Adams
2012-10-30 3:05 ` Stefan Monnier
2012-10-30 6:18 ` Drew Adams
2012-10-30 6:33 ` Drew Adams
2012-11-15 2:03 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).