Subject: Re: bug#6539: ispell-complete-word looks for wrong file (at least on w32)

From: Agustin Martin <[email-masked]>
Date: Tue, 6 Jul 2010 17:12:45 +0200

On Mon, Jul 05, 2010 at 08:23:49PM +0300, Eli Zaretskii wrote:
> > Date: Mon, 5 Jul 2010 13:52:22 +0200
> > From: Agustin Martin <[email-masked]>
> > Cc: 
> > 
> > However, docstrings and error handling for this complete-word stuff needs 
> > to be improved, so it provides useful and accurate information. I am 
> > currently trying something like attached patch where 
> > `ispell-alternate-dictionary' is set to nil if no default is found and 
> > checked before use. I find error message too long, but better that before.
> > As usually, suggestions are welcome.
> 
> For the error message, just put 2 spaces between the sentences, as we
> do everywhere in Emacs.

Thanks, that is the kind of things casual contributors like me usually
forget.

> For the rest, the patch looks fine, but why not test /usr/dict/words
> with file-readable-p as well?  Then, if it does not exist, the
> defcustom will evaluate to nil, and the error message will be even
> more to the point.

Seems already checked. Last /usr/dict/words was originally the return value
when nothing is found and has been removed.

Regarding the `ispell-alternate-dictionary'/`ispell-complete-word-dict'
duality I am modifying `ispell-complete-word-dict' default value to nil and
using 

  (or ispell-complete-word-dict
      ispell-alternate-dictionary))

for previous standalone calls of any of both variables. That way
`ispell-complete-word-dict' is only used if explicitly set and
`ispell-alternate-dictionary' is used otherwise. If they are ever used 
for something different this should not disturb and leave things ready.

For those curious, I am attaching a diff with my current changes. Will check
a bit more and if no further problems are found will commit to trunk
tomorrow to allow for wider checking of changes.

-- 
Agustin
--- ispell.el.orig	2010-07-05 11:31:21.000000000 +0200
+++ ispell.el	2010-07-06 16:24:00.000000000 +0200
@@ -357,21 +357,21 @@
   :group 'ispell)
 
 (defcustom ispell-alternate-dictionary
-  (cond ((file-exists-p "/usr/dict/web2") "/usr/dict/web2")
-	((file-exists-p "/usr/share/dict/web2") "/usr/share/dict/web2")
-	((file-exists-p "/usr/dict/words") "/usr/dict/words")
-	((file-exists-p "/usr/lib/dict/words") "/usr/lib/dict/words")
-	((file-exists-p "/usr/share/dict/words") "/usr/share/dict/words")
-	((file-exists-p "/usr/share/lib/dict/words")
+  (cond ((file-readable-p "/usr/dict/web2") "/usr/dict/web2")
+	((file-readable-p "/usr/share/dict/web2") "/usr/share/dict/web2")
+	((file-readable-p "/usr/dict/words") "/usr/dict/words")
+	((file-readable-p "/usr/lib/dict/words") "/usr/lib/dict/words")
+	((file-readable-p "/usr/share/dict/words") "/usr/share/dict/words")
+	((file-readable-p "/usr/share/lib/dict/words")
 	 "/usr/share/lib/dict/words")
-	((file-exists-p "/sys/dict") "/sys/dict")
-	(t "/usr/dict/words"))
-  "*Alternate dictionary for spelling help."
+	((file-readable-p "/sys/dict") "/sys/dict"))
+  "*Alternate plain word-list dictionary for spelling help."
   :type '(choice file (const :tag "None" nil))
   :group 'ispell)
 
-(defcustom ispell-complete-word-dict ispell-alternate-dictionary
-  "*Dictionary used for word completion."
+(defcustom ispell-complete-word-dict nil
+  "*Plain word-list dictionary used for word completion if
+different from `ispell-alternate-dictionary'."
   :type '(choice file (const :tag "None" nil))
   :group 'ispell)
 
@@ -2049,10 +2049,11 @@
 			      (erase-buffer)
 			      (setq count ?0
 				    skipped 0
-				    mode-line-format
+				    mode-line-format ;; setup the *Choices* buffer with valid data.
 				    (concat "--  %b  --  word: " new-word
-					    "  --  dict: "
-					    ispell-alternate-dictionary)
+					    "  --  word-list: "
+					    (or ispell-complete-word-dict
+						ispell-alternate-dictionary))
 				    miss (lookup-words new-word)
 				    choices miss
 				    line ispell-choices-win-default-height)
@@ -2267,11 +2268,20 @@
 search for the words (usually egrep).
 
 Optional second argument contains the dictionary to use; the default is
-`ispell-alternate-dictionary'."
+`ispell-alternate-dictionary', overriden by `ispell-complete-word-dict'
+if defined."
   ;; We don't use the filter for this function, rather the result is written
   ;; into a buffer.  Hence there is no need to save the filter values.
   (if (null lookup-dict)
-      (setq lookup-dict ispell-alternate-dictionary))
+      (setq lookup-dict (or ispell-complete-word-dict
+			    ispell-alternate-dictionary)))
+
+  (if lookup-dict
+      (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.")))
 
   (let* ((process-connection-type ispell-use-ptys-p)
 	 (wild-p (string-match "\\*" word))
@@ -3342,7 +3352,8 @@
 	      (lookup-words (concat (and interior-frag "*") word
 				    (if (or interior-frag (null ispell-look-p))
 					"*"))
-			    ispell-complete-word-dict)))
+			    (or ispell-complete-word-dict
+				ispell-alternate-dictionary))))
     (cond ((eq possibilities t)
 	   (message "No word to complete"))
 	  ((null possibilities)