|PATCH| describe-minor-mode and describe-minor-mode-from-indicator

|PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

Dear, Stefan Monnier

(I found your name on the MAINTAINERS file as a developer 
who has interests on minor-mode/major-mode infrastructure.
So I've put your name to To: field of this mail.)

I've created a prototype of describe-minor-mode.

Background: Few days ago, I found a strange string
on mode-line: "doe". I wonder what is "doe". However,
I cannot find a way to know what is doe. M-x describe-mode
tells me nothing about doe. Finally I have to do 
"grep doe emacs/lisp/*.el".

What I did: 
I think we need something help
functions for minor mode. I wrote 3 things:

1. M-x describe-minor-mode
   The user of describe-minor-mode can give its argument
   with completion. The completion table is built from the
   name of minor modes activated on the current buffer.

2. M-x describe-minor-mode-from-indicator
   Almost the same as describe-minor-mode but the user 
   can pass an indicator instead of minor mode name to
   get help. I could not implment completion.
   (You will be not satisfied with "without completion".)

3. I added "Describe `MinorMode'" menu item to "Minor Modes" 
   popup menu. The user can get help about a minor mode with mouse-3.

I believe these functions helps beginner of Emacs. In other word
the user might be confused with strange minor mode strings. 

Regards,
Masatake YAMATO

Index: help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.255
diff -u -r1.255 help.el
--- help.el	13 Jan 2003 08:04:46 -0000	1.255
+++ help.el	18 Jan 2003 08:19:27 -0000
@@ -611,6 +611,20 @@
 	  (setq minor-modes (cdr minor-modes))))
       (print-help-return-message))))
 
+(defun describe-minor-mode (minor-mode)
+  "Display documentation of a minor mode given as MINOR-MODE."
+  (interactive (list (intern (completing-read 
+			      "Minor mode: "
+			      (delete nil (mapcar
+					   (function (lambda (x)
+						       (if (eval (car x))
+							   (list (symbol-name (car x))))))
+					   minor-mode-alist))))))
+  (if (fboundp minor-mode)
+      (describe-function minor-mode)
+    (describe-variable minor-mode)))
+
+
 \f
 ;;; Automatic resizing of temporary buffers.
 
Index: bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.112
diff -u -r1.112 bindings.el
--- bindings.el	21 Dec 2002 20:19:37 -0000	1.112
+++ bindings.el	18 Jan 2003 08:19:28 -0000
@@ -376,6 +376,14 @@
   "Return the value of symbol VAR if it is bound, else nil."
   `(and (boundp (quote ,var)) ,var))
 
+
+(define-key mode-line-mode-menu [describe-minor-mode]
+  `(menu-item ,'(format "Describe `%s'" 
+			(substring mode-line-mode-menu-grab-mode-indicator 1))
+	      (lambda () (interactive) (describe-minor-mode-from-indicator))
+	      :visible (and mode-line-mode-menu-grab-mode-indicator
+			    (string= " " (substring mode-line-mode-menu-grab-mode-indicator 0 1)))))
+
 (define-key mode-line-mode-menu [overwrite-mode]
   `(menu-item ,(purecopy "Overwrite (Ovwrt)") overwrite-mode
 	      :button (:toggle . overwrite-mode)))
@@ -413,9 +421,49 @@
   `(menu-item ,(purecopy "Abbrev (Abbrev)") abbrev-mode
 	      :button (:toggle . abbrev-mode)))
 
+(defun get-mode-indicator-from-event (event)
+  (car (nth 4 (car (cdr event)))))
+(defvar mode-line-mode-menu-grab-mode-indicator nil)
+
 (defun mode-line-mode-menu (event)
   (interactive "@e")
+  (setq mode-line-mode-menu-grab-mode-indicator (get-mode-indicator-from-event event))
   (x-popup-menu event mode-line-mode-menu))
+
+(defun describe-minor-mode-from-indicator (&optional indicator)
+  "Display documentation of a minor mode specified by INDICATOR."
+  (interactive "sIndicator: ")
+  (unless indicator
+    (setq indicator mode-line-mode-menu-grab-mode-indicator))
+  (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
+    (if minor-mode
+	(describe-minor-mode minor-mode)
+      (message "Cannot find minor mode for `%s'" indicator))))
+
+(defun lookup-minor-mode-from-indicator (indicator)
+  "Return a minor mode symbol from its indicator on the modeline."
+  (if (and (< 0 (length indicator)) 
+	   (not (string= " " (substring indicator 0 1))))
+      (setq indicator (concat " " indicator)))
+  (let ((minor-modes minor-mode-alist)
+	result)
+    (while minor-modes
+      (let* ((minor-mode (car (car minor-modes)))
+	     (anindicator (car (cdr (car minor-modes)))))
+	(while (and anindicator (symbolp anindicator)
+		    (boundp anindicator)
+		    (not (eq anindicator (symbol-value anindicator))))
+	  (setq anindicator (symbol-value anindicator)))
+	(if (and (consp anindicator) 
+		 (keywordp (car anindicator))
+		 (eq :eval (car anindicator)))
+	    (setq anindicator (eval (cadr anindicator))))
+	(if (and (stringp anindicator) 
+		 (string= anindicator indicator))
+	    (setq result minor-mode
+		  minor-modes nil)
+	  (setq minor-modes (cdr minor-modes)))))
+    result))
 
 ;; Add menu of buffer operations to the buffer identification part
 ;; of the mode line.or header line.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

I wrote:

> Background: Few days ago, I found a strange string
> on mode-line: "doe". I wonder what is "doe". However,
> I cannot find a way to know what is doe. M-x describe-mode
> tells me nothing about doe. Finally I have to do 
> "grep doe emacs/lisp/*.el".

I'm very sorry. This is my misunderstanding.
The code which puts "doe" on the mode string
is at my .emacs;-)

However, I think the follwoing function I wrote is still useful:

     3. I added "Describe `MinorMode'" menu item to "Minor Modes" 
	popup menu. The user can get help about a minor mode with mouse-3.


John Paul Wallington<jpw@shootybangbang.com> lets me know my misunderstanding.
Thank you, John.

Masatake YAMATO

\f
What I put in my .emacs.
;; From: Richard Mlynarik <Mly@POBox.COM>
;; Subject: Re: case-fold-search indicator
;; Newsgroups: gnu.emacs.sources
;; Date: Fri, 22 Jun 2001 22:04:52 GMT

;;    From: jcasadonte@northbound-train.com (Joe Casadonte)
;;    Date: 21 Jun 2001 07:21:42 -0700
   
;;    Before I go and write this myself, does anyone have a nifty way to
;;    indicate whether or not case-fold-search is set?  I'm thinking
;;    something in the modeline, like overwrite does.
   
;; I've been using these since the year dot:

(or (assoc 'case-fold-search minor-mode-alist)
    (setq minor-mode-alist
          (cons '(case-fold-search "" " NoCFS") minor-mode-alist)))
(or (assoc 'debug-on-error minor-mode-alist)
    (setq minor-mode-alist
          (append minor-mode-alist '((debug-on-error " doe")))))
(or (assoc 'debug-on-quit minor-mode-alist)
    (setq minor-mode-alist
          (append minor-mode-alist '((debug-on-quit " doq")))))
(or (assoc 'make-backup-files minor-mode-alist)
    (setq minor-mode-alist
          (append minor-mode-alist '((make-backup-files "" (buffer-file-name " NoBack" ""))))))
(or (assoc 'buffer-auto-save-file-name minor-mode-alist)
    (setq minor-mode-alist
          (append minor-mode-alist '((buffer-auto-save-file-name "" (buffer-file-name " NoSave" ""))))))

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    1. M-x describe-minor-mode
       The user of describe-minor-mode can give its argument
       with completion. The completion table is built from the
       name of minor modes activated on the current buffer.

    2. M-x describe-minor-mode-from-indicator
       Almost the same as describe-minor-mode but the user 
       can pass an indicator instead of minor mode name to
       get help. I could not implment completion.
       (You will be not satisfied with "without completion".)

This is a very useful feature.  It could be more useful if the user
could click on a minor mode name in the mode line and get a
description of it.  This could work for mahor modes too.  Right now
only mouse-3 is used on this part of the mode line.

(It is not hard to call completing-read--did you find this
difficult?).

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

I've updated my patch.

I wrote:
    1. M-x describe-minor-mode
       The user of describe-minor-mode can give its argument
       with completion. The completion table is built from the
       name of minor modes activated on the current buffer.

    2. M-x describe-minor-mode-from-indicator
       Almost the same as describe-minor-mode but the user 
       can pass an indicator instead of minor mode name to
       get help. I could not implment completion.
       (You will be not satisfied with "without completion".)

    3. I added "Describe `MinorMode'" menu item to "Minor Modes" 
       popup menu. The user can get help about a minor mode with mouse-3.

Richard Stallman wrote:
    This is a very useful feature.  It could be more useful if the user
    could click on a minor mode name in the mode line and get a
    description of it.  This could work for mahor modes too.  Right now
    only mouse-3 is used on this part of the mode line.

    (It is not hard to call completing-read--did you find this
    difficult?).

Thank you for your comment.
The update patch supports:

2' M-x describe-minor-mode-from-indicator
   uses completing-read to read indicator.

3' Added popup menu for major mode. I introduced just another
   popup menu for major mode. 

Tell me when I should write ChangeLog entry:)
Masatake YAMATO
    
Index: help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.255
diff -u -r1.255 help.el
--- help.el	13 Jan 2003 08:04:46 -0000	1.255
+++ help.el	20 Jan 2003 17:09:59 -0000
@@ -611,6 +611,75 @@
 	  (setq minor-modes (cdr minor-modes))))
       (print-help-return-message))))
 
+(defun describe-minor-mode (minor-mode)
+  "Display documentation of a minor mode given as MINOR-MODE."
+  (interactive (list (intern (completing-read 
+			      "Minor mode: "
+			      (delete nil (mapcar
+					   (function (lambda (x)
+						       (if (eval (car x))
+							   (symbol-name (car x)))))
+					   minor-mode-alist))))))
+  (if (fboundp minor-mode)
+      (describe-function minor-mode)
+    (describe-variable minor-mode)))
+
+(defun describe-minor-mode-from-indicator (indicator)
+  "Display documentation of a minor mode specified by INDICATOR."
+  (interactive (list 
+		(completing-read 
+		 "Minor mode indicator: "
+		 (delete nil 
+			 (mapcar
+			  #'(lambda (x)
+			      (if (eval (car x))
+				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
+				    (if (and (< 0 (length i))
+					     (string= " " (substring i 0 1)))
+					(substring i 1)
+				      i))))
+			  minor-mode-alist)))))
+  (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
+    (if minor-mode
+	(describe-minor-mode minor-mode)
+      (error "Cannot find minor mode for `%s'" indicator))))
+
+(defun lookup-minor-mode-from-indicator (indicator)
+  "Return a minor mode symbol from its indicator on the modeline."
+  (if (and (< 0 (length indicator)) 
+	   (not (string= " " (substring indicator 0 1))))
+      (setq indicator (concat " " indicator)))
+  (let ((minor-modes minor-mode-alist)
+	result)
+    (while minor-modes
+      (let* ((minor-mode (car (car minor-modes)))
+	     (anindicator (car (cdr (car minor-modes)))))
+	(setq anindicator (expand-minor-mode-indicator-object anindicator))
+	(if (and (stringp anindicator) 
+		 (string= anindicator indicator))
+	    (setq result minor-mode
+		  minor-modes nil)
+	  (setq minor-modes (cdr minor-modes)))))
+    result))
+
+(defun expand-minor-mode-indicator-object (obj)
+  "Expand OBJ that represents a minor-mode indicator.
+cdr part of a `minor-mode-alist' element(indicator object) is the
+indicator of minor mode that is in car part.  Normally indicator
+object is a string. However, in some case it is more compound object
+like cons cell. This function tries to make the compound object a string."
+  ;; copied from describe-mode
+  (while (and obj (symbolp obj)
+	      (boundp obj)
+	      (not (eq obj (symbol-value obj))))
+    (setq obj (symbol-value obj)))
+  ;; eval :eval
+  (if (and (consp obj) 
+	   (keywordp (car obj))
+	   (eq :eval (car obj)))
+      (setq obj (eval (cadr obj))))
+  obj)
+
 \f
 ;;; Automatic resizing of temporary buffers.
 
Index: bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.112
diff -u -r1.112 bindings.el
--- bindings.el	21 Dec 2002 20:19:37 -0000	1.112
+++ bindings.el	20 Jan 2003 17:09:59 -0000
@@ -249,6 +249,9 @@
 
 (make-variable-buffer-local 'mode-line-modified)
 
+(defvar mode-line-major-mode-keymap nil "\
+Keymap to display on major mode.")
+
 ;; Actual initialization is below.
 (defvar mode-line-position nil
   "Mode-line control for displaying line number, column number and fraction.")
@@ -257,12 +260,17 @@
   "Mode-line control for displaying major and minor modes.")
 
 (defvar mode-line-minor-mode-keymap nil "\
-Keymap to display on major and minor modes.")
+Keymap to display on minor modes.")
+
+;; Menu of major mode.
+(let ((map (make-sparse-keymap)))
+  (define-key map [mode-line down-mouse-3] 'mode-line-major-mode-menu-1)
+  (setq mode-line-major-mode-keymap map))
 
 ;; Menu of minor modes.
 (let ((map (make-sparse-keymap)))
-  (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
-  (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
+  (define-key map [mode-line down-mouse-3] 'mode-line-minor-mode-menu-1)
+  (define-key map [header-line down-mouse-3] 'mode-line-minor-mode-menu-1)
   (setq mode-line-minor-mode-keymap map))
 
 (let* ((help-echo
@@ -292,8 +300,13 @@
   (setq-default mode-line-modes
     (list
      (propertize "%[(" 'help-echo help-echo)
-     `(:propertize ("" mode-name mode-line-process minor-mode-alist)
-		   help-echo "mouse-3: minor mode menu"
+     ;; major mode
+     `(:propertize ("" mode-name) 
+		   help-echo "mouse-3: major mode menu"
+		   local-map ,mode-line-major-mode-keymap)
+     ;; minor modes
+     `(:propertize ("" mode-line-process minor-mode-alist)
+		   help-echo "mouse-3: minor modes menu"
 		   local-map ,mode-line-minor-mode-keymap)
      (propertize "%n" 'help-echo "mouse-2: widen"
 		 'local-map (make-mode-line-mouse-map
@@ -359,63 +372,104 @@
   (interactive)
   (switch-to-buffer (other-buffer)))
 
-(defvar mode-line-mode-menu (make-sparse-keymap "Minor Modes") "\
-Menu of mode operations in the mode line.")
+(defvar mode-line-major-mode-menu (make-sparse-keymap "Major Mode") "\
+Menu of major mode operations in the mode line.")
 
-(defun mode-line-mode-menu-1 (event)
-  (interactive "e")
+(defvar mode-line-minor-mode-menu (make-sparse-keymap "Minor Modes") "\
+Menu of minor modes operations in the mode line.")
+
+(defun mode-line-mode-menu-1 (event menu)
   (save-selected-window
     (select-window (posn-window (event-start event)))
-    (let* ((selection (mode-line-mode-menu event))
-	   (binding (and selection (lookup-key mode-line-mode-menu
+    (let* ((selection (funcall menu event))
+	   (binding (and selection (lookup-key (eval menu)
 					       (vector (car selection))))))
       (if binding
 	  (call-interactively binding)))))
+  
+(defun mode-line-major-mode-menu-1 (event)
+  (interactive "e")
+  (mode-line-mode-menu-1 event 'mode-line-major-mode-menu))
+
+(defun mode-line-minor-mode-menu-1 (event)
+  (interactive "e")
+  (mode-line-mode-menu-1 event 'mode-line-minor-mode-menu))
 
 (defmacro bound-and-true-p (var)
   "Return the value of symbol VAR if it is bound, else nil."
   `(and (boundp (quote ,var)) ,var))
 
-(define-key mode-line-mode-menu [overwrite-mode]
+(define-key mode-line-major-mode-menu [describe-mode]
+  `(menu-item ,'(format "Describe `%s'" 
+			mode-line-mode-menu-grab-mode-indicator)
+	      describe-mode
+	      :visible (and mode-line-mode-menu-grab-mode-indicator
+			    (not (string= " " (substring mode-line-mode-menu-grab-mode-indicator 0 1))))))
+
+(define-key mode-line-minor-mode-menu [overwrite-mode]
   `(menu-item ,(purecopy "Overwrite (Ovwrt)") overwrite-mode
 	      :button (:toggle . overwrite-mode)))
-(define-key mode-line-mode-menu [outline-minor-mode]
+(define-key mode-line-minor-mode-menu [outline-minor-mode]
   `(menu-item ,(purecopy "Outline (Outl)") outline-minor-mode
 	      :button (:toggle . (bound-and-true-p outline-minor-mode))))
-(define-key mode-line-mode-menu [line-number-mode]
+(define-key mode-line-minor-mode-menu [line-number-mode]
   `(menu-item ,(purecopy "Line number") line-number-mode
 	      :button (:toggle . line-number-mode)))
-(define-key mode-line-mode-menu [highlight-changes-mode]
+(define-key mode-line-minor-mode-menu [highlight-changes-mode]
   `(menu-item ,(purecopy "Highlight changes (Chg)") highlight-changes-mode
 	      :button (:toggle . highlight-changes-mode)))
-(define-key mode-line-mode-menu [glasses-mode]
+(define-key mode-line-minor-mode-menu [glasses-mode]
   `(menu-item ,(purecopy "Glasses (o^o)") glasses-mode
 	      :button (:toggle . (bound-and-true-p glasses-mode))))
-(define-key mode-line-mode-menu [hide-ifdef-mode]
+(define-key mode-line-minor-mode-menu [hide-ifdef-mode]
   `(menu-item ,(purecopy "Hide ifdef (Ifdef)") hide-ifdef-mode
 	      :button (:toggle . (bound-and-true-p hide-ifdef-mode))))
-(define-key mode-line-mode-menu [font-lock-mode]
+(define-key mode-line-minor-mode-menu [font-lock-mode]
   `(menu-item ,(purecopy "Font Lock") font-lock-mode
 	      :button (:toggle . font-lock-mode)))
-(define-key mode-line-mode-menu [flyspell-mode]
+(define-key mode-line-minor-mode-menu [flyspell-mode]
   `(menu-item ,(purecopy "Flyspell (Fly)") flyspell-mode
 	      :button (:toggle . (bound-and-true-p flyspell-mode))))
-(define-key mode-line-mode-menu [column-number-mode]
+(define-key mode-line-minor-mode-menu [column-number-mode]
   `(menu-item ,(purecopy "Column number") column-number-mode
 	      :button (:toggle . column-number-mode)))
-(define-key mode-line-mode-menu [auto-fill-mode]
+(define-key mode-line-minor-mode-menu [auto-fill-mode]
   `(menu-item ,(purecopy "Auto Fill (Fill)") auto-fill-mode
 	      :button (:toggle . auto-fill-function)))
-(define-key mode-line-mode-menu [auto-revert-mode]
+(define-key mode-line-minor-mode-menu [auto-revert-mode]
   `(menu-item ,(purecopy "Auto revert (ARev)") auto-revert-mode
 	      :button (:toggle . auto-revert-mode)))
-(define-key mode-line-mode-menu [abbrev-mode]
+(define-key mode-line-minor-mode-menu [abbrev-mode]
   `(menu-item ,(purecopy "Abbrev (Abbrev)") abbrev-mode
 	      :button (:toggle . abbrev-mode)))
 
-(defun mode-line-mode-menu (event)
+(autoload 'describe-minor-mode-from-indicator "help"
+  "Display documentation of a minor mode given as MINOR-MODE." t)
+(define-key mode-line-minor-mode-menu [describe-minor-mode]
+  `(menu-item ,'(format "Describe `%s'" 
+			(substring mode-line-mode-menu-grab-mode-indicator 1))
+	      (lambda () 
+		(interactive)
+		(describe-minor-mode-from-indicator 
+		 mode-line-mode-menu-grab-mode-indicator))
+	      :visible (and mode-line-mode-menu-grab-mode-indicator
+			    (string= " " (substring mode-line-mode-menu-grab-mode-indicator 0 1)))))
+
+(defun get-mode-indicator-from-event (event)
+  (car (nth 4 (car (cdr event)))))
+(defvar mode-line-mode-menu-grab-mode-indicator nil)
+
+(defun mode-line-mode-menu (event menu)
+  (setq mode-line-mode-menu-grab-mode-indicator (get-mode-indicator-from-event event))
+  (x-popup-menu event menu))
+
+(defun mode-line-major-mode-menu (event)
+  (interactive "@e")
+  (mode-line-mode-menu event mode-line-major-mode-menu))
+
+(defun mode-line-minor-mode-menu (event)
   (interactive "@e")
-  (x-popup-menu event mode-line-mode-menu))
+  (mode-line-mode-menu event mode-line-minor-mode-menu))
 
 ;; Add menu of buffer operations to the buffer identification part
 ;; of the mode line.or header line.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

I wrote:

    Thank you for your comment.
    The update patch supports:

    2' M-x describe-minor-mode-from-indicator
       uses completing-read to read indicator.

    3' Added popup menu for major mode. I introduced just another
       popup menu for major mode. 

The attached patch is also needed to use my describe-minor-mode* 
patch.

Masatake YAMATO

Index: subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.337
diff -u -r1.337 subr.el
--- subr.el	19 Jan 2003 01:07:34 -0000	1.337
+++ subr.el	21 Jan 2003 07:14:34 -0000
@@ -2120,7 +2120,7 @@
 					 minor-mode-alist)))))))
   ;; Add the toggle to the minor-modes menu if requested.
   (when (get toggle :included)
-    (define-key mode-line-mode-menu
+    (define-key mode-line-minor-mode-menu
       (vector toggle)
       (list 'menu-item
 	    (concat
Index: ruler-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/ruler-mode.el,v
retrieving revision 1.10
diff -u -r1.10 ruler-mode.el
--- ruler-mode.el	13 Jan 2003 08:22:50 -0000	1.10
+++ ruler-mode.el	21 Jan 2003 07:14:34 -0000
@@ -548,7 +548,7 @@
                  #'force-mode-line-update t)))
 \f
 ;; Add ruler-mode to the minor mode menu in the mode line
-(define-key mode-line-mode-menu [ruler-mode]
+(define-key mode-line-minor-mode-menu [ruler-mode]
   `(menu-item "Ruler" ruler-mode
               :button (:toggle . ruler-mode)))

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    2' M-x describe-minor-mode-from-indicator
       uses completing-read to read indicator.

That fills the gap.  Thanks.

    3' Added popup menu for major mode. I introduced just another
       popup menu for major mode. 

I see you added a menu for major modes and also did what I
suggested--to add a mouse command (which you put in the menu) to
describe a minor mode or a major mode.

What I was thinking of was to put the "describe" command on mouse-1 or
mouse-2 rather than to put it in the menu.

As for whether a major mode menu is useful, the question is, what
major modes would we put in it?  One obvious answer is "all of them",
but there are too many.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

    I see you added a menu for major modes and also did what I
    suggested--to add a mouse command (which you put in the menu) to
    describe a minor mode or a major mode.

    What I was thinking of was to put the "describe" command on mouse-1 or
    mouse-2 rather than to put it in the menu.

There is trade-off in two methods: just-one-click or choosing-from-menu.

If you are familiar with Emacs, you will do not hesitate to click the
mode line because you might know what happens next. You might be satisfied
with quick operation. Further more, you might think GUI menu is a toy that
make computer operation slow down.

If you are new to Emacs, you must hesitate to click somewhere unknown/unfamiliar 
place on Emacs because you don't know what happens if you click. 
The mode line is one of the such place. The menu does not solve 
this issue perfectly. However, the menu's VISUAL FEEDBACK will much help for the
emacs beginners.

What I implemented is targeted on emacs beginners, so I used popup menu.
Quick operation was not the my goal. How do you think?

I used mouse-3, not mouse-1 and mouse-2 because Some gtk+/gnome applicaiton uses
mouse-3 as context-menu. I expected Emacs beginners can use emacs's menu by analogy.
Do you think we should bind all keys and mouse buttons with commands?
(Yes, in my .emacs I've tried to bind super-* and hyper-* with many 
commands:-P)

BTW, is there any policy about mouse button bindings; what kind of
commands should be bound to mouse-1, what other kind of commands
should be bound to mouse-2, and what another kind of commands should
be bound to mouse-3? Binding any empty button to commands without
policy will be cause of confusions. If it is already written in info,
I have to read.

In other hand, key binding policy for C-x prefix(global) and C-c 
prefix(mode specific) is understandable.

Masatake YAMATO

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    There is trade-off in two methods: just-one-click or choosing-from-menu.

    If you are familiar with Emacs, you will do not hesitate to click the
    mode line because you might know what happens next. You might be satisfied
    with quick operation. Further more, you might think GUI menu is a toy that
    make computer operation slow down.

The tooltip says what the clicks do.  If it says "Mouse-1: mode help",
people won't be scared.

Please put it on mouse-2 and update the tooltip string.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

>     There is trade-off in two methods: just-one-click or choosing-from-menu.
> 
>     If you are familiar with Emacs, you will do not hesitate to click the
>     mode line because you might know what happens next. You might be satisfied
>     with quick operation. Further more, you might think GUI menu is a toy that
>     make computer operation slow down.
> 
> The tooltip says what the clicks do.  If it says "Mouse-1: mode help",
> people won't be scared.
> 
> Please put it on mouse-2 and update the tooltip string.

I did. Please try the attached patch.
You don't need to apply patches for ruler-mode.el and subr.el I posted here.
Because some symbols in the new patch are reverted.

Masatake YAMATO

Index: help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.255
diff -u -r1.255 help.el
--- help.el	13 Jan 2003 08:04:46 -0000	1.255
+++ help.el	30 Jan 2003 17:19:07 -0000
@@ -611,6 +611,75 @@
 	  (setq minor-modes (cdr minor-modes))))
       (print-help-return-message))))
 
+(defun describe-minor-mode (minor-mode)
+  "Display documentation of a minor mode given as MINOR-MODE."
+  (interactive (list (intern (completing-read 
+			      "Minor mode: "
+			      (delete nil (mapcar
+					   (function (lambda (x)
+						       (if (eval (car x))
+							   (symbol-name (car x)))))
+					   minor-mode-alist))))))
+  (if (fboundp minor-mode)
+      (describe-function minor-mode)
+    (describe-variable minor-mode)))
+
+(defun describe-minor-mode-from-indicator (indicator)
+  "Display documentation of a minor mode specified by INDICATOR."
+  (interactive (list 
+		(completing-read 
+		 "Minor mode indicator: "
+		 (delete nil 
+			 (mapcar
+			  #'(lambda (x)
+			      (if (eval (car x))
+				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
+				    (if (and (< 0 (length i))
+					     (string= " " (substring i 0 1)))
+					(substring i 1)
+				      i))))
+			  minor-mode-alist)))))
+  (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
+    (if minor-mode
+	(describe-minor-mode minor-mode)
+      (error "Cannot find minor mode for `%s'" indicator))))
+
+(defun lookup-minor-mode-from-indicator (indicator)
+  "Return a minor mode symbol from its indicator on the modeline."
+  (if (and (< 0 (length indicator)) 
+	   (not (string= " " (substring indicator 0 1))))
+      (setq indicator (concat " " indicator)))
+  (let ((minor-modes minor-mode-alist)
+	result)
+    (while minor-modes
+      (let* ((minor-mode (car (car minor-modes)))
+	     (anindicator (car (cdr (car minor-modes)))))
+	(setq anindicator (expand-minor-mode-indicator-object anindicator))
+	(if (and (stringp anindicator) 
+		 (string= anindicator indicator))
+	    (setq result minor-mode
+		  minor-modes nil)
+	  (setq minor-modes (cdr minor-modes)))))
+    result))
+
+(defun expand-minor-mode-indicator-object (obj)
+  "Expand OBJ that represents a minor-mode indicator.
+cdr part of a `minor-mode-alist' element(indicator object) is the
+indicator of minor mode that is in car part.  Normally indicator
+object is a string. However, in some case it is more compound object
+like cons cell. This function tries to make the compound object a string."
+  ;; copied from describe-mode
+  (while (and obj (symbolp obj)
+	      (boundp obj)
+	      (not (eq obj (symbol-value obj))))
+    (setq obj (symbol-value obj)))
+  ;; eval :eval
+  (if (and (consp obj) 
+	   (keywordp (car obj))
+	   (eq :eval (car obj)))
+      (setq obj (eval (cadr obj))))
+  obj)
+
 \f
 ;;; Automatic resizing of temporary buffers.
 
Index: bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.112
diff -u -r1.112 bindings.el
--- bindings.el	21 Dec 2002 20:19:37 -0000	1.112
+++ bindings.el	30 Jan 2003 17:19:08 -0000
@@ -256,8 +256,16 @@
 (defvar mode-line-modes nil
   "Mode-line control for displaying major and minor modes.")
 
+(defvar mode-line-major-mode-keymap nil "\
+Keymap to display on major mode.")
+
 (defvar mode-line-minor-mode-keymap nil "\
-Keymap to display on major and minor modes.")
+Keymap to display on  minor modes.")
+
+;; Menu of major mode.
+(let ((map (make-sparse-keymap)))
+  (define-key map [mode-line mouse-2] 'describe-mode)
+  (setq mode-line-major-mode-keymap map))
 
 ;; Menu of minor modes.
 (let ((map (make-sparse-keymap)))
@@ -292,8 +300,15 @@
   (setq-default mode-line-modes
     (list
      (propertize "%[(" 'help-echo help-echo)
-     `(:propertize ("" mode-name mode-line-process minor-mode-alist)
-		   help-echo "mouse-3: minor mode menu"
+     ;; major mode
+     `(:propertize ("" mode-name) 
+		   help-echo ,#'(lambda (window object point)
+				 (format "mouse-2: help for \"%s\""
+					 object))
+		   local-map ,mode-line-major-mode-keymap)
+     ;; minor modes
+     `(:propertize ("" mode-line-process minor-mode-alist)
+		   help-echo "mouse-3: minor modes menu"
 		   local-map ,mode-line-minor-mode-keymap)
      (propertize "%n" 'help-echo "mouse-2: widen"
 		 'local-map (make-mode-line-mouse-map
@@ -413,9 +428,23 @@
   `(menu-item ,(purecopy "Abbrev (Abbrev)") abbrev-mode
 	      :button (:toggle . abbrev-mode)))
 
+(defvar mode-line-mode-menu-grab-mode-indicator nil)
+(autoload 'describe-minor-mode-from-indicator "help"
+  "Display documentation of a minor mode given as MINOR-MODE." t)
+(define-key mode-line-mode-menu [describe-minor-mode]
+  `(menu-item ,'(format "Describe `%s'" 
+			(substring mode-line-mode-menu-grab-mode-indicator 1))
+	      (lambda () 
+		(interactive)
+		(describe-minor-mode-from-indicator 
+		 mode-line-mode-menu-grab-mode-indicator))
+	      :visible (and mode-line-mode-menu-grab-mode-indicator
+			    (string= " " (substring mode-line-mode-menu-grab-mode-indicator 0 1)))))
 (defun mode-line-mode-menu (event)
   (interactive "@e")
-  (x-popup-menu event mode-line-mode-menu))
+  (let ((get-mode-indicator-from-event #'(lambda (e) (car (nth 4 (car (cdr e)))))))
+    (setq mode-line-mode-menu-grab-mode-indicator (funcall get-mode-indicator-from-event event))
+    (x-popup-menu event mode-line-mode-menu)))
 
 ;; Add menu of buffer operations to the buffer identification part
 ;; of the mode line.or header line.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

(Two months ago, my patch is reviewd by RMS and I got comments about help 
for minor modes. I could not find time to improve. Now I do.)


This patch provides functions that:

1. you can get help for current selected major mode and minor modes
   with clicking major and minor modes string on the mode line.
2. M-x describe-minor-mode, help for a specified minor mode like 
   M-x describe-mode.

Review results(summary): 

I used popup menu to invoke describe-mode and describe-minor-mode.
RMS gave me a comment that I should use mouse-2 and help-echo instead
of popup menu. Here I did.

Masatake YAMATO

2003-03-30  Masatake YAMATO  <jet@gyve.org>

	* bindings.el (mode-line-major-mode-keymap): New variable.
	(mode-line-minor-mode-help): New function. bound to 
	mode-line-minor-mode-keymap.
	(mode-line-modes): split mode-line-mode definitions to
	mode-name, mode-line-process and minor-mode-alist.
	For mode-name, mode-line-major-mode-keymap is used.

	* help.el (describe-minor-mode): 
	(describe-minor-mode-from-indicator): 
	(lookup-minor-mode-from-indicator): New functions.

Index: help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.257
diff -u -r1.257 help.el
--- help.el	12 Feb 2003 23:13:43 -0000	1.257
+++ help.el	30 Mar 2003 08:13:16 -0000
@@ -611,6 +611,74 @@
 	  (setq minor-modes (cdr minor-modes))))
       (print-help-return-message))))
 
+(defun describe-minor-mode (minor-mode)
+  "Display documentation of a minor mode given as MINOR-MODE."
+  (interactive (list (intern (completing-read 
+			      "Minor mode: "
+			      (delete nil (mapcar
+					   (function (lambda (x)
+						       (if (eval (car x))
+							   (symbol-name (car x)))))
+					   minor-mode-alist))))))
+  (if (fboundp minor-mode)
+      (describe-function minor-mode)
+    (describe-variable minor-mode)))
+
+(defun describe-minor-mode-from-indicator (indicator)
+  "Display documentation of a minor mode specified by INDICATOR."
+  (interactive (list 
+		(completing-read 
+		 "Minor mode indicator: "
+		 (delete nil 
+			 (mapcar
+			  #'(lambda (x)
+			      (if (eval (car x))
+				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
+				    (if (and (< 0 (length i))
+					     (string= " " (substring i 0 1)))
+					(substring i 1)
+				      i))))
+			  minor-mode-alist)))))
+  (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
+    (if minor-mode
+	(describe-minor-mode minor-mode)
+      (error "Cannot find minor mode for `%s'" indicator))))
+
+(defun lookup-minor-mode-from-indicator (indicator)
+  "Return a minor mode symbol from its indicator on the modeline."
+  (if (and (< 0 (length indicator)) 
+	   (not (string= " " (substring indicator 0 1))))
+      (setq indicator (concat " " indicator)))
+  (let ((minor-modes minor-mode-alist)
+	result)
+    (while minor-modes
+      (let* ((minor-mode (car (car minor-modes)))
+	     (anindicator (car (cdr (car minor-modes)))))
+	(setq anindicator (expand-minor-mode-indicator-object anindicator))
+	(if (and (stringp anindicator) 
+		 (string= anindicator indicator))
+	    (setq result minor-mode
+		  minor-modes nil)
+	  (setq minor-modes (cdr minor-modes)))))
+    result))
+
+(defun expand-minor-mode-indicator-object (obj)
+  "Expand OBJ that represents a minor-mode indicator.
+cdr part of a `minor-mode-alist' element(indicator object) is the
+indicator of minor mode that is in car part.  Normally indicator
+object is a string. However, in some case it is more compound object
+like cons cell. This function tries to make the compound object a string."
+  ;; copied from describe-mode
+  (while (and obj (symbolp obj)
+	      (boundp obj)
+	      (not (eq obj (symbol-value obj))))
+    (setq obj (symbol-value obj)))
+  (when (and (consp obj) 
+	     (keywordp (car obj))
+	     (eq :eval (car obj)))
+    (setq obj (eval (cadr obj))))
+  obj)
+
 \f
 ;;; Automatic resizing of temporary buffers.
 
Index: bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.112
diff -u -r1.112 bindings.el
--- bindings.el	21 Dec 2002 20:19:37 -0000	1.112
+++ bindings.el	30 Mar 2003 08:13:16 -0000
@@ -256,11 +256,19 @@
 (defvar mode-line-modes nil
   "Mode-line control for displaying major and minor modes.")
 
+(defvar mode-line-major-mode-keymap nil "\
+Keymap to display on major mode.")
+
 (defvar mode-line-minor-mode-keymap nil "\
-Keymap to display on major and minor modes.")
+Keymap to display on minor modes.")
+
+(let ((map (make-sparse-keymap)))
+  (define-key map [mode-line mouse-2] 'describe-mode)
+  (setq mode-line-major-mode-keymap map))
 
 ;; Menu of minor modes.
 (let ((map (make-sparse-keymap)))
+  (define-key map [mode-line mouse-2] 'mode-line-minor-mode-help)
   (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
   (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
   (setq mode-line-minor-mode-keymap map))
@@ -292,8 +300,12 @@
   (setq-default mode-line-modes
     (list
      (propertize "%[(" 'help-echo help-echo)
-     `(:propertize ("" mode-name mode-line-process minor-mode-alist)
-		   help-echo "mouse-3: minor mode menu"
+     `(:propertize ("" mode-name)
+		   help-echo "mouse-2: help for current major mode"
+		   local-map ,mode-line-major-mode-keymap)
+     `(:propertize ("" mode-line-process))
+     `(:propertize ("" minor-mode-alist)
+		   help-echo "mouse-2: help for minor modes, mouse-3: minor mode menu"
 		   local-map ,mode-line-minor-mode-keymap)
      (propertize "%n" 'help-echo "mouse-2: widen"
 		 'local-map (make-mode-line-mouse-map
@@ -416,6 +428,12 @@
 (defun mode-line-mode-menu (event)
   (interactive "@e")
   (x-popup-menu event mode-line-mode-menu))
+
+(defun mode-line-minor-mode-help (event)
+  "Describe minor mode for EVENT occured on minor modes area of the mode line."
+  (interactive "@e")
+  (let ((indicator (car (nth 4 (car (cdr event))))))
+    (describe-minor-mode-from-indicator indicator)))
 
 ;; Add menu of buffer operations to the buffer identification part
 ;; of the mode line.or header line.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Stefan Monnier]

> +(defun describe-minor-mode (minor-mode)
> +  "Display documentation of a minor mode given as MINOR-MODE."
> +  (interactive (list (intern (completing-read 
> +			      "Minor mode: "
> +			      (delete nil (mapcar
> +					   (function (lambda (x)

`function' is not necessary, and it forces too much indendation here.

> +						       (if (eval (car x))
> +							   (symbol-name (car x)))))

I generally believe that `eval' should be avoided.  This is especially
true here since you call `symbol-name' so you already assume that (car x)
is a symbol, so you could just call `symbol-value' instead of `eval'.
But note also that nothing guarantees you that (car x) is bound.
Finally, I think it's perfectly OK (if not preferable) to list all
the minor modes rather than just the currently active ones, so
I'd just use

  (delq nil (mapcar (lambda (x) (symbol-name (car x))) minor-mode-alist))

> +(defun describe-minor-mode-from-indicator (indicator)
> +  "Display documentation of a minor mode specified by INDICATOR."
> +  (interactive (list
> +		(completing-read
> +		 "Minor mode indicator: "
> +		 (delete nil
> +			 (mapcar
> +			  #'(lambda (x)
> +			      (if (eval (car x))
> +				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
> +				    (if (and (< 0 (length i))
> +					     (string= " " (substring i 0 1)))
> +					(substring i 1)
> +				      i))))
> +			  minor-mode-alist)))))

How about

 (delq nil (mapcar (lambda (x)
                     (let ((i (format-mode-line x)))
                       (if (> (length i) 0)
                           (if (eq (aref i 0) ?\ )
                               (substring i 1) i))))
                   minor-mode-alist)) ?

> +(defun lookup-minor-mode-from-indicator (indicator)
> +  "Return a minor mode symbol from its indicator on the modeline."
> +  (if (and (< 0 (length indicator))
> +	   (not (string= " " (substring indicator 0 1))))
> +      (setq indicator (concat " " indicator)))

I'd rather not assume that indicators start with a space.

> +cdr part of a `minor-mode-alist' element(indicator object) is the
> +indicator of minor mode that is in car part.  Normally indicator
> +object is a string. However, in some case it is more compound object
> +like cons cell.

Actually, minor-mode-alist is a mode-line-spec.
So we can simply use `formal-mode-line' to interpret it.

>  (defvar mode-line-minor-mode-keymap nil "\
> -Keymap to display on major and minor modes.")
> +Keymap to display on minor modes.")
> +
> +(let ((map (make-sparse-keymap)))
> +  (define-key map [mode-line mouse-2] 'describe-mode)
> +  (setq mode-line-major-mode-keymap map))

I recommend the use of

  (defvar foo-map
    (let ((map (...)
      (define-key ...)
      ...
      map))

I think that the coding cnvention also recommend it.  Admittedly, a lot
of Emacs code doesn't use it :-(

In any case, it seems that your patch removes the "mouse-3 on major mode"
behavior that allowed to turn on minor modes by clicking mouse-3
on the major-mode name.  I think this is important since it can
often happen that there is no active minor-mode on which to
click mouse-3.


	Stefan

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

Thank you for reviewing my patch.

I agree with the almost all of your suggestions.
I'll reflect suggestions to my patch.

> > +(defun lookup-minor-mode-from-indicator (indicator)
> > +  "Return a minor mode symbol from its indicator on the modeline."
> > +  (if (and (< 0 (length indicator))
> > +	   (not (string= " " (substring indicator 0 1))))
> > +      (setq indicator (concat " " indicator)))
> 
> I'd rather not assume that indicators start with a space.

What can I do?

(defun lookup-minor-mode-from-indicator (indicator)
  "Return a minor mode symbol from its indicator on the modeline."
  (if (and (< 0 (length indicator)) 
	   (not (eq (aref indicator 0) ?\ )))
      (setq indicator (concat " " indicator)))
  (let ((minor-modes minor-mode-alist)
	result)
    (while minor-modes
      (let* ((minor-mode (car (car minor-modes)))
	     (anindicator (car (cdr (car minor-modes)))))
	(setq anindicator (format-mode-line anindicator))
	(if (and (stringp anindicator) 
		 (string= anindicator indicator))

Should I use string-match instead of string= to compare indicator 
and anindicator? 

(concat " " indicator) is wrong idea?

Masatake YAMATO

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Stefan Monnier]

> > > +(defun lookup-minor-mode-from-indicator (indicator)
> > > +  "Return a minor mode symbol from its indicator on the modeline."
> > > +  (if (and (< 0 (length indicator))
> > > +	   (not (string= " " (substring indicator 0 1))))
> > > +      (setq indicator (concat " " indicator)))
> > 
> > I'd rather not assume that indicators start with a space.
> 
> What can I do?
> 
> (defun lookup-minor-mode-from-indicator (indicator)
>   "Return a minor mode symbol from its indicator on the modeline."
>   (if (and (< 0 (length indicator)) 
> 	   (not (eq (aref indicator 0) ?\ )))
>       (setq indicator (concat " " indicator)))
>   (let ((minor-modes minor-mode-alist)
> 	result)
>     (while minor-modes
>       (let* ((minor-mode (car (car minor-modes)))
> 	     (anindicator (car (cdr (car minor-modes)))))
> 	(setq anindicator (format-mode-line anindicator))
> 	(if (and (stringp anindicator) 
> 		 (string= anindicator indicator))
> 
> Should I use string-match instead of string= to compare indicator 
> and anindicator? 
> 
> (concat " " indicator) is wrong idea?

How about

  (if (> (length anindicator) 0)
      (equal indicator (if (eq (aref anindicator 0) ?\ )
                           (substring anindicator 1) anindicator)))

I.e. apply the exact same transformation to `anindicator' as you did
to `indicator'.


	Stefan

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    I generally believe that `eval' should be avoided.  This is especially
    true here since you call `symbol-name' so you already assume that (car x)
    is a symbol, so you could just call `symbol-value' instead of `eval'.
    But note also that nothing guarantees you that (car x) is bound.
    Finally, I think it's perfectly OK (if not preferable) to list all
    the minor modes rather than just the currently active ones, so
    I'd just use

      (delq nil (mapcar (lambda (x) (symbol-name (car x))) minor-mode-alist))

minor-mode-alist can get quite long, so I think it should show only
the enabled ones.  Using boundp and symbol-value is a good approach.

I've alredy asked Juanma to install the code that Masataki sent.
How about if you install the improvements that you can see to make?

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

> minor-mode-alist can get quite long, so I think it should show only
> the enabled ones.  Using boundp and symbol-value is a good approach.

If you want to know only minor modes currenly active, 
describe-minor-mode-from-indicator is good.

> I've alredy asked Juanma to install the code that Masataki sent.
> How about if you install the improvements that you can see to make?

I'd like to reflect the suggestions given by Stefan Monnier to my
patch. I've already created new patch against the current Emacs source 
code. I have to change the doc and NEWS file next.

Masatake YAMATO

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

> I'd like to reflect the suggestions given by Stefan Monnier to my
> patch. I've already created new patch against the current Emacs source 
> code. I have to change the doc and NEWS file next.

NEWS entry:
**** Help for the specified minor mode.
M-x describe-minor-mode shows help documents for the specified minor
mode. M-x describe-minor-mode-from-indicator shows the same thing, too.
However, `describe-minor-mode-from-indicator' takes an indicator appeared
on the mode line as the argument. You can invoke this function with mouse 
operations.

**** Displaying help documents for modes with mouse-2 on the mode line.
You can click(mouse-2) a mode string on the mode line to show its help
documents. Both major mode and minor modes are supproted.

Index: help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.258
diff -u -r1.258 help.el
--- help.el	31 Mar 2003 20:22:58 -0000	1.258
+++ help.el	1 Apr 2003 13:30:36 -0000
@@ -615,30 +615,29 @@
   "Display documentation of a minor mode given as MINOR-MODE."
   (interactive (list (intern (completing-read 
 			      "Minor mode: "
-			      (delete nil (mapcar
-					   (function (lambda (x)
-						       (if (eval (car x))
-							   (symbol-name (car x)))))
-					   minor-mode-alist))))))
+			      (delq nil (mapcar 
+					 (lambda (x) 
+					   (symbol-name (car x)))
+					 minor-mode-alist))))))
   (if (fboundp minor-mode)
       (describe-function minor-mode)
     (describe-variable minor-mode)))
 
 (defun describe-minor-mode-from-indicator (indicator)
-  "Display documentation of a minor mode specified by INDICATOR."
+  "Display documentation of a minor mode specified by INDICATOR.
+If you call this function interactively, you can give indicator which
+is currently activated with completion."
   (interactive (list 
 		(completing-read 
 		 "Minor mode indicator: "
-		 (delete nil 
-			 (mapcar
-			  #'(lambda (x)
-			      (if (eval (car x))
-				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
-				    (if (and (< 0 (length i))
-					     (string= " " (substring i 0 1)))
-					(substring i 1)
-				      i))))
-			  minor-mode-alist)))))
+		 (delq nil 
+		       (mapcar (lambda (x)
+				 (let ((i (format-mode-line x)))
+				   ;; remove first space if existed
+				   (if (> (length i) 0)
+				       (if (eq (aref i 0) ?\ )
+					   (substring i 1) i))))
+			       minor-mode-alist)))))
   (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
     (if minor-mode
 	(describe-minor-mode minor-mode)
@@ -646,38 +645,26 @@
 
 (defun lookup-minor-mode-from-indicator (indicator)
   "Return a minor mode symbol from its indicator on the modeline."
+  ;; remove first space if existed
   (if (and (< 0 (length indicator)) 
-	   (not (string= " " (substring indicator 0 1))))
-      (setq indicator (concat " " indicator)))
+	   (eq (aref indicator 0) ?\ ))
+      (setq indicator (substring indicator 1)))
   (let ((minor-modes minor-mode-alist)
 	result)
     (while minor-modes
       (let* ((minor-mode (car (car minor-modes)))
-	     (anindicator (car (cdr (car minor-modes)))))
-	(setq anindicator (expand-minor-mode-indicator-object anindicator))
+	     (anindicator (format-mode-line 
+			   (car (cdr (car minor-modes))))))
+	;; remove first space if existed
 	(if (and (stringp anindicator) 
-		 (string= anindicator indicator))
+		 (> (length anindicator) 0)
+		 (eq (aref anindicator 0) ?\ ))
+	    (setq anindicator (substring anindicator 1)))
+	(if (equal indicator anindicator)
 	    (setq result minor-mode
 		  minor-modes nil)
 	  (setq minor-modes (cdr minor-modes)))))
     result))
-
-(defun expand-minor-mode-indicator-object (obj)
-  "Expand OBJ that represents a minor-mode indicator.
-cdr part of a `minor-mode-alist' element(indicator object) is the
-indicator of minor mode that is in car part.  Normally indicator
-object is a string. However, in some case it is more compound object
-like cons cell. This function tries to make the compound object a string."
-  ;; copied from describe-mode
-  (while (and obj (symbolp obj)
-	      (boundp obj)
-	      (not (eq obj (symbol-value obj))))
-    (setq obj (symbol-value obj)))
-  (when (and (consp obj) 
-	     (keywordp (car obj))
-	     (eq :eval (car obj)))
-    (setq obj (eval (cadr obj))))
-  obj)
 
 \f
 ;;; Automatic resizing of temporary buffers.
Index: bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.113
diff -u -r1.113 bindings.el
--- bindings.el	31 Mar 2003 20:24:56 -0000	1.113
+++ bindings.el	1 Apr 2003 13:30:37 -0000
@@ -256,22 +256,20 @@
 (defvar mode-line-modes nil
   "Mode-line control for displaying major and minor modes.")
 
-(defvar mode-line-major-mode-keymap nil "\
+(defvar mode-line-major-mode-keymap 
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mode-line mouse-2] 'describe-mode)
+    (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
+    map) "\
 Keymap to display on major mode.")
 
-(defvar mode-line-minor-mode-keymap nil "\
+(defvar mode-line-minor-mode-keymap 
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mode-line mouse-2] 'mode-line-minor-mode-help)
+    (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
+    (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
+    map) "\
 Keymap to display on minor modes.")
-
-(let ((map (make-sparse-keymap)))
-  (define-key map [mode-line mouse-2] 'describe-mode)
-  (setq mode-line-major-mode-keymap map))
-
-;; Menu of minor modes.
-(let ((map (make-sparse-keymap)))
-  (define-key map [mode-line mouse-2] 'mode-line-minor-mode-help)
-  (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
-  (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
-  (setq mode-line-minor-mode-keymap map))
 
 (let* ((help-echo
 	;; The multi-line message doesn't work terribly well on the

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    I'd like to reflect the suggestions given by Stefan Monnier to my
    patch. I've already created new patch against the current Emacs source 
    code. I have to change the doc and NEWS file next.

Please do.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

It occurs to me that describe-minor-mode-from-indicator and
describe-minor-mode should be combined.  There should be one function
that accepts both kinds of arguments.  This should not be difficult
to do, and it would make things simpler.

Meanwhile, we were talking about how to make the completion list for
describe-minor-mode--which minor modes to include.  Someone described
it as a "menu", so I assumed we were talking about which minor modes
to display in the menu.  But I see it is really a completion list.
Definitely it should include all minor modes, not just the enabled ones.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

    It occurs to me that describe-minor-mode-from-indicator and
    describe-minor-mode should be combined.  There should be one function
    that accepts both kinds of arguments.  This should not be difficult
    to do, and it would make things simpler.

I did.

2003-04-07  Masatake YAMATO  <jet@gyve.org>

	* help.el (describe-minor-mode): New function implementation.
	Accept both minor mode string and minor mode indicator.
	(describe-minor-mode-completion-table-for-indicator)
	(describe-minor-mode-completion-table-for-symbol): New functions.
	(describe-minor-mode-from-symbol): renamed
	from (old) describe-minor-mode. Use 
	describe-minor-mode-completion-table-for-symbol.
	Don't use eval. Just use symbol-name.
	(describe-minor-mode-from-indicator): Document is updated.
	Use `format-mode-line'. Use
	describe-minor-mode-from-symbol instead of
	describe-minor-mode. 
	Use describe-minor-mode-completion-table-for-indicator.
	(expand-minor-mode-indicator-object): removed.
	(lookup-minor-mode-from-indicator): remove the fist white
	space from both indicator and anindicator before comparing them.

	* bindings.el (mode-line-major-mode-keymap)
	(mode-line-minor-mode-keymap): defined keys for the maps
	here in `defvar'.

Warning: Remote host denied X11 forwarding.
Index: lisp/help.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/help.el,v
retrieving revision 1.258
diff -u -r1.258 help.el
--- lisp/help.el	31 Mar 2003 20:22:58 -0000	1.258
+++ lisp/help.el	7 Apr 2003 15:44:54 -0000
@@ -612,72 +612,91 @@
       (print-help-return-message))))
 
 (defun describe-minor-mode (minor-mode)
-  "Display documentation of a minor mode given as MINOR-MODE."
+  "Display documentation of a minor mode given as MINOR-MODE.
+MINOR-MODE can be a minor mode symbol or a minor mode indicator string
+appeared on the mode-line."
+  (interactive (list (completing-read 
+		      "Minor mode: "
+			      (nconc
+			       (describe-minor-mode-completion-table-for-symbol)
+			       (describe-minor-mode-completion-table-for-indicator)
+			       ))))
+  (if (symbolp minor-mode)
+      (setq minor-mode (symbol-name minor-mode)))
+  (let ((symbols (describe-minor-mode-completion-table-for-symbol))
+	(indicators (describe-minor-mode-completion-table-for-indicator)))
+    (cond
+     ((member minor-mode symbols)
+      (describe-minor-mode-from-symbol (intern minor-mode)))
+     ((member minor-mode indicators)
+      (describe-minor-mode-from-indicator minor-mode))
+     (t
+      (error "No such minor mode: %s" minor-mode)))))
+
+;; symbol    
+(defun describe-minor-mode-completion-table-for-symbol ()
+  (delq nil (mapcar 
+	     (lambda (x) 
+	       (symbol-name (car x)))
+	     minor-mode-alist)))
+(defun describe-minor-mode-from-symbol (symbol)
+  "Display documentation of a minor mode given as a symbol, SYMBOL"
   (interactive (list (intern (completing-read 
-			      "Minor mode: "
-			      (delete nil (mapcar
-					   (function (lambda (x)
-						       (if (eval (car x))
-							   (symbol-name (car x)))))
-					   minor-mode-alist))))))
-  (if (fboundp minor-mode)
-      (describe-function minor-mode)
-    (describe-variable minor-mode)))
+			      "Minor mode symbol: "
+			      (describe-minor-mode-completion-table-for-symbol)))))
+  (if (fboundp symbol)
+      (describe-function symbol)
+    (describe-variable symbol)))
 
+;; indicator
+(defun describe-minor-mode-completion-table-for-indicator ()
+  (delq nil 
+	(mapcar (lambda (x)
+		  (let ((i (format-mode-line x)))
+		    ;; remove first space if existed
+		    (cond
+		     ((= 0 (length i))
+		      nil)
+		     ((eq (aref i 0) ?\ )
+		      (substring i 1))
+		     (t 
+		      i))))
+		minor-mode-alist)))
 (defun describe-minor-mode-from-indicator (indicator)
-  "Display documentation of a minor mode specified by INDICATOR."
+  "Display documentation of a minor mode specified by INDICATOR.
+If you call this function interactively, you can give indicator which
+is currently activated with completion."
   (interactive (list 
 		(completing-read 
 		 "Minor mode indicator: "
-		 (delete nil 
-			 (mapcar
-			  #'(lambda (x)
-			      (if (eval (car x))
-				  (let ((i (expand-minor-mode-indicator-object (cadr x))))
-				    (if (and (< 0 (length i))
-					     (string= " " (substring i 0 1)))
-					(substring i 1)
-				      i))))
-			  minor-mode-alist)))))
+		 (describe-minor-mode-completion-table-for-indicator))))
   (let ((minor-mode (lookup-minor-mode-from-indicator indicator)))
     (if minor-mode
-	(describe-minor-mode minor-mode)
+	(describe-minor-mode-from-symbol minor-mode)
       (error "Cannot find minor mode for `%s'" indicator))))
 
 (defun lookup-minor-mode-from-indicator (indicator)
   "Return a minor mode symbol from its indicator on the modeline."
+  ;; remove first space if existed
   (if (and (< 0 (length indicator)) 
-	   (not (string= " " (substring indicator 0 1))))
-      (setq indicator (concat " " indicator)))
+	   (eq (aref indicator 0) ?\ ))
+      (setq indicator (substring indicator 1)))
   (let ((minor-modes minor-mode-alist)
 	result)
     (while minor-modes
       (let* ((minor-mode (car (car minor-modes)))
-	     (anindicator (car (cdr (car minor-modes)))))
-	(setq anindicator (expand-minor-mode-indicator-object anindicator))
+	     (anindicator (format-mode-line 
+			   (car (cdr (car minor-modes))))))
+	;; remove first space if existed
 	(if (and (stringp anindicator) 
-		 (string= anindicator indicator))
+		 (> (length anindicator) 0)
+		 (eq (aref anindicator 0) ?\ ))
+	    (setq anindicator (substring anindicator 1)))
+	(if (equal indicator anindicator)
 	    (setq result minor-mode
 		  minor-modes nil)
 	  (setq minor-modes (cdr minor-modes)))))
     result))
-
-(defun expand-minor-mode-indicator-object (obj)
-  "Expand OBJ that represents a minor-mode indicator.
-cdr part of a `minor-mode-alist' element(indicator object) is the
-indicator of minor mode that is in car part.  Normally indicator
-object is a string. However, in some case it is more compound object
-like cons cell. This function tries to make the compound object a string."
-  ;; copied from describe-mode
-  (while (and obj (symbolp obj)
-	      (boundp obj)
-	      (not (eq obj (symbol-value obj))))
-    (setq obj (symbol-value obj)))
-  (when (and (consp obj) 
-	     (keywordp (car obj))
-	     (eq :eval (car obj)))
-    (setq obj (eval (cadr obj))))
-  obj)
 
 \f
 ;;; Automatic resizing of temporary buffers.
Index: lisp/bindings.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/bindings.el,v
retrieving revision 1.113
diff -u -r1.113 bindings.el
--- lisp/bindings.el	31 Mar 2003 20:24:56 -0000	1.113
+++ lisp/bindings.el	7 Apr 2003 15:44:55 -0000
@@ -256,22 +256,20 @@
 (defvar mode-line-modes nil
   "Mode-line control for displaying major and minor modes.")
 
-(defvar mode-line-major-mode-keymap nil "\
+(defvar mode-line-major-mode-keymap 
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mode-line mouse-2] 'describe-mode)
+    (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
+    map) "\
 Keymap to display on major mode.")
 
-(defvar mode-line-minor-mode-keymap nil "\
+(defvar mode-line-minor-mode-keymap 
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mode-line mouse-2] 'mode-line-minor-mode-help)
+    (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
+    (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
+    map) "\
 Keymap to display on minor modes.")
-
-(let ((map (make-sparse-keymap)))
-  (define-key map [mode-line mouse-2] 'describe-mode)
-  (setq mode-line-major-mode-keymap map))
-
-;; Menu of minor modes.
-(let ((map (make-sparse-keymap)))
-  (define-key map [mode-line mouse-2] 'mode-line-minor-mode-help)
-  (define-key map [mode-line down-mouse-3] 'mode-line-mode-menu-1)
-  (define-key map [header-line down-mouse-3] 'mode-line-mode-menu-1)
-  (setq mode-line-minor-mode-keymap map))
 
 (let* ((help-echo
 	;; The multi-line message doesn't work terribly well on the

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

It looks good.  You would need to merge it with the small changes
that I made yesterday which were designed to make describe-minor-mode
handle all the minor modes in minor-mode-alist.

Meanwhile, I found that all the minor modes in minor-mode-alist
is just a small subset of them.  It really ought to handle ALL
minor modes.  Which means we need a new list which would be the
list of all minor mode symbols.

Making add-minor-mode build such a list would get most of the way there.
So I will do that.  It's called minor-mode-list.

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

    It looks good.  You would need to merge it with the small changes
    that I made yesterday which were designed to make describe-minor-mode
    handle all the minor modes in minor-mode-alist.

    Meanwhile, I found that all the minor modes in minor-mode-alist
    is just a small subset of them.  It really ought to handle ALL
    minor modes.  Which means we need a new list which would be the
    list of all minor mode symbols.

    Making add-minor-mode build such a list would get most of the way there.
    So I will do that.  It's called minor-mode-list.

I think your change is nothing to do with indicator related functions
(describe-minor-mode-completion-table-for-indicator, 
 describe-minor-mode-from-indicator,
 lookup-minor-mode-from-indicator).

I changed describe-minor-mode-completion-table-for-symbol only.

(defun describe-minor-mode-completion-table-for-symbol ()
  ;; In order to list up all minor modes, minor-mode-list
  ;; is used here instead of minor-mode-alist.
  (delq nil (mapcar 
	     (lambda (x) 
	       (symbol-name x))
	     minor-mode-list)))

If, Ok, I'll install to the CVS repository.

Masatake YAMATO

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Stefan Monnier]

>   (delq nil (mapcar 
> 	     (lambda (x) 
> 	       (symbol-name x))
> 	     minor-mode-list)))

Aka
    (mapcar 'symbol-name minor-mode-list)


-- Stefan

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

    I think your change is nothing to do with indicator related functions
    (describe-minor-mode-completion-table-for-indicator, 
     describe-minor-mode-from-indicator,
     lookup-minor-mode-from-indicator).

That is true, but didn't you merge describe-minor-mode-from-indicator
with describe-minor-mode into one combined function?

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Masatake YAMATO]

>     I think your change is nothing to do with indicator related functions
>     (describe-minor-mode-completion-table-for-indicator, 
>      describe-minor-mode-from-indicator,
>      lookup-minor-mode-from-indicator).
> 
> That is true, but didn't you merge describe-minor-mode-from-indicator
> with describe-minor-mode into one combined function?

Yes, I did. But new describe-minor-mode calls both
describe-minor-mode-completion-table-for-symbol and 
describe-minor-mode-completion-table-for-indicator.
So I asked above question to you.
I wanted to make sure that your change(introducing minor-mode-list)
affects on describe-minor-mode-completion-table-for-symbol, not
on describe-minor-mode-completion-table-for-indicator befre installing.

Could I install?

Masatake YAMATO

Re: |PATCH| describe-minor-mode and describe-minor-mode-from-indicator

[Richard Stallman]

Please do install the change.