Avoid global recursive calls to kill-buffer-hooks, and fit into 80 cols.

* lisp/textmodes/ispell.el (ispell-process-buffer-name): Remove.
(ispell-start-process): Avoid setq and simplify logic.
(ispell-init-process): Setup kill-buffer-hook locally when needed.
(kill-buffer-hook): Don't use it globally with code that uses
expand-file-name since that may call kill-buffer via
code_conversion_restore.

2 files changed, 114 insertions, 94 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 28092a6a086..c463614a34b 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,13 @@
+2010-09-04  Stefan Monnier  <monnier@iro.umontreal.ca>
+
+        Avoid global recursive calls to kill-buffer-hooks, and fit into 80 cols.
+        * textmodes/ispell.el (ispell-process-buffer-name): Remove.
+        (ispell-start-process): Avoid setq and simplify logic.
+        (ispell-init-process): Setup kill-buffer-hook locally when needed.
+        (kill-buffer-hook): Don't use it globally with code that uses
+        expand-file-name since that may call kill-buffer via
+        code_conversion_restore.
+
 2010-09-04  Noorul Islam K M  <noorul@noorul.com>  (tiny change)
 
         * emacs-lisp/package.el (package-directory-list): Only call
@@ -5,8 +15,8 @@
 
 2010-09-02  Chong Yidong  <cyd@stupidchicken.com>
 
-        * emacs-lisp/package.el (package--download-one-archive): Ensure
-        that archive-contents is valid before saving it.
+        * emacs-lisp/package.el (package--download-one-archive):
+        Ensure that archive-contents is valid before saving it.
         (package-activate-1, package-mark-obsolete, define-package)
         (package-compute-transaction, package-list-maybe-add): Use push.
 
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 64253593a79..e30da02df4f 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -221,10 +221,10 @@ compatibility function in case `version<=' is not available."
         (let (ver mver)
           (if (string-match "[0-9]+" version start-ver)
               (setq start-ver (match-end 0)
-                    ver (string-to-number (substring version (match-beginning 0) (match-end 0)))))
+                    ver (string-to-number (match-string 0 version))))
           (if (string-match "[0-9]+" minver start-mver)
               (setq start-mver (match-end 0)
-                    mver (string-to-number (substring minver (match-beginning 0) (match-end 0)))))
+                    mver (string-to-number (match-string 0 minver))))
 
           (if (or ver mver)
               (progn
@@ -310,7 +310,9 @@ Warning!  Not checking comments, when a comment start is embedded in strings,
 may produce undesired results."
   :type '(choice (const exclusive) (const :tag "off" nil) (const :tag "on" t))
   :group 'ispell)
-;;;###autoload(put 'ispell-check-comments 'safe-local-variable (lambda (a) (memq a '(nil t exclusive))))
+;;;###autoload
+(put 'ispell-check-comments 'safe-local-variable
+     (lambda (a) (memq a '(nil t exclusive))))
 
 (defcustom ispell-query-replace-choices nil
   "*Corrections made throughout region when non-nil.
@@ -514,7 +516,8 @@ is automatically set when defined in the file with either
   :type '(choice string
                  (const :tag "default" nil))
   :group 'ispell)
-;;;###autoload(put 'ispell-local-dictionary 'safe-local-variable 'string-or-null-p)
+;;;###autoload
+(put 'ispell-local-dictionary 'safe-local-variable 'string-or-null-p)
 
 (make-variable-buffer-local 'ispell-local-dictionary)
 
@@ -738,8 +741,8 @@ Note that the CASECHARS and OTHERCHARS slots of the alist should
 contain the same character set as casechars and otherchars in the
 LANGUAGE.aff file \(e.g., english.aff\).")
 
-(defvar ispell-really-aspell nil)   ; Non-nil if aspell extensions should be used
-(defvar ispell-really-hunspell nil) ; Non-nil if hunspell extensions should be used
+(defvar ispell-really-aspell nil)   ; Non-nil if we can use aspell extensions.
+(defvar ispell-really-hunspell nil) ; Non-nil if we can use hunspell extensions.
 (defvar ispell-encoding8-command nil
   "Command line option prefix to select UTF-8 if supported, nil otherwise.
 If UTF-8 if supported by spellchecker and is selectable from the command line
@@ -962,7 +965,8 @@ Internal use.")
         (setq found (nconc found (list dict)))))
     (setq ispell-aspell-dictionary-alist found)
     ;; Add a default entry
-    (let ((default-dict '(nil "[[:alpha:]]" "[^[:alpha:]]" "[']" nil ("-B") nil utf-8)))
+    (let ((default-dict
+           '(nil "[[:alpha:]]" "[^[:alpha:]]" "[']" nil ("-B") nil utf-8)))
       (push default-dict ispell-aspell-dictionary-alist))))
 
 (defvar ispell-aspell-data-dir nil
@@ -1026,7 +1030,8 @@ Assumes that value contains no whitespace."
 (defun ispell-aspell-add-aliases (alist)
   "Find aspell's dictionary aliases and add them to dictionary ALIST.
 Return the new dictionary alist."
-  (let ((aliases (file-expand-wildcards
+  (let ((aliases
+         (file-expand-wildcards
                   (concat (or ispell-aspell-dict-dir
                               (setq ispell-aspell-dict-dir
                                     (ispell-get-aspell-config-value "dict-dir")))
@@ -1168,7 +1173,8 @@ The variable `ispell-library-directory' defines the library location."
         `(menu-item ,(purecopy "Complete Word") ispell-complete-word
                     :help ,(purecopy "Complete word at cursor using dictionary")))
       (define-key ispell-menu-map [ispell-complete-word-interior-frag]
-        `(menu-item ,(purecopy "Complete Word Fragment") ispell-complete-word-interior-frag
+        `(menu-item ,(purecopy "Complete Word Fragment")
+                    ispell-complete-word-interior-frag
                     :help ,(purecopy "Complete word fragment at cursor")))))
 
 ;;;###autoload
@@ -1185,7 +1191,8 @@ The variable `ispell-library-directory' defines the library location."
         `(menu-item ,(purecopy "Spell-Check Word") ispell-word
                     :help ,(purecopy "Spell-check word at cursor")))
       (define-key ispell-menu-map [ispell-comments-and-strings]
-        `(menu-item ,(purecopy "Spell-Check Comments") ispell-comments-and-strings
+        `(menu-item ,(purecopy "Spell-Check Comments")
+                    ispell-comments-and-strings
                     :help ,(purecopy "Spell-check only comments and strings")))))
 
 ;;;###autoload
@@ -1334,9 +1341,6 @@ Protects against bogus binding of `enable-multibyte-characters' in XEmacs."
 (defvar ispell-process-directory nil
   "The directory where `ispell-process' was started.")
 
-(defvar ispell-process-buffer-name nil
-  "The buffer where `ispell-process' was started.")
-
 (defvar ispell-filter nil
   "Output filter from piped calls to Ispell.")
 
@@ -1400,7 +1404,8 @@ The last occurring definition in the buffer will be used.")
     (ispell-dictionary-keyword     forward-line)
     (ispell-pdict-keyword          forward-line)
     (ispell-parsing-keyword        forward-line)
-    (,(purecopy "^---*BEGIN PGP [A-Z ]*--*") . ,(purecopy "^---*END PGP [A-Z ]*--*"))
+    (,(purecopy "^---*BEGIN PGP [A-Z ]*--*")
+     . ,(purecopy "^---*END PGP [A-Z ]*--*"))
     ;; assume multiline uuencoded file? "\nM.*$"?
     (,(purecopy "^begin [0-9][0-9][0-9] [^ \t]+$") . ,(purecopy "\nend\n"))
     (,(purecopy "^%!PS-Adobe-[123].0")   . ,(purecopy "\n%%EOF\n"))
@@ -1880,9 +1885,10 @@ Global `ispell-quit' set to start location to continue spell session."
     ;; setup the *Choices* buffer with valid data.
     (with-current-buffer (get-buffer-create ispell-choices-buffer)
       (setq mode-line-format
-            (concat "--  %b  --  word: " word
-                    "  --  dict: " (or ispell-current-dictionary "default")
-                    "  --  prog: " (file-name-nondirectory ispell-program-name)))
+            (concat
+             "--  %b  --  word: " word
+             "  --  dict: " (or ispell-current-dictionary "default")
+             "  --  prog: " (file-name-nondirectory ispell-program-name)))
       ;; XEmacs: no need for horizontal scrollbar in choices window
       (with-no-warnings
        (and (fboundp 'set-specifier)
@@ -2280,8 +2286,9 @@ if defined."
       (unless (file-readable-p lookup-dict)
         (error "lookup-words error: Unreadable or missing plain word-list %s."
                lookup-dict))
-    (error (concat "lookup-words error: No plain word-list found at system default "
-                   "locations.  Customize `ispell-alternate-dictionary' to set yours.")))
+    (error (concat "lookup-words error: No plain word-list found at system"
+                   "default locations.  "
+                   "Customize `ispell-alternate-dictionary' to set yours.")))
 
   (let* ((process-connection-type ispell-use-ptys-p)
          (wild-p (string-match "\\*" word))
@@ -2332,16 +2339,16 @@ if defined."
     results))
 
 
-;;; "ispell-filter" is a list of output lines from the generating function.
-;;;   Each full line (ending with \n) is a separate item on the list.
-;;; "output" can contain multiple lines, part of a line, or both.
-;;; "start" and "end" are used to keep bounds on lines when "output" contains
-;;;   multiple lines.
-;;; "ispell-filter-continue" is true when we have received only part of a
-;;;   line as output from a generating function ("output" did not end with \n)
-;;; THIS FUNCTION WILL FAIL IF THE PROCESS OUTPUT DOESN'T END WITH \n!
-;;;   This is the case when a process dies or fails. The default behavior
-;;;   in this case treats the next input received as fresh input.
+;; "ispell-filter" is a list of output lines from the generating function.
+;;   Each full line (ending with \n) is a separate item on the list.
+;; "output" can contain multiple lines, part of a line, or both.
+;; "start" and "end" are used to keep bounds on lines when "output" contains
+;;   multiple lines.
+;; "ispell-filter-continue" is true when we have received only part of a
+;;   line as output from a generating function ("output" did not end with \n)
+;; THIS FUNCTION WILL FAIL IF THE PROCESS OUTPUT DOESN'T END WITH \n!
+;;   This is the case when a process dies or fails. The default behavior
+;;   in this case treats the next input received as fresh input.
 
 (defun ispell-filter (process output)
   "Output filter function for ispell, grep, and look."
@@ -2573,37 +2580,34 @@ When asynchronous processes are not supported, `run' is always returned."
 (defun ispell-start-process ()
   "Start the ispell process, with support for no asynchronous processes.
 Keeps argument list for future ispell invocations for no async support."
-  (let ((default-directory default-directory)
-        args)
-    (unless (and (file-directory-p default-directory)
-                 (file-readable-p default-directory))
-      ;; Defend against bad `default-directory'.
-      (setq default-directory (expand-file-name "~/")))
-    ;; Local dictionary becomes the global dictionary in use.
-    (setq ispell-current-dictionary
-          (or ispell-local-dictionary ispell-dictionary))
-    (setq ispell-current-personal-dictionary
-          (or ispell-local-pdict ispell-personal-dictionary))
-    (setq args (ispell-get-ispell-args))
-    (if (and ispell-current-dictionary  ; use specified dictionary
-             (not (member "-d" args)))  ; only define if not overridden
-        (setq args
-              (append (list "-d" ispell-current-dictionary) args)))
-    (if ispell-current-personal-dictionary      ; use specified pers dict
-        (setq args
-              (append args
-                      (list "-p"
-                            (expand-file-name ispell-current-personal-dictionary)))))
-
-    ;; If we are using recent aspell or hunspell, make sure we use the right encoding
-    ;; for communication. ispell or older aspell/hunspell does not support this
-    (if ispell-encoding8-command
-        (setq args
-              (append args
-                      (list
-                       (concat ispell-encoding8-command
-                               (symbol-name (ispell-get-coding-system)))))))
-    (setq args (append args ispell-extra-args))
+  ;; Local dictionary becomes the global dictionary in use.
+  (setq ispell-current-dictionary
+        (or ispell-local-dictionary ispell-dictionary))
+  (setq ispell-current-personal-dictionary
+        (or ispell-local-pdict ispell-personal-dictionary))
+  (let* ((default-directory
+           (if (and (file-directory-p default-directory)
+                    (file-readable-p default-directory))
+               default-directory
+             ;; Defend against bad `default-directory'.
+             (expand-file-name "~/")))
+         (args
+          (append
+           (if (and ispell-current-dictionary ; Use specified dictionary.
+                    (not (member "-d" args))) ; Only define if not overridden.
+               (list "-d" ispell-current-dictionary))
+           (ispell-get-ispell-args)
+           (if ispell-current-personal-dictionary ; Use specified pers dict.
+               (list "-p"
+                     (expand-file-name ispell-current-personal-dictionary)))
+           ;; If we are using recent aspell or hunspell, make sure we use the
+           ;; right encoding for communication. ispell or older aspell/hunspell
+           ;; does not support this.
+           (if ispell-encoding8-command
+               (list
+                (concat ispell-encoding8-command
+                        (symbol-name (ispell-get-coding-system)))))
+           ispell-extra-args)))
 
     ;; Initially we don't know any buffer's local words.
     (setq ispell-buffer-local-name nil)
@@ -2612,9 +2616,11 @@ Keeps argument list for future ispell invocations for no async support."
         (let ((process-connection-type ispell-use-ptys-p))
           (apply 'start-process
                  "ispell" nil ispell-program-name
-                 "-a"                                ; accept single input lines
-                 (if ispell-really-hunspell "" "-m") ; make root/affix combos not in dict
-                 args))                              ; hunspell -m option means different
+                 "-a"                   ; Accept single input lines.
+                 ;; Make root/affix combos not in dict.
+                 ;; hunspell -m option means different.
+                 (if ispell-really-hunspell "" "-m")
+                 args))
       (setq ispell-cmd-args args
             ispell-output-buffer (generate-new-buffer " *ispell-output*")
             ispell-session-buffer (generate-new-buffer " *ispell-session*"))
@@ -2650,10 +2656,11 @@ Keeps argument list for future ispell invocations for no async support."
     ;; Check if process needs restart
     (if (and ispell-process
              (eq (ispell-process-status) 'run)
-             ;; Unless we are using an explicit personal dictionary,
-             ;; ensure we're in the same default directory!
-             ;; Restart check for personal dictionary is done in
-             ;; `ispell-internal-change-dictionary', called from `ispell-buffer-local-dict'
+             ;; Unless we are using an explicit personal dictionary, ensure
+             ;; we're in the same default directory!  Restart check for
+             ;; personal dictionary is done in
+             ;; `ispell-internal-change-dictionary', called from
+             ;; `ispell-buffer-local-dict'
              (or (or ispell-local-pdict ispell-personal-dictionary)
                  (equal ispell-process-directory default-directory)))
         (setq ispell-filter nil ispell-filter-continue nil)
@@ -2667,17 +2674,25 @@ Keeps argument list for future ispell invocations for no async support."
             ispell-filter nil
             ispell-filter-continue nil
             ispell-process-directory default-directory)
-      ;; When spellchecking minibuffer contents, assign ispell process to parent
-      ;; buffer if known (not known for XEmacs).  Use (buffer-name) otherwise.
-      (setq ispell-process-buffer-name
+
+      ;; Kill ispell process when killing its associated buffer if using Ispell
+      ;; per-directory personal dictionaries.
+      (unless (equal ispell-process-directory (expand-file-name "~/"))
+        (with-current-buffer
             (if (and (window-minibuffer-p)
-                     (fboundp 'minibuffer-selected-window)) ;; Not XEmacs
+                     (fboundp 'minibuffer-selected-window)) ;; E.g. XEmacs.
+                ;; When spellchecking minibuffer contents, assign ispell
+                ;; process to parent buffer if known (not known for XEmacs).
+                ;; Use (buffer-name) otherwise.
                 (window-buffer (minibuffer-selected-window))
-              (buffer-name)))
+              (current-buffer))
+          (add-hook 'kill-buffer-hook (lambda () (ispell-kill-ispell t))
+                    nil 'local)))
 
       (if ispell-async-processp
           (set-process-filter ispell-process 'ispell-filter))
-      ;; protect against bogus binding of `enable-multibyte-characters' in XEmacs
+      ;; protect against bogus binding of `enable-multibyte-characters' in
+      ;; XEmacs.
       (if (and (or (featurep 'xemacs)
                    (and (boundp 'enable-multibyte-characters)
                         enable-multibyte-characters))
@@ -2735,19 +2750,10 @@ With NO-ERROR, just return non-nil if there was no Ispell running."
       (kill-buffer ispell-session-buffer)
       (setq ispell-output-buffer nil
             ispell-session-buffer nil))
-    (setq ispell-process-buffer-name nil)
     (setq ispell-process nil)
     (message "Ispell process killed")
     nil))
 
-;; Kill ispell process when killing its associated buffer if using Ispell
-;; per-directory personal dictionaries.
-(add-hook 'kill-buffer-hook
-          '(lambda ()
-             (if (and (not (equal ispell-process-directory (expand-file-name "~/")))
-                      (equal ispell-process-buffer-name (buffer-name)))
-                 (ispell-kill-ispell t))))
-
 ;;; ispell-change-dictionary is set in some people's hooks.  Maybe this should
 ;;;  call ispell-init-process rather than wait for a spell checking command?
 
@@ -2844,9 +2850,10 @@ Return nil if spell session is quit,
                   (set-marker skip-region-start (- (point) (length key)))
                   (goto-char reg-start)))
             (let (message-log-max)
-              (message "Continuing spelling check using %s with %s dictionary..."
-                       (file-name-nondirectory ispell-program-name)
-                       (or ispell-current-dictionary "default")))
+              (message
+               "Continuing spelling check using %s with %s dictionary..."
+               (file-name-nondirectory ispell-program-name)
+               (or ispell-current-dictionary "default")))
             (set-marker rstart reg-start)
             (set-marker ispell-region-end reg-end)
             (while (and (not ispell-quit)
@@ -3111,9 +3118,9 @@ Point is placed at end of skipped region."
           (sit-for 2)))))
 
 
-;;; Grab the next line of data.
-;;; Returns a string with the line data
 (defun ispell-get-line (start end in-comment)
+  "Grab the next line of data.
+Returns a string with the line data."
   (let ((ispell-casechars (ispell-get-casechars))
         string)
     (cond                               ; LOOK AT THIS LINE AND SKIP OR PROCESS
@@ -3140,7 +3147,8 @@ Point is placed at end of skipped region."
                                        (point) (+ (point) len))
                                       coding)))))
 
-;;; Avoid error messages when compiling for these dynamic variables.
+;; Avoid error messages when compiling for these dynamic variables.
+;; FIXME: dynamically scoped vars should have an "ispell-" prefix.
 (defvar start)
 (defvar end)
 
@@ -3275,10 +3283,12 @@ Returns the sum SHIFT due to changes in word replacements."
               ;;                           (length (car poss)))))
               ))
             (if (not ispell-quit)
+                ;; FIXME: remove redundancy with identical code above.
                 (let (message-log-max)
-                  (message "Continuing spelling check using %s with %s dictionary..."
-                           (file-name-nondirectory ispell-program-name)
-                           (or ispell-current-dictionary "default"))))
+                  (message
+                   "Continuing spelling check using %s with %s dictionary..."
+                   (file-name-nondirectory ispell-program-name)
+                   (or ispell-current-dictionary "default"))))
             (sit-for 0)
             (setq start (marker-position line-start)
                   end (marker-position line-end))
@@ -3351,7 +3361,7 @@ Returns the sum SHIFT due to changes in word replacements."
 
 
 ;;; Interactive word completion.
-;;; Forces "previous-word" processing.  Do we want to make this selectable?
+;; Forces "previous-word" processing.  Do we want to make this selectable?
 
 ;;;###autoload
 (defun ispell-complete-word (&optional interior-frag)