patch to imenu.el and lisp-mode.el for better menu in Lisp modes

patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Drew Adams]

Below are patches to imenu.el and lisp-mode.el. They divide Lisp definitions
in the Index imenu into submenus. For Emacs Lisp, the submenus are: `Faces',
`User Options', (other) `Variables', `Functions', `Macros', `Keys', `Keys in
Maps', and `Other', the last being a catch-all. For other Lisps, the
submenus are: `Variables', `Functions', `Types', and `Other'.

The essential changes are these:

lisp-mode.el -

1. Factored `lisp-imenu-generic-expression' into submenus.

2. Added variables `lisp-imenu-var-defn-regexp',
`lisp-imenu-fn-defn-regexp', `lisp-imenu-type-defn-regexp',
`lisp-imenu-macro-defn-regexp', `lisp-imenu-other-defn-regexp',
`emacs-lisp-imenu-face-defn-regexp', `emacs-lisp-imenu-option-defn-regexp',
`emacs-lisp-imenu-key-defn-regexp1', `emacs-lisp-imenu-key-defn-regexp2',
`emacs-lisp-imenu-generic-expression'.

3. In `emacs-lisp-mode', set `imenu-generic-expression' to
`emacs-lisp-imenu-generic-expression'.


imenu.el -

1. Added functions `imenu-toggle-sort' (with alias `toggle-imenu-sort') and
`imenu--sort-submenu'. Added variable `imenu-last-sort-function'.

2. `imenu-sort-function': Changed default value from nil to
`imenu--sort-by-name'. Mention `toggle-imenu-sort' in doc string.

3. `imenu-mouse-menu': Sort each submenu before, instead of after, splitting
submenus. Mention TITLE arg in doc string.

4. `imenu-update-menubar': Sort each submenu before, instead of after,
splitting submenus. Added doc string.


Notes/questions:

1. Question: Isn't `imenu-generic-expression' in the wrong section of
`imenu.el'? It is not a customizable variable, but it is in the
`Customizable variables' section.

2. Note: `defgroup' definitions are indexed in both the `Faces' submenu and
the `Options' submenu. I think this makes more sense than putting them in
only one or the other or putting them in submenu `Other'.

3. To do: add toggling of sorting to the `Index' menu as a `*Toggle
Sorting*' item, at the end with the `*Rescan*' item, for easy access. Would
someone like to do this?


------------8<----------- lisp-mode.el patch ----------------

*** lisp-mode-CVS-2007-07-13.el	Fri Jul 13 17:45:54 2007
--- lisp-mode-CVS-patched-2007-07-13.el	Fri Jul 13 19:03:18 2007
***************
*** 88,129 ****

  (define-abbrev-table 'lisp-mode-abbrev-table ())

! (defvar lisp-imenu-generic-expression
!   (list
!    (list nil
  	 (purecopy (concat "^\\s-*("
  			   (eval-when-compile
  			     (regexp-opt
! 			      '("defun" "defun*" "defsubst" "defmacro"
! 				"defadvice" "define-skeleton"
! 				"define-minor-mode" "define-global-minor-mode"
! 				"define-globalized-minor-mode"
! 				"define-derived-mode" "define-generic-mode"
! 				"define-compiler-macro" "define-modify-macro"
! 				"defsetf" "define-setf-expander"
! 				"define-method-combination"
! 				"defgeneric" "defmethod") t))
  			   "\\s-+\\(\\(\\sw\\|\\s_\\)+\\)"))
! 	 2)
!    (list (purecopy "Variables")
  	 (purecopy (concat "^\\s-*("
  			   (eval-when-compile
! 			     (regexp-opt
! 			      '("defvar" "defconst" "defconstant" "defcustom"
! 				"defparameter" "define-symbol-macro") t))
! 			   "\\s-+\\(\\(\\sw\\|\\s_\\)+\\)"))
! 	 2)
!    (list (purecopy "Types")
  	 (purecopy (concat "^\\s-*("
  			   (eval-when-compile
  			     (regexp-opt
! 			      '("defgroup" "deftheme" "deftype" "defstruct"
! 				"defclass" "define-condition" "define-widget"
! 				"defface" "defpackage") t))
! 			   "\\s-+'?\\(\\(\\sw\\|\\s_\\)+\\)"))
! 	 2))

-   "Imenu generic expression for Lisp mode.  See
`imenu-generic-expression'.")

  ;; This was originally in autoload.el and is still used there.
  (put 'autoload 'doc-string-elt 3)
--- 88,184 ----

  (define-abbrev-table 'lisp-mode-abbrev-table ())

! (defvar lisp-imenu-other-defn-regexp
    (purecopy (concat "^\\s-*("
                      (eval-when-compile
                       (regexp-opt
!                       '("define-condition" "define-widget" "defpackage"
"deftheme") t))
                      "\\s-+\\(\\(\\sw\\|\\s_\\)+\\)"))
!   "*Regexp that recognizes other (miscellaneous) Lisp definitions.")
!
! (defvar lisp-imenu-macro-defn-regexp
!   (purecopy (concat "\\s-*("
!                     (eval-when-compile
!                      (regexp-opt '("defmacro" "define-compiler-macro"
"define-modify-macro"
!                                    "define-symbol-macro") t))
!                     "\\s-+\\([^ \t()]+\\)"))
!   "*Regexp that recognizes Lisp macro definitions.")
!
! (defvar lisp-imenu-type-defn-regexp
    (purecopy (concat "^\\s-*("
                      (eval-when-compile
!                      (regexp-opt '("deftype" "defstruct" "defclass") t))
!                     "\\s-+'?\\(\\(\\sw\\|\\s_\\)+\\)"))
!   "*Regexp that recognizes Lisp type definitions.")
!
! (defvar lisp-imenu-fn-defn-regexp
    (purecopy (concat "^\\s-*("
                      (eval-when-compile
                       (regexp-opt
!                       '("defun" "defun*" "defsubst" "defadvice"
"define-skeleton"
!                         "define-minor-mode" "define-global-minor-mode"
"define-derived-mode"
!                         "define-generic-mode" "defsetf"
"define-setf-expander"
!                         "define-method-combination" "defgeneric"
"defmethod") t))
!                     "\\s-+\\(\\sw\\(\\sw\\|\\s_\\)+\\)"))
!   "*Regexp that recognizes Lisp function definitions.")
!
! (defvar lisp-imenu-var-defn-regexp
!   (purecopy (concat "^\\s-*("
!                     (eval-when-compile
!                      (regexp-opt '("defvar" "defconst" "defconstant"
"defparameter") t))
!                     "\\s-+\\(\\(\\sw\\|\\s_\\)+\\)"))
!   "*Regexp that recognizes global Lisp variable definitions.
! This does not include user options (defcustom).")
!
! (defvar lisp-imenu-generic-expression
!   (list
!    (list (purecopy "Other") lisp-imenu-other-defn-regexp 2)
!    (list (purecopy "Macros") lisp-imenu-macro-defn-regexp 2)
!    (list (purecopy "Types") lisp-imenu-type-defn-regexp 2)
!    (list (purecopy "Functions") lisp-imenu-fn-defn-regexp 2)
!    (list (purecopy "Variables") lisp-imenu-var-defn-regexp 2)
!    )
!   "*Imenu generic expression for Lisp mode.
! See `imenu-generic-expression'.")
!
! (defvar emacs-lisp-imenu-key-defn-regexp-1
!   (purecopy (concat "\\s-*("
!                     (eval-when-compile
!                      (regexp-opt '("global-set-key" "local-set-key"
"global-unset-key"
!                                    "local-unset-key"
"undefine-keys-bound-to") t))
!                     "\\s-*\\(\"[^\"]+\"\\|[[][^]]+[]]\\)"))
!   "*Regexp that recognizes Emacs key definitions.
! See also `emacs-lisp-imenu-key-defn-regexp-2'.")
!
! (defvar emacs-lisp-imenu-key-defn-regexp-2
!   (purecopy
"(\\s-*\\(define-key\\(-after\\)?\\s-+\\|substitute-key-definition\\s-+'\\)\
! \\(\\S-+\\)\\s-*'?\\(\"[^\"]+\"\\|[[][^]]+[]]\\)")
!   "*Regexp that recognizes Emacs key definitions in specific maps.
! See also `emacs-lisp-imenu-key-defn-regexp-1'.")
!
! (defvar emacs-lisp-imenu-face-defn-regexp
!   (purecopy "(\\s-*\\(defface\\|defgroup\\)\\s-+\\([^ \t()]+\\)")
!   "*Regexp for Emacs face definitions (defface).")
!
! (defvar emacs-lisp-imenu-option-defn-regexp
!   (purecopy "(\\s-*\\(defcustom\\|defgroup\\)\\s-+\\([^ \t()]+\\)")
!   "*Regexp for Emacs user option definitions (defcustom).")
!
! (defvar emacs-lisp-imenu-generic-expression
!   (list
!    (list "Other" lisp-imenu-other-defn-regexp 2)
!    (list "Keys in Maps" emacs-lisp-imenu-key-defn-regexp-2 4)
!    (list "Keys" emacs-lisp-imenu-key-defn-regexp-1 4)
!    (list "Macros" lisp-imenu-macro-defn-regexp 2)
!    (list "Functions" lisp-imenu-fn-defn-regexp 2)
!    (list "Variables" lisp-imenu-var-defn-regexp 2)
!    (list "User Options" emacs-lisp-imenu-option-defn-regexp 2)
!    (list "Faces" emacs-lisp-imenu-face-defn-regexp 2)
!    )
!   "*Imenu generic expression for Emacs Lisp mode.
! See `imenu-generic-expression'.")
!


  ;; This was originally in autoload.el and is still used there.
  (put 'autoload 'doc-string-elt 3)
***************
*** 359,364 ****
--- 414,421 ----
    (setq major-mode 'emacs-lisp-mode)
    (setq mode-name "Emacs-Lisp")
    (lisp-mode-variables)
+   (make-local-variable 'imenu-generic-expression)
+   (setq imenu-generic-expression emacs-lisp-imenu-generic-expression)
    (setq imenu-case-fold-search nil)
    (run-mode-hooks 'emacs-lisp-mode-hook))
  (put 'emacs-lisp-mode 'custom-mode-group 'lisp)




------------8<------------- imenu.el patch ------------------

*** imenu-CVS-2007-07-13.el	Fri Jul 13 15:39:26 2007
--- imenu-CVS-patched-2007-07-13.el	Fri Jul 13 19:19:46 2007
***************
*** 138,144 ****
    :group 'imenu)

  ;;;###autoload
! (defcustom imenu-sort-function nil
    "*The function to use for sorting the index mouse-menu.

  Affects only the mouse index menu.
--- 138,144 ----
    :group 'imenu)

  ;;;###autoload
! (defcustom imenu-sort-function 'imenu--sort-by-name
    "*The function to use for sorting the index mouse-menu.

  Affects only the mouse index menu.
***************
*** 151,157 ****

  The function should take two arguments and return t if the first
  element should come before the second.  The arguments are cons cells;
! \(NAME . POSITION).  Look at `imenu--sort-by-name' for an example."
    :type '(choice (const :tag "No sorting" nil)
  		 (const :tag "Sort by name" imenu--sort-by-name)
  		 (function :tag "Another function"))
--- 151,160 ----

  The function should take two arguments and return t if the first
  element should come before the second.  The arguments are cons cells;
! \(NAME . POSITION).  Look at `imenu--sort-by-name' for an example.
!
! You can toggle this value (without saving it) at any time, using
! command `toggle-imenu-sort'."
    :type '(choice (const :tag "No sorting" nil)
  		 (const :tag "Sort by name" imenu--sort-by-name)
  		 (function :tag "Another function"))
***************
*** 188,193 ****
--- 191,197 ----
    :type 'string
    :group 'imenu)

+ ;; Why does this belong here? It's not a customizable variable.
  ;;;###autoload
  (defvar imenu-generic-expression nil
    "The regex pattern to use for creating a buffer index.
***************
*** 263,269 ****
  ;;;###autoload
  (make-variable-buffer-local 'imenu-default-goto-function)

-
  (defun imenu--subalist-p (item)
    (and (consp (cdr item)) (listp (cadr item))
         (not (eq (car (cadr item)) 'lambda))))
--- 267,272 ----
***************
*** 420,425 ****
--- 423,431 ----
  ;;;
  ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

+ (defvar imenu-last-sort-function nil
+   "The last non-nil value for `imenu-sort-function' during this session.")
+
  ;; The item to use in the index for rescanning the buffer.
  (defconst imenu--rescan-item '("*Rescan*" . -99))

***************
*** 885,895 ****

  (defun imenu--mouse-menu (index-alist event &optional title)
    "Let the user select from a buffer index from a mouse menu.
!
! INDEX-ALIST is the buffer index and EVENT is a mouse event.
!
! 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 (car menu)
  				    (cdr (if (< 1 (length (cdr menu)))
--- 891,906 ----

  (defun imenu--mouse-menu (index-alist event &optional title)
    "Let the user select from a buffer index from a mouse menu.
! INDEX-ALIST is the buffer index.
! EVENT is a mouse event.
! TITLE is the menu title.
! Returns t for rescan, or else an element or subelement of INDEX-ALIST."
!   (setq index-alist
!         (imenu--split-submenus
!          (if imenu-sort-function
!              (mapcar (lambda (sm) (imenu--sort-submenu sm
imenu-sort-function))
!                      index-alist)
!            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)))
***************
*** 897,902 ****
--- 908,922 ----
                                             (car (cdr menu)))))))
      (popup-menu map event)))

+ (defun imenu--sort-submenu (submenu predicate)
+   "Create an imenu SUBMENU, sorting with PREDICATE."
+   (let ((menu-name (car submenu))
+         (menu-items (cdr submenu)))
+     (cons menu-name (if (and (consp menu-items)
+                              (consp (cdr menu-items)))
+                         (sort menu-items predicate)
+                       menu-items))))
+
  (defun imenu-choose-buffer-index (&optional prompt alist)
    "Let the user select from a buffer index and return the chosen index.

***************
*** 972,977 ****
--- 992,998 ----
  (make-variable-buffer-local 'imenu-menubar-modified-tick)

  (defun imenu-update-menubar ()
+   "Update the imenu. Use as `menu-bar-update-hook'."
    (when (and (current-local-map)
  	     (keymapp (lookup-key (current-local-map) [menu-bar index]))
  	     (/= (buffer-chars-modified-tick) imenu-menubar-modified-tick))
***************
*** 982,990 ****
        (unless (equal index-alist imenu--last-menubar-index-alist)
  	(let (menu menu1 old)
  	  (setq imenu--last-menubar-index-alist index-alist)
! 	  (setq index-alist (imenu--split-submenus index-alist))
! 	  (setq menu (imenu--split-menu index-alist
! 					(buffer-name)))
  	  (setq menu1 (imenu--create-keymap (car menu)
  					    (cdr (if (< 1 (length (cdr menu)))
  						     menu
--- 1003,1015 ----
        (unless (equal index-alist imenu--last-menubar-index-alist)
          (let (menu menu1 old)
            (setq imenu--last-menubar-index-alist index-alist)
!           (setq index-alist
!                 (imenu--split-submenus
!                  (if imenu-sort-function
!                      (mapcar (lambda (sm) (imenu--sort-submenu sm
imenu-sort-function))
!                              index-alist)
!                    index-alist)))
!           (setq menu (imenu--split-menu index-alist (buffer-name)))
            (setq menu1 (imenu--create-keymap (car menu)
                                              (cdr (if (< 1 (length (cdr
menu)))
                                                       menu
***************
*** 1020,1025 ****
--- 1045,1069 ----
    (goto-char position))

  ;;;###autoload
+ (defalias 'toggle-imenu-sort 'imenu-toggle-sort)
+ (defun imenu-toggle-sort (force-p)
+   "Toggle imenu between sorting menus and not.
+ Non-nil prefix FORCE-P => Sort iff FORCE-P >= 0."
+   (interactive "P")
+   (cond (imenu-sort-function
+          (setq imenu-last-sort-function imenu-sort-function) ; Save it.
+          (when (or (null force-p) (<= (prefix-numeric-value force-p) 0))
+            (setq imenu-sort-function nil))) ; Don't sort.
+         ((or (null force-p) (> (prefix-numeric-value force-p) 0)) ; Ask to
sort
+          (if imenu-last-sort-function  ; Sort using saved sort fn.
+              (setq imenu-sort-function imenu-last-sort-function)
+            (error "You first need to set `imenu-sort-function'"))))
+   (imenu--menubar-select imenu--rescan-item)
+   (if imenu-sort-function
+       (message "Imenus are now being sorted via `%s'."
imenu-sort-function)
+     (message "Imenus are in buffer order (not sorted).")))
+
+ ;;;###autoload
  (defun imenu (index-item)
    "Jump to a place in the buffer chosen using a buffer menu or mouse menu.
  INDEX-ITEM specifies the position.  See `imenu-choose-buffer-index'

Re: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Stefan Monnier]

> Below are patches to imenu.el and lisp-mode.el. They divide Lisp definitions
> in the Index imenu into submenus.

Please be careful: only the lisp-mode.el patch does what you say.
The imenu.el one does something different.

When you post such suggestions, please don't mix things up that way.
And clearly distinguish between bug-fixes (which are usually
non-controversial and might even be installed without any prior discussion),
new features, and changes to existing defaults (this latter one is usually
more sensitive so you will probably want to be more careful and make it
clear that the rest of the suggested changes do not depend on it).


        Stefan

Re: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Richard Stallman]

Could people please try these patches and say whether they like
the results?  If people here generally like them, we should
install them.

Re: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Richard Stallman]

    1. Question: Isn't `imenu-generic-expression' in the wrong section of
    `imenu.el'? It is not a customizable variable, but it is in the
    `Customizable variables' section.

Yes, it should be moved.

    2. Note: `defgroup' definitions are indexed in both the `Faces' submenu and
    the `Options' submenu.

I don't understand the issue.

RE: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Drew Adams]

> > Below are patches to imenu.el and lisp-mode.el. They divide
> > Lisp definitions in the Index imenu into submenus.
>
> Please be careful: only the lisp-mode.el patch does what you say.
> The imenu.el one does something different.
> When you post such suggestions, please don't mix things up that way.

I thought when I posted them that both patches were necessary to provide the
submenus for Lisp. You're right that the imenu.el changes are not needed for
that.

> And clearly distinguish between bug-fixes (which are usually
> non-controversial and might even be installed without any prior
> discussion),

I didn't intend or mention any bug fixes.

Do you mean sorting the submenus prior to splitting instead of afterward? To
be honest, I don't remember why I made that change (I made it in 1999 in my
own code). I tried it both ways just now, and the only difference I can see
is that *Rescan* is placed (correctly, IMO) at the end of the menu with my
version, instead of at the beginning. I don't think that in itself was why I
made that change, but I don't remember the reason, and I can't see any other
difference now. I even compared the change to the vanilla Emacs 20 behavior
just now, but that seems no different from the Emacs 22 behavior in this
respect. Maybe I'm low on caffeine...

Unless someone has an idea why sorting prior to splitting would be
preferable, you can ignore the imenu.el patch.

> new features, and changes to existing defaults (this latter one is usually
> more sensitive so you will probably want to be more careful and make it
> clear that the rest of the suggested changes do not depend on it).

I called attention to the change in default value of `imenu-sort-function'.

Yes, the rest of the changes do not strictly depend on the default value of
`imenu-sort-function'. However, if definitions are classified by type
(submenu), then I think it makes more sense to sort the names.

The change in default value affects more than just Lisp buffers, but I think
it still makes sense for the default behavior to sort index entries. That
might also encourage implementors of `imenu-generic-expression' for other
languages to classify definitions into submenus.

Let me withdraw the imenu.el patch, unless someone sees a good reason to
sort before splitting, and instead propose only that we change the default
value of `imenu-sort-function' from nil (no sorting) to
`imenu--sort-by-name'.

I also think it would be a good idea to add a command to toggle sorting (and
add it to the menu, like *Rescan*), but you can ignore that too.

Another change we might like to make is to the default value of
`emacs-lisp-mode-hook'. Instead of nil, I'd propose
(imenu-add-menubar-index). I'm not sure how many users will look to
customizing the mode hook to find that they can easily have an index menu
that helps them access definitions.

Thanks for making me look at the patches a little closer.

RE: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Drew Adams]

>     2. Note: `defgroup' definitions are indexed in both the
>        `Faces' submenu and the `Options' submenu.  I think
>        this makes more sense than putting them in only one
>        or the other or putting them in submenu `Other'.
>
> I don't understand the issue.

No issue. I just wanted to point out where I put `defgroup', in case others
feel differently. The choices I see are in `Faces', `User Options', or
`Other'.

Re: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Richard Stallman]

    No issue. I just wanted to point out where I put `defgroup', in case others
    feel differently. The choices I see are in `Faces', `User Options', or
    `Other'.

Which defgroup(s) are you talking about?
What source file are they located in?

RE: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Drew Adams]

>     No issue. I just wanted to point out where I put `defgroup',
>     in case others feel differently. The choices I see are in
>     `Faces', `User Options', or `Other'.
>
> Which defgroup(s) are you talking about?
> What source file are they located in?

Sorry, I really don't know what you're talking about.

This is about putting Lisp definitions in an Imenu index menu. There are
definitions of functions (`defun'), variables (`defvar'), and so on. There
are also definitions of custom groups (introduced by `defgroup'). The
question is where in the menu to put such group-definition entries. The
patch I sent puts them in both the `Faces' submenu and the `User Options'
submenu.

Please elaborate on the question if this doesn't clear it up.

Re: patch to imenu.el and lisp-mode.el for better menu in Lisp modes

[Richard Stallman]

    This is about putting Lisp definitions in an Imenu index menu. There are
    definitions of functions (`defun'), variables (`defvar'), and so on. There
    are also definitions of custom groups (introduced by `defgroup'). The
    question is where in the menu to put such group-definition entries. The
    patch I sent puts them in both the `Faces' submenu and the `User Options'
    submenu.

Now I understand the question.  Your choice sounds good to me.