Subject: Re: [patch] convert footnote.el to use define-minor-mode

From: Eduard Wiebe <[email-masked]>
Date: Sun, 14 Dec 2008 00:46:14 +0100

Severity: wishlist

[ resent from
  http://lists.gnu.org/archive/html/emacs-devel/2008-12/msg00534.html ]

Stefan Monnier <[email-masked]> writes:

>> 	* mail/footnote.el (footnote-mode): Use define-minor-mode
>> 	(footnote-prefix): Use defcustom
>
> Looks very good, thank you.  Except you've renamed footnote-prefix to
> footnote-mode-prefix, which doesn't seem to be worth the trouble (of
> and ChangeLog text should end with ".").

Stefan, thank you for comments.

2008-12-14  Eduard Wiebe  <[email-masked]>

	* mail/footnote.el (footnote-mode): Use define-minor-mode.
	(footnote-prefix): Use defcustom.

diff --git a/lisp/mail/footnote.el b/lisp/mail/footnote.el
index 506f6e7..a7b372b 100644
--- a/lisp/mail/footnote.el
+++ b/lisp/mail/footnote.el
@@ -45,15 +45,6 @@
   :version "21.1"
   :group 'message)
 
-(defcustom footnote-mode-line-string " FN"
-  "String to display in modes section of the mode-line."
-  :group 'footnote)
-
-(defcustom footnote-mode-hook nil
-  "Hook functions run when footnote-mode is activated."
-  :type 'hook
-  :group 'footnote)
-
 (defcustom footnote-narrow-to-footnotes-when-editing nil
   "If non-nil, narrow to footnote text body while editing a footnote."
   :type 'boolean
@@ -84,8 +75,12 @@ displaying footnotes."
   :type 'integer
   :group 'footnote)
 
-(defvar footnote-prefix [(control ?c) ?!]
-  "*When not using `message-mode', the prefix to bind in `mode-specific-map'")
+(defcustom footnote-prefix "\C-c!"
+  "Prefix key to use for Footnote command in Footnote minor mode.
+The value of this variable is checked as part of loading Footnote mode.
+After that, changing the prefix key requires manipulating keymaps."
+  :type  'string
+  :group 'footnote)
 
 ;;; Interface variables that probably shouldn't be changed
 
@@ -143,10 +138,6 @@ has no effect on buffers already displaying footnotes."
 (defvar footnote-mouse-highlight 'highlight
   "Text property name to enable mouse over highlight.")
 
-(defvar footnote-mode nil
-  "Variable indicating whether footnote minor mode is active.")
-(make-variable-buffer-local 'footnote-mode)
-
 ;;; Default styles
 ;;; NUMERIC
 (defconst footnote-numeric-regexp "[0-9]+"
@@ -743,47 +734,31 @@ being set it is automatically widened."
 	(widen))
       (goto-char (cadr (assq note footnote-pointer-marker-alist))))))
 
-(defvar footnote-mode-map nil
-  "Keymap used for footnote minor mode.")
-
-;; Set up our keys
-(unless footnote-mode-map
-  (setq footnote-mode-map (make-sparse-keymap))
-  (define-key footnote-mode-map "a" 'Footnote-add-footnote)
-  (define-key footnote-mode-map "b" 'Footnote-back-to-message)
-  (define-key footnote-mode-map "c" 'Footnote-cycle-style)
-  (define-key footnote-mode-map "d" 'Footnote-delete-footnote)
-  (define-key footnote-mode-map "g" 'Footnote-goto-footnote)
-  (define-key footnote-mode-map "r" 'Footnote-renumber-footnotes)
-  (define-key footnote-mode-map "s" 'Footnote-set-style))
-
-(defvar footnote-minor-mode-map nil
-  "Keymap used for binding footnote minor mode.")
-
-(unless footnote-minor-mode-map
-  (define-key global-map footnote-prefix footnote-mode-map))
+(defvar footnote-mode-map
+  (let ((map (make-sparse-keymap))
+	(prefix-map (make-sparse-keymap)))
+    (define-key map footnote-prefix prefix-map)
+    (define-key prefix-map "a" 'Footnote-add-footnote)
+    (define-key prefix-map "b" 'Footnote-back-to-message)
+    (define-key prefix-map "c" 'Footnote-cycle-style)
+    (define-key prefix-map "d" 'Footnote-delete-footnote)
+    (define-key prefix-map "g" 'Footnote-goto-footnote)
+    (define-key prefix-map "r" 'Footnote-renumber-footnotes)
+    (define-key prefix-map "s" 'Footnote-set-style)
+    map))
 
 ;;;###autoload
-(defun footnote-mode (&optional arg)
+(define-minor-mode footnote-mode
   "Toggle footnote minor mode.
-\\<message-mode-map>
 This minor mode provides footnote support for `message-mode'.  To get
 started, play around with the following keys:
-key		binding
----		-------
-\\[Footnote-add-footnote]		Footnote-add-footnote
-\\[Footnote-back-to-message]		Footnote-back-to-message
-\\[Footnote-delete-footnote]		Footnote-delete-footnote
-\\[Footnote-goto-footnote]		Footnote-goto-footnote
-\\[Footnote-renumber-footnotes]		Footnote-renumber-footnotes
-\\[Footnote-cycle-style]		Footnote-cycle-style
-\\[Footnote-set-style]		Footnote-set-style
+\\{footnote-mode-map}
 "
-  (interactive "*P")
+  :init-value nil
+  :lighter    " FN"
+  :keymap     footnote-mode-map
+  :group     'footnote
   ;; (filladapt-mode t)
-  (setq footnote-mode
-	(if (null arg) (not footnote-mode)
-	  (> (prefix-numeric-value arg) 0)))
   (when footnote-mode
     ;; (Footnote-setup-keybindings)
     (make-local-variable 'footnote-style)
@@ -808,14 +783,7 @@ key		binding
 	(unless (assoc bullet-regexp filladapt-token-table)
 	  (setq filladapt-token-table
 		(append filladapt-token-table
-			(list (list bullet-regexp 'bullet)))))))
-
-    (run-hooks 'footnote-mode-hook)))
-
-(unless (assq 'footnote-mode minor-mode-alist)
-  (setq minor-mode-alist
-	(cons '(footnote-mode footnote-mode-line-string)
-	      minor-mode-alist)))
+			(list (list bullet-regexp 'bullet)))))))))
 
 (provide 'footnote)

-- 
Eduard Wiebe