bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Hello,

On Windows if one uses cygwin ispell, then one should fix ispell LIBDIR 
(which is not correct absolute path) relative to cygwin root by setting 
`ispell-library-directory'. Or one also might wish to use custom 
location of dictionaries.
Currently ispell.el breaks this possibiliity, because it resets
`ispell-library-directory' to possibly incorrect or unwanted ispell 
LIBDIR on every ispell call. Suggested patch attached.

I'm not sure if it doesn't break something (that's why i'm asking in 
patch comments), but AFAICS it shouldn't.
Could someone familiar with ispell.el overview and comment on this tiny 
patch?

[-- Attachment #2: ispell.patch --]
[-- Type: text/plain, Size: 1135 bytes --]

--- ispell.el.orig	2015-04-02 10:23:06.000000000 +0300
+++ ispell.el	2016-09-14 22:36:58.446548500 +0300
@@ -3044,7 +3044,17 @@
 	       ispell-program-name
 	       (or ispell-local-dictionary ispell-dictionary "default"))
       (sit-for 0)
-      (setq ispell-library-directory (ispell-check-version)
+      (setq
+        ;; Why do we need to update `ispell-library-directory' on every 
+        ;; ispell process launch? If we do not change `ispell-program-name', 
+        ;; then `ispell-library-directory' should also not be changed once 
+        ;; it is set/initialized in `ispell-set-spellchecker-params'.
+        ;; Besides, this breaks custom setting `ispell-library-directory' 
+        ;; (e.g. cygwin ispell with LIBDIR relative to cygwin root, that 
+        ;; should be fixed; custom location of dictionaries), because it 
+        ;; resets `ispell-library-directory' to (possibly incorrect, 
+        ;; unwanted) ispell LIBDIR on every ispell call.
+        ;; ispell-library-directory (ispell-check-version) 
 	    ispell-process (ispell-start-process)
 	    ispell-filter nil
 	    ispell-filter-continue nil

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Date: Wed, 14 Sep 2016 23:02:03 +0300
> 
> On Windows if one uses cygwin ispell, then one should fix ispell LIBDIR 
> (which is not correct absolute path) relative to cygwin root by setting 
> `ispell-library-directory'. Or one also might wish to use custom 
> location of dictionaries.

ispell-library-directory is not supposed to be customized by users,
it's an internal variable set and used by ispell.el.  To change the
location of dictionaries, users are supposed to customize
ispell-local-dictionary-alist.  Why can't you do that instead?

> Currently ispell.el breaks this possibiliity, because it resets
> `ispell-library-directory' to possibly incorrect or unwanted ispell 
> LIBDIR on every ispell call.

That's not what I see.  What I see is that ispell-library-directory is
set once whenever the ispell process is created, which happens only
once in a session, or when you change your ispell dictionary.  So I
think this is completely justified, because the user could install
additional dictionaries before restarting ispell.

> Could someone familiar with ispell.el overview and comment on this tiny 
> patch?

I would prefer not to mess with the internal workings of ispell.el, if
possible.

If the suggestions above don't solve your problems, perhaps you could
write a batch file or a script which reported the library directory in
the format that ispell.el expects in a native Windows build of Emacs.
Or maybe you could switch to Hunspell or Aspell, which don't need
ispell-library-directory at all.  A native Windows port of Hunspell is
available (and works very well for me).

Thanks.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Eli Zaretskii wrote:
>> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
>> Date: Wed, 14 Sep 2016 23:02:03 +0300
>>
>> On Windows if one uses cygwin ispell, then one should fix ispell LIBDIR
>> (which is not correct absolute path) relative to cygwin root by setting
>> `ispell-library-directory'. Or one also might wish to use custom
>> location of dictionaries.
>
> ispell-library-directory is not supposed to be customized by users,
> it's an internal variable set and used by ispell.el.  To change the
> location of dictionaries, users are supposed to customize
> ispell-local-dictionary-alist.  Why can't you do that instead?

I do understand, that it's internal variable, though it it useful (and i 
would say crucial) for customization. How am i suppose to customize
`ispell-local-dictionary-alist'? AFAICS dictionaries, that are added 
there, are not visible to `ispell-change-dictionary', because 
`ispell-library-directory' points to a wrong directory, where 
`ispell-valid-dictionary-list' can't find them, so they are ommited.

> That's not what I see.  What I see is that ispell-library-directory is
> set once whenever the ispell process is created, which happens only
> once in a session, or when you change your ispell dictionary.  So I

Correct: once per session. But it is redundant either, because it is 
already set before in `ispell-set-spellchecker-params', which is called
by ispell-(region word change-dictionary).

> think this is completely justified, because the user could install
> additional dictionaries before restarting ispell.

IMHO, resetting `ispell-library-directory', once it is already set, 
without changing ispell-program-name is simply redundant. It is clear 
from logic in `ispell-set-spellchecker-params'

     (unless (eq ispell-last-program-name ispell-program-name) ...)

How (re)setting `ispell-library-directory' helps user installing new 
dictionaries? It's `ispell-valid-dictionary-list' responsibility to do 
actual work, besides `ispell-library-directory' is already set (before 
launching ispell process) and is not supposed to be changed if user 
doesn't change 'ispell-program-name'.

> If the suggestions above don't solve your problems, perhaps you could
> write a batch file or a script which reported the library directory in
> the format that ispell.el expects in a native Windows build of Emacs.
> Or maybe you could switch to Hunspell or Aspell, which don't need
> ispell-library-directory at all.  A native Windows port of Hunspell is
> available (and works very well for me).

Thanks, i'll consider that, but currently i'm happy with 
`ispell-library-directory customization' and elisp that fixes it.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> Cc: 24439@debbugs.gnu.org
> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Date: Thu, 15 Sep 2016 18:56:38 +0300
> 
> > ispell-library-directory is not supposed to be customized by users,
> > it's an internal variable set and used by ispell.el.  To change the
> > location of dictionaries, users are supposed to customize
> > ispell-local-dictionary-alist.  Why can't you do that instead?
> 
> I do understand, that it's internal variable, though it it useful (and i 
> would say crucial) for customization. How am i suppose to customize
> `ispell-local-dictionary-alist'?

It's a defcustom, so using Customize is the preferred way.  Of course,
you can also set it from Lisp.

What I meant specifically is to use the -d switch to tell ispell where
to look for the dictionaries.  The -d switch accepts absolute file
names, AFAIR, and will not try to look in ispell-library-directory for
such absolute file names.

If that doesn't help, then maybe I don't understand what problem,
exactly, are you trying to solve.  What bad things happen if you don't
change the value of ispell-library-directory from its default?

> AFAICS dictionaries, that are added 
> there, are not visible to `ispell-change-dictionary', because 
> `ispell-library-directory' points to a wrong directory

Even if the dictionaries that are arguments of the -d switch are given
as absolute file names?  If so, maybe that's the problem that needs
fixing.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Eli Zaretskii wrote:

 > What I meant specifically is to use the -d switch to tell ispell where
 > to look for the dictionaries.  The -d switch accepts absolute file
 > names, AFAIR, and will not try to look in ispell-library-directory for
 > such absolute file names.

-d doesn't work, because of this code:

;; Include if the dictionary is in the library, or dir not defined.
(if (and
name
;; For Aspell, we already know which dictionaries exist.
(or ispell-really-aspell
;; Include all dictionaries if lib directory not known.
;; Same for Hunspell, where ispell-library-directory is nil.
(not ispell-library-directory)
(file-exists-p (concat ispell-library-directory
"/" dict-bname ".hash"))
(file-exists-p (concat ispell-library-directory
"/" dict-bname ".has"))))
(push name dict-list)))


 > If that doesn't help, then maybe I don't understand what problem,
 > exactly, are you trying to solve.  What bad things happen if you don't
 > change the value of ispell-library-directory from its default?

Then it's not possible to add custom dictionaries (when using cygwin 
ispell) or dictionaries from other directory, than 
`ispell-library-directory', because they are ommited by 
`ispell-valid-dictionary-alist'.

 > Even if the dictionaries that are arguments of the -d switch are given
 > as absolute file names?  If so, maybe that's the problem that needs
 > fixing.

Yes.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> Cc: 24439@debbugs.gnu.org
> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Date: Thu, 15 Sep 2016 19:51:37 +0300
> 
> Eli Zaretskii wrote:
> 
>  > What I meant specifically is to use the -d switch to tell ispell where
>  > to look for the dictionaries.  The -d switch accepts absolute file
>  > names, AFAIR, and will not try to look in ispell-library-directory for
>  > such absolute file names.
> 
> -d doesn't work, because of this code:
> 
> ;; Include if the dictionary is in the library, or dir not defined.
> (if (and
> name
> ;; For Aspell, we already know which dictionaries exist.
> (or ispell-really-aspell
> ;; Include all dictionaries if lib directory not known.
> ;; Same for Hunspell, where ispell-library-directory is nil.
> (not ispell-library-directory)
> (file-exists-p (concat ispell-library-directory
> "/" dict-bname ".hash"))
> (file-exists-p (concat ispell-library-directory
> "/" dict-bname ".has"))))
> (push name dict-list)))
> 
> 
>  > If that doesn't help, then maybe I don't understand what problem,
>  > exactly, are you trying to solve.  What bad things happen if you don't
>  > change the value of ispell-library-directory from its default?
> 
> Then it's not possible to add custom dictionaries (when using cygwin 
> ispell) or dictionaries from other directory, than 
> `ispell-library-directory', because they are ommited by 
> `ispell-valid-dictionary-alist'.
> 
>  > Even if the dictionaries that are arguments of the -d switch are given
>  > as absolute file names?  If so, maybe that's the problem that needs
>  > fixing.
> 
> Yes.

OK, so adding a condition for dict-bname being an absolute file name
in the above snippet would solve your problem?

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Eli Zaretskii wrote:

> OK, so adding a condition for dict-bname being an absolute file name
> in the above snippet would solve your problem?

Well, the problem is not only mine and it would solve it only partially 
(in case of cygwin ispell), because dictionaries would have to be added 
to `ispell-[local-]dictionary-alist' with abspath (a little bit 
cumbersome; especially in case of roaming .emacs) even if they are in 
ispell LIBDIR (`ispell-library-directory').
Anyway, that fix would be wellcome, though i would prefer to have a 
possibility to customize directory where dictionaries are looked up.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Cc: 24439@debbugs.gnu.org
> Date: Thu, 15 Sep 2016 20:53:57 +0300
> 
> Eli Zaretskii wrote:
> 
> > OK, so adding a condition for dict-bname being an absolute file name
> > in the above snippet would solve your problem?
> 
> Well, the problem is not only mine and it would solve it only partially 
> (in case of cygwin ispell), because dictionaries would have to be added 
> to `ispell-[local-]dictionary-alist' with abspath (a little bit 
> cumbersome; especially in case of roaming .emacs) even if they are in 
> ispell LIBDIR (`ispell-library-directory').

Well, using a Cygwin ispell with a native Windows build of Emacs is
not exactly the normal use case, either.  Why don't you use a native
Windows build of ispell (or a Cygwin build of Emacs)?

> Anyway, that fix would be wellcome

Can you propose a patch and test it in your setup?  Or would you like
me to propose a patch for you to test?

> though i would prefer to have a possibility to customize directory
> where dictionaries are looked up.

ispell.el used to request a lot of such manual configury, but nowadays
it attempts to glean such information from the spellers themselves, as
far as possible.  It would have worked for you out of the box, were it
not for the fact that Emacs doesn't understand Cygwin file names (if I
understand correctly the nature of your problem).  So going back to
manual configuration would be a step in the wrong direction, IMO.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Eli Zaretskii wrote:

> Well, using a Cygwin ispell with a native Windows build of Emacs is
> not exactly the normal use case, either.  Why don't you use a native
> Windows build of ispell (or a Cygwin build of Emacs)?

I can't find one (ispell win32) and i guess it is not trivial to build.
And now i'd like to ask you why i don't see ispell in ezwinports...? :-)
Anyway, i use cygwin (a lot of it), but prefer native Emacs. Hunspell 
works with myspell dictionaries, right? (then maybe it is the best 
solution to my cygwin ispell problem).

> Can you propose a patch and test it in your setup?  Or would you like
> me to propose a patch for you to test?

Ok, i'll try. But i'm going to propose two patches; one as you proposed 
and another, where dictionaries would be looked up in this order:
  * -d if it's absolute path;
  * ispell-library-directory;
  * custom directory (say defcustom ispell-local-library-directory).


> not for the fact that Emacs doesn't understand Cygwin file names (if I
> understand correctly the nature of your problem).  So going back to

cygwin ispell reports its LIBDIR as '/usr/local/lib'. Even some old 
win32 build(s), i once had, had LIBDIR pointing to '/usr/lib' and 
dictionaries should have been be placed in C:\usr\lib... :-)


> manual configuration would be a step in the wrong direction, IMO.

IMO its the same kind of manual configuration as to manually specify 
dictionaries with -d (absolute path). `ispell-local-library-directory' 
would be transparent to user (won't require any 
configuration/consideration if he doesn't use it).

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Ken Brown]

On 9/15/2016 4:10 PM, Laimonas Vėbra wrote:
> cygwin ispell reports its LIBDIR as '/usr/local/lib'. Even some old
> win32 build(s), i once had, had LIBDIR pointing to '/usr/lib' and
> dictionaries should have been be placed in C:\usr\lib... :-)

You must be using an old version of Cygwin ispell.  In current Cygwin, 
ispell is a wrapper around aspell, and the dictionaries are in 
/usr/lib/aspell-0.60.

Ken

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Ken Brown wrote:

> You must be using an old version of Cygwin ispell.  In current Cygwin,
> ispell is a wrapper around aspell, and the dictionaries are in
> /usr/lib/aspell-0.60.

Actually it's a cygwin build of ispell, because cygwin does not provide 
ispell package. Anyway, wrapper (shell script) obviously doesn't work 
from native command line, so native Emacs can't use it.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Ken Brown]

On 9/15/2016 5:20 PM, Laimonas Vėbra wrote:
> Ken Brown wrote:
>
>> You must be using an old version of Cygwin ispell.  In current Cygwin,
>> ispell is a wrapper around aspell, and the dictionaries are in
>> /usr/lib/aspell-0.60.
>
> Actually it's a cygwin build of ispell, because cygwin does not provide
> ispell package.

Then whoever built it is responsible for the setting of LIBDIR.  I just 
looked at the ispell sources, and the default value of LIBDIR is 
/usr/local/lib on all platforms.  But the README explains how to change it.

Ken

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Ken Brown wrote:

> Then whoever built it is responsible for the setting of LIBDIR.  I just
> looked at the ispell sources, and the default value of LIBDIR is
> /usr/local/lib on all platforms.  But the README explains how to change it.

It is going offtopic...
LIBDIR can't be set (without patching source code) to native Windows 
path because then it won't find dictionaries either (even from cygwin). 
If it is set to cygwin-unix path, then it finds dictionaries in LIBDIR 
even when called from windows shell, but it doesn't work with Emacs, 
BECAUSE currently ispell.el (ispell-valid-dictionary-list) ommits all 
dictionaries that doesn't exist in LIBDIR
(it's obvious why (file-exists-p (concat (LIBDIR dictionary)) fails)
If LIBDIR was set to native windows path, then dictionaries wouldn't be 
ommited by `ispell-valid-dictionary-list', but cygwin ispell would not 
work either, because it could not find dictionaries in LIBDIR... :-)

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> Cc: 24439@debbugs.gnu.org
> From: Ken Brown <kbrown@cornell.edu>
> Date: Thu, 15 Sep 2016 18:17:43 -0400
> 
> > Actually it's a cygwin build of ispell, because cygwin does not provide
> > ispell package.
> 
> Then whoever built it is responsible for the setting of LIBDIR.  I just 
> looked at the ispell sources, and the default value of LIBDIR is 
> /usr/local/lib on all platforms.  But the README explains how to change it.

That is true, but OP's problem is that /usr/local/lib is not a native
Windows file name, so a native Windows build of Emacs doesn't
understand it, and will convert it to X:/usr/local/lib, for some value
of X.

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> Cc: 24439@debbugs.gnu.org
> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Date: Thu, 15 Sep 2016 23:10:49 +0300
> 
> Eli Zaretskii wrote:
> 
> > Well, using a Cygwin ispell with a native Windows build of Emacs is
> > not exactly the normal use case, either.  Why don't you use a native
> > Windows build of ispell (or a Cygwin build of Emacs)?
> 
> I can't find one (ispell win32) and i guess it is not trivial to build.

I was building it and using it for years, before I switched to
Hunspell.  It isn't hard, certainly not harder than any other package
with Posix configury.

> And now i'd like to ask you why i don't see ispell in ezwinports...? :-)

Because I switched to Hunspell before I started the site.  I never saw
anyone who'd request a ported ispell, but I still have the zip files
and can upload one if there's a demand.

> Hunspell works with myspell dictionaries, right?

Yes.  And as most Posix distributions switched to Aspell and Hunspell,
it is much easier to find libraries for Hunspell than for Ispell.
Some languages cannot even have an Ispell dictionary in principle,
because they use linguistic features Ispell doesn't support.

> > Can you propose a patch and test it in your setup?  Or would you like
> > me to propose a patch for you to test?
> 
> Ok, i'll try. But i'm going to propose two patches; one as you proposed 
> and another, where dictionaries would be looked up in this order:
>   * -d if it's absolute path;
>   * ispell-library-directory;
>   * custom directory (say defcustom ispell-local-library-directory).

Fine with me, thanks.

> > manual configuration would be a step in the wrong direction, IMO.
> 
> IMO its the same kind of manual configuration as to manually specify 
> dictionaries with -d (absolute path).

Yes, but that one already exists (and probably always will be).

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

Me wrote:
 > LIBDIR can't be set (without patching source code) to native Windows
 > path because then it won't find dictionaries either (even from
 > cygwin).

Sorry, that is not true (mistakenly thought, that fopen won't work with 
windows paths). Then i should really rebuild ispell with LIBDIR set to 
native path and it will work with Emacs.

Though ispell.el problem (dictionaries with absolute path (-d) are 
omitted by `ispell-valid-dictionary-alist') still exists and needs to be 
fixed (i'll do my best).

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Date: Fri, 16 Sep 2016 04:35:14 +0300
> 
> Me wrote:
>  > LIBDIR can't be set (without patching source code) to native Windows
>  > path because then it won't find dictionaries either (even from
>  > cygwin).
> 
> Sorry, that is not true (mistakenly thought, that fopen won't work with 
> windows paths). Then i should really rebuild ispell with LIBDIR set to 
> native path and it will work with Emacs.

I think you don't even need that.  You need to set ISPELL_DICTDIR in
the environment to point to that directory.

(The reason I didn't suggest this earlier is that I didn't know Cygwin
can fopen Windows-style absolute file names.)

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

[-- Attachment #1: Type: text/plain, Size: 2728 bytes --]

Eli Zaretskii wrote:

>> Sorry, that is not true (mistakenly thought, that fopen won't work with
>> windows paths). Then i should really rebuild ispell with LIBDIR set to
>> native path and it will work with Emacs.
>
> I think you don't even need that.  You need to set ISPELL_DICTDIR in
> the environment to point to that directory.
>
> (The reason I didn't suggest this earlier is that I didn't know Cygwin
> can fopen Windows-style absolute file names.)

Actually that won't work with current ispell.el, because LIBRARYVAR is 
completely ignored and dictionaries searched only in LIBDIR (though it 
can be overridden by LIBRARYVAR).

Fixed this and other (-d with an absolute path) issues in attached patch.

Below is a explanatory test:

(require 'ispell)
ispell

;; cygwin ispell reports its LIBDIR as
(ispell-check-version)
"/usr/local/lib"

;; and it's not a correct/existing windows path
(file-exists-p "/usr/local/lib")
nil

(shell-command-to-string "cygpath -w /usr/local/lib")
"C:\\cygwin64\\usr\\local\\lib
"

;; so ispell.el still can not see valid/installed dictionaries
(shell-command-to-string "ls -l /usr/local/lib")
"total 1126
lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 american.hash -> 
americanmed.hash
-rw-r--r-- 1 Admin None 1125408 Jul 29 01:38 americanmed.hash
-rw-r--r-- 1 Admin None   24095 Jul 29 01:38 english.aff
lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 english.hash -> 
americanmed.hash
"
(ispell-valid-dictionary-list)
("default")


;; Though now we can add dictionaries explicitly (-d with an absolute path)
ispell-local-dictionary-alist
nil

(add-to-list 'ispell-local-dictionary-alist
	     '("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil
	       ("-d" "c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))
(("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))


(add-to-list 'ispell-local-dictionary-alist
	     '("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil
	       ("-d" "c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1))
(("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1) ("american" 
"[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))

(ispell-valid-dictionary-list)
("american" "english" "default")

;; or we can set ispell LIBRARYVAR (default: ISPELL_DICTDIR) environment 
variable
(setq ispell-local-dictionary-alist nil)
nil
(ispell-valid-dictionary-list)
("default")

(setenv "ISPELL_DICTDIR" "c:/cygwin64/usr/local/lib")
"c:/cygwin64/usr/local/lib"

(setq ispell-library-directory (ispell-check-version))
"c:/cygwin64/usr/local/lib"

(ispell-valid-dictionary-list)
("english" "american" "default")


[-- Attachment #2: ispell.patch --]
[-- Type: text/plain, Size: 2996 bytes --]

--- lisp/textmodes/ispell.el.orig	2015-04-02 10:23:06.000000000 +0300
+++ lisp/textmodes/ispell.el	2016-09-24 17:58:21.637841100 +0300
@@ -848,7 +848,12 @@
   (let ((default-directory (or (and (boundp 'temporary-file-directory)
 				    temporary-file-directory)
 			       default-directory))
-	result status ispell-program-version)
+	(get-config-var
+	 (lambda (var)
+	   (when (re-search-forward
+		  (concat var " = \\\"\\(.+?\\)\\\"") nil t)
+	     (match-string 1))))
+	result libvar status ispell-program-version)
 
     (with-temp-buffer
       (setq status (ispell-call-process
@@ -870,9 +875,13 @@
 				 ", "
 				 ispell-version))
 	    (message "%s" result))
-	;; return library directory.
-	(if (re-search-forward "LIBDIR = \\\"\\([^ \t\n]*\\)\\\"" nil t)
-	    (setq result (match-string 1))))
+	;; return LIBDIR or LIBRARYVAR (overrides LIBDIR) env.
+	(progn
+	  (setq result (funcall get-config-var "LIBDIR")
+		libvar (funcall get-config-var "LIBRARYVAR"))
+	  (when libvar
+	    (setq libvar (getenv libvar))
+	    (unless (member libvar '(nil "")) (setq result libvar)))))
       (goto-char (point-min))
       (if (not (memq status '(0 nil)))
 	  (error "%s exited with %s %s" ispell-program-name
@@ -1463,23 +1472,29 @@
 
   (let ((dicts (append ispell-local-dictionary-alist ispell-dictionary-alist))
 	(dict-list (cons "default" nil))
-	name dict-bname)
+	(dict-locate
+	 (lambda (dict &optional dir)
+	   (locate-file (file-name-nondirectory dict)
+			`(,(or dir (file-name-directory dict)))
+			(unless (file-name-extension dict) '(".hash" ".has")))))
+	name dict-explt dict-bname)
     (dolist (dict dicts)
       (setq name (car dict)
-	    dict-bname (or (car (cdr (member "-d" (nth 5 dict))))
-			   name))
-      ;; Include if the dictionary is in the library, or dir not defined.
-      (if (and
-	   name
-	   ;; For Aspell, we already know which dictionaries exist.
-	   (or ispell-really-aspell
-	       ;; Include all dictionaries if lib directory not known.
-	       ;; Same for Hunspell, where ispell-library-directory is nil.
-	       (not ispell-library-directory)
-	       (file-exists-p (concat ispell-library-directory
-				      "/" dict-bname ".hash"))
-	       (file-exists-p (concat ispell-library-directory
-				      "/" dict-bname ".has"))))
+	    ;; Explicitly (via ispell-args) specified dictionary.
+	    dict-explt (car (cdr (member "-d" (nth 5 dict))))
+	    dict-bname (or dict-explt name))
+      (if (and name
+	       (or
+		;; Include all for Aspell (we already know existing dicts)
+		ispell-really-aspell
+		;; Include all if `ispell-library-directory' is nil (Hunspell)
+		(not ispell-library-directory)
+		;; If explicit (-d with an absolute path) and existing dict.
+		(and dict-explt
+		     (file-name-absolute-p dict-explt)
+		     (funcall dict-locate dict-explt))
+		;; If dict located in `ispell-library-directory'.
+		(funcall dict-locate dict-bname ispell-library-directory)))
 	  (push name dict-list)))
     dict-list))
 

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Laimonas Vėbra]

[-- Attachment #1: Type: text/plain, Size: 2729 bytes --]

Eli Zaretskii wrote:

>> Sorry, that is not true (mistakenly thought, that fopen won't work with
>> windows paths). Then i should really rebuild ispell with LIBDIR set to
>> native path and it will work with Emacs.
>
> I think you don't even need that.  You need to set ISPELL_DICTDIR in
> the environment to point to that directory.
>
> (The reason I didn't suggest this earlier is that I didn't know Cygwin
> can fopen Windows-style absolute file names.)

Actually that won't work with current ispell.el, because LIBRARYVAR is 
completely ignored and dictionaries searched only in LIBDIR (though it 
can be overridden by LIBRARYVAR).

Fixed this and other (-d with an absolute path) issues in attached patch.

Below is a explanatory test:

(require 'ispell)
ispell

;; cygwin ispell reports its LIBDIR as
(ispell-check-version)
"/usr/local/lib"

;; and it's not a correct/existing windows path
(file-exists-p "/usr/local/lib")
nil

(shell-command-to-string "cygpath -w /usr/local/lib")
"C:\\cygwin64\\usr\\local\\lib
"

;; so ispell.el still can not see valid/installed dictionaries
(shell-command-to-string "ls -l /usr/local/lib")
"total 1126
lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 american.hash -> 
americanmed.hash
-rw-r--r-- 1 Admin None 1125408 Jul 29 01:38 americanmed.hash
-rw-r--r-- 1 Admin None   24095 Jul 29 01:38 english.aff
lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 english.hash -> 
americanmed.hash
"
(ispell-valid-dictionary-list)
("default")


;; Though now we can add dictionaries explicitly (-d with an absolute path)
ispell-local-dictionary-alist
nil

(add-to-list 'ispell-local-dictionary-alist
	     '("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil
	       ("-d" "c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))
(("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))


(add-to-list 'ispell-local-dictionary-alist
	     '("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil
	       ("-d" "c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1))
(("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1) ("american" 
"[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
"c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))

(ispell-valid-dictionary-list)
("american" "english" "default")

;; or we can set ispell LIBRARYVAR (default: ISPELL_DICTDIR) environment 
variable
(setq ispell-local-dictionary-alist nil)
nil
(ispell-valid-dictionary-list)
("default")

(setenv "ISPELL_DICTDIR" "c:/cygwin64/usr/local/lib")
"c:/cygwin64/usr/local/lib"

(setq ispell-library-directory (ispell-check-version))
"c:/cygwin64/usr/local/lib"

(ispell-valid-dictionary-list)
("english" "american" "default")



[-- Attachment #2: ispell.patch --]
[-- Type: text/plain, Size: 2996 bytes --]

--- lisp/textmodes/ispell.el.orig	2015-04-02 10:23:06.000000000 +0300
+++ lisp/textmodes/ispell.el	2016-09-24 17:58:21.637841100 +0300
@@ -848,7 +848,12 @@
   (let ((default-directory (or (and (boundp 'temporary-file-directory)
 				    temporary-file-directory)
 			       default-directory))
-	result status ispell-program-version)
+	(get-config-var
+	 (lambda (var)
+	   (when (re-search-forward
+		  (concat var " = \\\"\\(.+?\\)\\\"") nil t)
+	     (match-string 1))))
+	result libvar status ispell-program-version)
 
     (with-temp-buffer
       (setq status (ispell-call-process
@@ -870,9 +875,13 @@
 				 ", "
 				 ispell-version))
 	    (message "%s" result))
-	;; return library directory.
-	(if (re-search-forward "LIBDIR = \\\"\\([^ \t\n]*\\)\\\"" nil t)
-	    (setq result (match-string 1))))
+	;; return LIBDIR or LIBRARYVAR (overrides LIBDIR) env.
+	(progn
+	  (setq result (funcall get-config-var "LIBDIR")
+		libvar (funcall get-config-var "LIBRARYVAR"))
+	  (when libvar
+	    (setq libvar (getenv libvar))
+	    (unless (member libvar '(nil "")) (setq result libvar)))))
       (goto-char (point-min))
       (if (not (memq status '(0 nil)))
 	  (error "%s exited with %s %s" ispell-program-name
@@ -1463,23 +1472,29 @@
 
   (let ((dicts (append ispell-local-dictionary-alist ispell-dictionary-alist))
 	(dict-list (cons "default" nil))
-	name dict-bname)
+	(dict-locate
+	 (lambda (dict &optional dir)
+	   (locate-file (file-name-nondirectory dict)
+			`(,(or dir (file-name-directory dict)))
+			(unless (file-name-extension dict) '(".hash" ".has")))))
+	name dict-explt dict-bname)
     (dolist (dict dicts)
       (setq name (car dict)
-	    dict-bname (or (car (cdr (member "-d" (nth 5 dict))))
-			   name))
-      ;; Include if the dictionary is in the library, or dir not defined.
-      (if (and
-	   name
-	   ;; For Aspell, we already know which dictionaries exist.
-	   (or ispell-really-aspell
-	       ;; Include all dictionaries if lib directory not known.
-	       ;; Same for Hunspell, where ispell-library-directory is nil.
-	       (not ispell-library-directory)
-	       (file-exists-p (concat ispell-library-directory
-				      "/" dict-bname ".hash"))
-	       (file-exists-p (concat ispell-library-directory
-				      "/" dict-bname ".has"))))
+	    ;; Explicitly (via ispell-args) specified dictionary.
+	    dict-explt (car (cdr (member "-d" (nth 5 dict))))
+	    dict-bname (or dict-explt name))
+      (if (and name
+	       (or
+		;; Include all for Aspell (we already know existing dicts)
+		ispell-really-aspell
+		;; Include all if `ispell-library-directory' is nil (Hunspell)
+		(not ispell-library-directory)
+		;; If explicit (-d with an absolute path) and existing dict.
+		(and dict-explt
+		     (file-name-absolute-p dict-explt)
+		     (funcall dict-locate dict-explt))
+		;; If dict located in `ispell-library-directory'.
+		(funcall dict-locate dict-bname ispell-library-directory)))
 	  (push name dict-list)))
     dict-list))
 

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

Agustin, could you please take a look at the proposed patch?  TIA.

> From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> Cc: 24439@debbugs.gnu.org
> Date: Thu, 6 Oct 2016 13:33:08 +0300
> 
> Eli Zaretskii wrote:
> 
> >> Sorry, that is not true (mistakenly thought, that fopen won't work with
> >> windows paths). Then i should really rebuild ispell with LIBDIR set to
> >> native path and it will work with Emacs.
> >
> > I think you don't even need that.  You need to set ISPELL_DICTDIR in
> > the environment to point to that directory.
> >
> > (The reason I didn't suggest this earlier is that I didn't know Cygwin
> > can fopen Windows-style absolute file names.)
> 
> Actually that won't work with current ispell.el, because LIBRARYVAR is 
> completely ignored and dictionaries searched only in LIBDIR (though it 
> can be overridden by LIBRARYVAR).
> 
> Fixed this and other (-d with an absolute path) issues in attached patch.
> 
> Below is a explanatory test:
> 
> (require 'ispell)
> ispell
> 
> ;; cygwin ispell reports its LIBDIR as
> (ispell-check-version)
> "/usr/local/lib"
> 
> ;; and it's not a correct/existing windows path
> (file-exists-p "/usr/local/lib")
> nil
> 
> (shell-command-to-string "cygpath -w /usr/local/lib")
> "C:\\cygwin64\\usr\\local\\lib
> "
> 
> ;; so ispell.el still can not see valid/installed dictionaries
> (shell-command-to-string "ls -l /usr/local/lib")
> "total 1126
> lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 american.hash -> 
> americanmed.hash
> -rw-r--r-- 1 Admin None 1125408 Jul 29 01:38 americanmed.hash
> -rw-r--r-- 1 Admin None   24095 Jul 29 01:38 english.aff
> lrwxrwxrwx 1 Admin None      16 Jul 29 01:38 english.hash -> 
> americanmed.hash
> "
> (ispell-valid-dictionary-list)
> ("default")
> 
> 
> ;; Though now we can add dictionaries explicitly (-d with an absolute path)
> ispell-local-dictionary-alist
> nil
> 
> (add-to-list 'ispell-local-dictionary-alist
> 	     '("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil
> 	       ("-d" "c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))
> (("american" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
> "c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))
> 
> 
> (add-to-list 'ispell-local-dictionary-alist
> 	     '("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil
> 	       ("-d" "c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1))
> (("english" "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
> "c:/cygwin64/usr/local/lib/english.hash") nil iso-8859-1) ("american" 
> "[A-Za-z]" "[^A-Za-z]" "[']" nil ("-d" 
> "c:/cygwin64/usr/local/lib/american.hash") nil iso-8859-1))
> 
> (ispell-valid-dictionary-list)
> ("american" "english" "default")
> 
> ;; or we can set ispell LIBRARYVAR (default: ISPELL_DICTDIR) environment 
> variable
> (setq ispell-local-dictionary-alist nil)
> nil
> (ispell-valid-dictionary-list)
> ("default")
> 
> (setenv "ISPELL_DICTDIR" "c:/cygwin64/usr/local/lib")
> "c:/cygwin64/usr/local/lib"
> 
> (setq ispell-library-directory (ispell-check-version))
> "c:/cygwin64/usr/local/lib"
> 
> (ispell-valid-dictionary-list)
> ("english" "american" "default")

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Agustin Martin]

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

2016-10-07 10:58 GMT+02:00 Eli Zaretskii <eliz@gnu.org>:

> Agustin, could you please take a look at the proposed patch?  TIA.
>
> > From: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> > Cc: 24439@debbugs.gnu.org
> > Date: Thu, 6 Oct 2016 13:33:08 +0300
> >
> > Eli Zaretskii wrote:
> >
> > >> Sorry, that is not true (mistakenly thought, that fopen won't work
> with
> > >> windows paths). Then i should really rebuild ispell with LIBDIR set to
> > >> native path and it will work with Emacs.
> > >
> > > I think you don't even need that.  You need to set ISPELL_DICTDIR in
> > > the environment to point to that directory.
> > >
> > > (The reason I didn't suggest this earlier is that I didn't know Cygwin
> > > can fopen Windows-style absolute file names.)
> >
> > Actually that won't work with current ispell.el, because LIBRARYVAR is
> > completely ignored and dictionaries searched only in LIBDIR (though it
> > can be overridden by LIBRARYVAR).
> >
> > Fixed this and other (-d with an absolute path) issues in attached patch.
>

Hi, Eli

I had a look at your patch and it looks really fine, thanks for caring
about this. Needed to first read the whole thread (gmail decided to tag
part of it as spam) and refresh my lisp a bit, I am currently busy with
other things and am less responsive.

I have also run some tests with it, and seems to work very well regarding
dictionary selection. Did not check actual spellchecking (Checked with an
old 24.4.1 where at least a function is not available, but could at least
check that lists of available dictionaries)

Best regards,

-- 
Agustin

[-- Attachment #2: Type: text/html, Size: 2693 bytes --]

bug#24439: ispell.el; cutom setting `ispell-library-directory'; patch

[Eli Zaretskii]

> From: Agustin Martin <agustin6martin@gmail.com>
> Date: Sat, 8 Oct 2016 13:30:50 +0200
> Cc: Laimonas Vėbra <laimonas.vebra@yahoo.com>
> 
> Hi, Eli
> 
> I had a look at your patch and it looks really fine, thanks for caring about this.

(It's not my patch, it was written by Laimonas.)

> I have also run some tests with it, and seems to work very well regarding dictionary selection. Did not check
> actual spellchecking (Checked with an old 24.4.1 where at least a function is not available, but could at least
> check that lists of available dictionaries)

Thanks for the review.  I pushed this to the master branch (in
Laimonas's name), and I'm marking this bug done.