Re: [mange@freemail.hu: Re: flyspell bug]

Re: [mange@freemail.hu: Re: flyspell bug]

[Eli Zaretskii]

> To: emacs-devel@gnu.org
> From: Magnus Henoch <mange@freemail.hu>
> Date: Wed, 27 Apr 2005 16:04:45 +0200
> Subject: Re: flyspell bug
> 
> So it seems that the variable ispell-really-aspell is moderately
> useful, as there are three situations: ispell, aspell in ispell
> compatibility mode, and "real" aspell.
> 
> The following patch eliminates the middle case.  The documentation of
> aspell 0.50.5 claims that it supports the "list" command, but I
> haven't tested it.
> 
> - --- orig/lisp/textmodes/ispell.el
> +++ mod/lisp/textmodes/ispell.el
> @@ -301,7 +301,8 @@
>    :type 'integer
>    :group 'ispell)
>  
> - -(defcustom ispell-program-name "ispell"
> +(defcustom ispell-program-name (or (executable-find "aspell")
> +				   "ispell")
>    "Program invoked by \\[ispell-word] and \\[ispell-region] commands."
>    :type 'string
>    :group 'ispell)

Thanks, I installed a slightly different change (using locate-file
instead of executable-find).

Re: flyspell bug

[Juri Linkov]

>> To: emacs-devel@gnu.org
>> From: Magnus Henoch <mange@freemail.hu>
>> Date: Wed, 27 Apr 2005 16:04:45 +0200
>> Subject: Re: flyspell bug
>> 
>> So it seems that the variable ispell-really-aspell is moderately
>> useful, as there are three situations: ispell, aspell in ispell
>> compatibility mode, and "real" aspell.
>> 
>> The following patch eliminates the middle case.  The documentation of
>> aspell 0.50.5 claims that it supports the "list" command, but I
>> haven't tested it.
>> 
>> - --- orig/lisp/textmodes/ispell.el
>> +++ mod/lisp/textmodes/ispell.el
>> @@ -301,7 +301,8 @@
>>    :type 'integer
>>    :group 'ispell)
>>  
>> - -(defcustom ispell-program-name "ispell"
>> +(defcustom ispell-program-name (or (executable-find "aspell")
>> +				   "ispell")
>>    "Program invoked by \\[ispell-word] and \\[ispell-region] commands."
>>    :type 'string
>>    :group 'ispell)
>
> Thanks, I installed a slightly different change (using locate-file
> instead of executable-find).

Now with using a non-default dictionary ispell.el fails with the
error message telling that ispell can't find a dictionary in
/usr/lib/aspell-0.60/.  That's becuase ispell and aspell have
different sets of dictionaries with different names.

ispell.el shouldn't insist on using "aspell" instead of "ispell"
even when aspell executable happens to be in exec-path.
Perhaps the following patch will do what was intended to fix the
original problem:

Index: lisp/textmodes/ispell.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/textmodes/ispell.el,v
retrieving revision 1.162
diff -u -r1.162 ispell.el
--- lisp/textmodes/ispell.el	7 May 2005 16:04:39 -0000	1.162
+++ lisp/textmodes/ispell.el	10 May 2005 00:05:05 -0000
@@ -302,7 +302,8 @@
   :group 'ispell)
 
 (defcustom ispell-program-name
-  (or (locate-file "aspell" exec-path exec-suffixes 'file-executable-p)
+  (or (and ispell-really-aspell
+	   (locate-file "aspell" exec-path exec-suffixes 'file-executable-p))
       "ispell")
   "Program invoked by \\[ispell-word] and \\[ispell-region] commands."
   :type 'string
@@ -815,7 +816,11 @@
         (goto-char (point-min))
         (let (case-fold-search)
           (setq ispell-really-aspell
-                (and (search-forward "(but really Aspell " nil t) t))))
+                (and (search-forward "(but really Aspell " nil t) t))
+	  (if ispell-really-aspell
+	      (let ((exec (locate-file "aspell" exec-path exec-suffixes
+				       'file-executable-p)))
+		(if exec (setq ispell-program-name exec))))))
       (kill-buffer (current-buffer)))
     result))
 
-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Date: Tue, 10 May 2005 03:06:48 +0300
> Cc: public@heslin.eclipse.co.uk, mange@freemail.hu, S.J.Eglen@damtp.cam.ac.uk
> 
> > Thanks, I installed a slightly different change (using locate-file
> > instead of executable-find).
> 
> Now with using a non-default dictionary ispell.el fails with the
> error message telling that ispell can't find a dictionary in
> /usr/lib/aspell-0.60/.  That's becuase ispell and aspell have
> different sets of dictionaries with different names.

Sorry about that.  I don't have aspell installed so I couldn't test
that, and people who were participating in the thread suggested that
this was the right solution.

> ispell.el shouldn't insist on using "aspell" instead of "ispell"
> even when aspell executable happens to be in exec-path.

I asked Richard and he said that's what we want, since aspell is newer
and is a GNU package.

> Perhaps the following patch will do what was intended to fix the
> original problem:
> 
> Index: lisp/textmodes/ispell.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/textmodes/ispell.el,v
> retrieving revision 1.162
> diff -u -r1.162 ispell.el
> --- lisp/textmodes/ispell.el	7 May 2005 16:04:39 -0000	1.162
> +++ lisp/textmodes/ispell.el	10 May 2005 00:05:05 -0000
> @@ -302,7 +302,8 @@
>    :group 'ispell)
>  
>  (defcustom ispell-program-name
> -  (or (locate-file "aspell" exec-path exec-suffixes 'file-executable-p)
> +  (or (and ispell-really-aspell
> +	   (locate-file "aspell" exec-path exec-suffixes 'file-executable-p))
>        "ispell")

??? Does this really work?  AFAIK, ispell-really-aspell is set when
ispell.el invokes the speller for the first time, not before that.  So
this defcustom wuill never try aspell, I think.

Am I missing something?

Re: flyspell bug

[Juri Linkov]

>> Index: lisp/textmodes/ispell.el
>> ===================================================================
>> RCS file: /cvsroot/emacs/emacs/lisp/textmodes/ispell.el,v
>> retrieving revision 1.162
>> diff -u -r1.162 ispell.el
>> --- lisp/textmodes/ispell.el	7 May 2005 16:04:39 -0000	1.162
>> +++ lisp/textmodes/ispell.el	10 May 2005 00:05:05 -0000
>> @@ -302,7 +302,8 @@
>>    :group 'ispell)
>>  
>>  (defcustom ispell-program-name
>> -  (or (locate-file "aspell" exec-path exec-suffixes 'file-executable-p)
>> +  (or (and ispell-really-aspell
>> +	   (locate-file "aspell" exec-path exec-suffixes 'file-executable-p))
>>        "ispell")
>
> ??? Does this really work?  AFAIK, ispell-really-aspell is set when
> ispell.el invokes the speller for the first time, not before that.  So
> this defcustom wuill never try aspell, I think.

This may be needed when users set `ispell-really-aspell' explicitly
in .emacs.  However, after looking at ispell.el it seems that
`ispell-check-version' always is called before invoking the speller
for the first time.  So maybe the second part of my patch makes
the search for aspell in defcustom unnecessary, and its initial
value should be reverted to "ispell".

Note that automatic changing the value of `ispell-program-name'
in `ispell-check-version' might surprise users of the aspell's
implementation of ispell.  Anyhow, this is the better place to
change ispell program name to "aspell" than defcustom.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, public@heslin.eclipse.co.uk,
> 	mange@freemail.hu, s.j.eglen@damtp.cam.ac.uk
> From: Juri Linkov <juri@jurta.org>
> Date: Tue, 10 May 2005 08:04:07 +0300
> 
> Note that automatic changing the value of `ispell-program-name'
> in `ispell-check-version' might surprise users of the aspell's
> implementation of ispell.  Anyhow, this is the better place to
> change ispell program name to "aspell" than defcustom.

I think the right thing to do is to modify what's currently in CVS so
that it doesn't fail with non-English dictionaries.  Can you describe
how it fails in that case, and why?

Re: flyspell bug

[Juri Linkov]

>> Cc: emacs-devel@gnu.org, public@heslin.eclipse.co.uk,
>> 	mange@freemail.hu, s.j.eglen@damtp.cam.ac.uk
>> From: Juri Linkov <juri@jurta.org>
>> Date: Tue, 10 May 2005 08:04:07 +0300
>> 
>> Note that automatic changing the value of `ispell-program-name'
>> in `ispell-check-version' might surprise users of the aspell's
>> implementation of ispell.  Anyhow, this is the better place to
>> change ispell program name to "aspell" than defcustom.
>
> I think the right thing to do is to modify what's currently in CVS so
> that it doesn't fail with non-English dictionaries.  Can you describe
> how it fails in that case, and why?

ispell and aspell use different dictionaries.  Changing the default
ispell program name from "ispell" to "aspell" is an incompatible
change for users configured Emacs to use ispell dictionaries,
not aspell dictionaries.  `ispell-program-name' can be changed
automatically to "aspell" only when `ispell-check-version' detects
aspell's implemention of ispell, i.e. a program for which `ispell -v'
returns:

@(#) International Ispell Version 3.1.20 (but really Aspell 0.50.5)

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Date: Wed, 11 May 2005 11:23:48 +0300
> 
> ispell and aspell use different dictionaries.  Changing the default
> ispell program name from "ispell" to "aspell" is an incompatible
> change for users configured Emacs to use ispell dictionaries,
> not aspell dictionaries.

I understand that.  What I need is a way to distinguish between three
situations:

  (1) user doesn't have aspell installed
  (2) user has aspell installed, but configured it to use ispell's
      dictionaries
  (3) user has aspell installed and wants to use aspell's dictionaries

I know how to detect (1) -- use locate-file or similar technique to
look for aspell.  But how to distinguish between (2) and (3)?

> `ispell-program-name' can be changed
> automatically to "aspell" only when `ispell-check-version' detects
> aspell's implemention of ispell, i.e. a program for which `ispell -v'
> returns:
> 
> @(#) International Ispell Version 3.1.20 (but really Aspell 0.50.5)

So the conditions for (2) should be:

  (a) locate-file finds an executable `ispell'
  (b) that `ispell', when invoked with -v outputs the above banner

Is that right?  Are we sure that no user will use aspell with ispell's
dictionaries _except_ through the `ispell' alias?

Re: flyspell bug

[Juri Linkov]

>   (1) user doesn't have aspell installed
>   (2) user has aspell installed, but configured it to use ispell's
>       dictionaries
>   (3) user has aspell installed and wants to use aspell's dictionaries

There is also a fourth situation:

    (4) user has aspell installed, but uses ispell with ispell's dictionaries

> I know how to detect (1) -- use locate-file or similar technique to
> look for aspell.  But how to distinguish between (2) and (3)?

AFAIK, the situation (2) doesn't exist.  There are the following
situations instead:

    (2) user has aspell installed and uses the compatibility script
        `ispell' as a wrapper around aspell with aspell's dictionaries

    (3) user has aspell installed and uses `aspell' executable with
        aspell's dictionaries

(2) is detected by checking the output of `ispell -v'.
(3) is configured by users by changing ispell-program-name to "aspell".

After the change in flyspell.el it doesn't work with the `ispell'
compatibility script.  Maybe ispell-really-aspell should have more
values to distinguish between (2) and (3).  (3) can be detected
by matching ispell-program-name against the regexp "aspell$".

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Eli Zaretskii]

> Cc: s.j.eglen@damtp.cam.ac.uk, mange@freemail.hu,
> 	public@heslin.eclipse.co.uk, emacs-devel@gnu.org
> From: Juri Linkov <juri@jurta.org>
> Date: Thu, 12 May 2005 08:29:55 +0300
> 
> >   (1) user doesn't have aspell installed
> >   (2) user has aspell installed, but configured it to use ispell's
> >       dictionaries
> >   (3) user has aspell installed and wants to use aspell's dictionaries
> 
> There is also a fourth situation:
> 
>     (4) user has aspell installed, but uses ispell with ispell's dictionaries

That is the situation in which Richard wanted Emacs to prefer aspell.

Re: flyspell bug

[Juri Linkov]

To my surprise I just discovered that the latest development release of
ispell.el at http://kdstevens.com/~stevens/ispell-page.html already
supports the option to choose between ispell and aspell:

(defcustom ispell-prefer-aspell nil
  "*Select preference between using the `ispell' or `aspell' program.
`ispell' is used by default.
When this variable is set, `aspell' is used if it is installed on the system."
  :type 'boolean
  :group 'ispell)

(defcustom ispell-program-name
  (or (and (exec-installed-p "aspell")
	   (or ispell-prefer-aspell
	       (not (exec-installed-p "ispell")))
	   "aspell")
      "ispell")
  "Program invoked by \\[ispell-word] and \\[ispell-region] commands."
  :type 'string
  :group 'ispell)

This seems right to me.  Are there any reasons why the latest
development release of ispell.el is not synced with Emacs CVS?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Date: Thu, 12 May 2005 09:14:47 +0300
> Cc: public@heslin.eclipse.co.uk, mange@freemail.hu, s.j.eglen@damtp.cam.ac.uk,
> 	k.stevens@ieee.org
> 
> To my surprise I just discovered that the latest development release of
> ispell.el at http://kdstevens.com/~stevens/ispell-page.html already
> supports the option to choose between ispell and aspell:

This uses exec-installed-p, which we don't have.  Where is it from?

Re: flyspell bug

[Ken Stevens]

"Eli Zaretskii" writes: 

> This uses exec-installed-p, which we don't have.  Where is it from?

I have been the developer and maintainer of this for years - however, I
have become neglectful of late due to work, family, and other
volunteering loads.

I have a version that doesn't use exec-installed-p, but have not put it
out for testing.

regards		-Ken

Re: flyspell bug

[Richard Stallman]

    I have a version that doesn't use exec-installed-p, but have not put it
    out for testing.

Could you email it to us?  We can look at the differences from your
previous version, and also test it.

Re: flyspell bug

[Ken Stevens]

Richard Stallman writes: 

>     I have a version that doesn't use exec-installed-p, but have not
>     put it out for testing.
> 
> Could you email it to us?  We can look at the differences from your
> previous version, and also test it.

The current version I have does not try to enforce a particular speller
- both ispell and aspell are supported.  The choice is determined by the
setting of the customized variable `ispell-prefer-aspell' when both ispell
and aspell exist on the system.

I had used the function `exec-installed-p' which had it's roots in APEL
(A Portable Emacs Library).  In digging deeper, it appears that this is
not necessary as the standard emacs function `executable-find' appears
to have the same desired functionality.

Here are the variables:


(defcustom ispell-prefer-aspell nil
  "*Select preference between using the `ispell' or `aspell' program.
`ispell' is used by default.
When this variable is set, `aspell' is used if it is installed on the system."
  :type 'boolean
  :group 'ispell)


(defcustom ispell-program-name
  (or (and (executable-find "aspell")
	   (or ispell-prefer-aspell
	       (not (executable-find "ispell")))
	   "aspell")
      "ispell")
  "Program invoked by \\[ispell-word] and \\[ispell-region] commands."
  :type 'string
  :group 'ispell)


I'm sure you'll argue to make `ispell-prefer-aspell' set by default ;-)
It is only used in the above location in the code.


I have several other patches, improvements to integrate into the current
version of ispell as well.

regards	   -Ken

Re: flyspell bug

[Richard Stallman]

    The current version I have does not try to enforce a particular speller
    - both ispell and aspell are supported.  The choice is determined by the
    setting of the customized variable `ispell-prefer-aspell' when both ispell
    and aspell exist on the system.

That sounds good.

    I have several other patches, improvements to integrate into the current
    version of ispell as well.

How about sending us this version in some form or other?

Re: flyspell bug

[Richard Stallman]

    This seems right to me.  Are there any reasons why the latest
    development release of ispell.el is not synced with Emacs CVS?

In principle there is no reason--it would be good to install
the latest code.  Ken, could you do this soon?

Re: flyspell bug

[Richard Stallman]

    ispell and aspell use different dictionaries.  Changing the default
    ispell program name from "ispell" to "aspell" is an incompatible
    change for users configured Emacs to use ispell dictionaries,
    not aspell dictionaries.

Maybe aspell could support ispell dictionaries.  Or, aspell could see
that it has been given an ispell dictionary file for a certain
language, and automatically use the aspell dictionary for the same
language (or report an error if it doesn't have one available).

Kevin, what do you think?

Re: flyspell bug

[Kevin Atkinson]

On Thu, 12 May 2005, Richard Stallman wrote:

>     ispell and aspell use different dictionaries.  Changing the default
>     ispell program name from "ispell" to "aspell" is an incompatible
>     change for users configured Emacs to use ispell dictionaries,
>     not aspell dictionaries.
> 
> Maybe aspell could support ispell dictionaries.  

They are in a compiled binary format and doing such would be rather 
difficult.

> Or, aspell could see that it has been given an ispell dictionary file
> for a certain language, and automatically use the aspell dictionary for
> the same language (or report an error if it doesn't have one available).

If you use "-d DICT" where DICT does NOT end in ".hash" or is a complete 
path to an Ispell dictionary than it should work as you suggested.  This 
strategy should work for both Ispell and Aspell as Ispell.  If it did not 
find one it should print an error to standard out and return a non-zero 
exit code.

If a complete path is provided and the supplied dictionary is not an 
Aspell one it will also report an error to standard out and return a 
non-zero exit code.

-- 
http://kevin.atkinson.dhs.org

Re: flyspell bug

[Richard Stallman]

    > Or, aspell could see that it has been given an ispell dictionary file
    > for a certain language, and automatically use the aspell dictionary for
    > the same language (or report an error if it doesn't have one available).

    If you use "-d DICT" where DICT does NOT end in ".hash" or is a complete 
    path to an Ispell dictionary than it should work as you suggested.

If this already works, how come the problem happens?
Why doesn't this make everything work right already?

Re: flyspell bug

[Kevin Atkinson]

On Tue, 24 May 2005, Richard Stallman wrote:

>     > Or, aspell could see that it has been given an ispell dictionary file
>     > for a certain language, and automatically use the aspell dictionary for
>     > the same language (or report an error if it doesn't have one available).
> 
>     If you use "-d DICT" where DICT does NOT end in ".hash" or is a complete 
>     path to an Ispell dictionary than it should work as you suggested.
> 
> If this already works, how come the problem happens?
> Why doesn't this make everything work right already?

Are you asking me or some of the peole on the CC list?

-- 
http://kevin.atkinson.dhs.org

Re: flyspell bug

[Richard Stallman]

    > If this already works, how come the problem happens?
    > Why doesn't this make everything work right already?

    Are you asking me or some of the peole on the CC list?

Whoever knows the answer, I guess.

Re: flyspell bug

[Juri Linkov]

>     If you use "-d DICT" where DICT does NOT end in ".hash" or is a complete 
>     path to an Ispell dictionary than it should work as you suggested.
>
> If this already works, how come the problem happens?
> Why doesn't this make everything work right already?

Users may have different sets of dictionaries installed for Ispell
and Aspell.  After replacing the default program name "ispell" by
"aspell", ispell.el may fail when a configured Ispell's dictionary
name doesn't match any of installed Aspell dictionary names.

To avoid such failure, ispell.el could check the Aspell's exit code
and in case when Aspell can't find a dictionary name among its list
of available dictionaries, ispell.el could change the default program
name from "aspell" to "ispell" and try to run Ispell with the same
dictionary name.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Kevin Atkinson]

On Wed, 25 May 2005, Juri Linkov wrote:

> >     If you use "-d DICT" where DICT does NOT end in ".hash" or is a complete 
> >     path to an Ispell dictionary than it should work as you suggested.
> >
> > If this already works, how come the problem happens?
> > Why doesn't this make everything work right already?
> 
> Users may have different sets of dictionaries installed for Ispell
> and Aspell.  After replacing the default program name "ispell" by
> "aspell", ispell.el may fail when a configured Ispell's dictionary
> name doesn't match any of installed Aspell dictionary names.
> 
> To avoid such failure, ispell.el could check the Aspell's exit code
> and in case when Aspell can't find a dictionary name among its list
> of available dictionaries, ispell.el could change the default program
> name from "aspell" to "ispell" and try to run Ispell with the same
> dictionary name.

Most Aspell dictionaries are installed with names that the orignal Ispell 
dictionarys use.  Just make sure it isn't a complete path or the name 
doesn't end if ".hash".

-- 
http://kevin.atkinson.dhs.org

Re: flyspell bug

[Richard Stallman]

    To avoid such failure, ispell.el could check the Aspell's exit code
    and in case when Aspell can't find a dictionary name among its list
    of available dictionaries, ispell.el could change the default program
    name from "aspell" to "ispell" and try to run Ispell with the same
    dictionary name.

That is an ugly temporary workaround.  What I would rather do is give
a clear error message that points the user at the ways to reconfigure.

Re: flyspell bug

[Richard Stallman]

    ispell.el shouldn't insist on using "aspell" instead of "ispell"
    even when aspell executable happens to be in exec-path.

Of course it should.  aspell is a GNU package.

What led you to think the contrary?

Re: flyspell bug

[Juri Linkov]

>     ispell.el shouldn't insist on using "aspell" instead of "ispell"
>     even when aspell executable happens to be in exec-path.
>
> Of course it should.  aspell is a GNU package.
>
> What led you to think the contrary?

I had the impression that the intention was to fix a problem of using
aspell with ispell.el.  However, the installed change is too radical.
It affects all users of ispell having aspell in exec-path.

If switching from ispell to aspell was intentional (I can't find
a discussion about this on emacs-devel) then we need to document
it in the NEWS file with a warning about possible problems, to inform
maintainers of ispell.el and aspell about this change asking them to
update documentation, web pages, and to test aspell dictionaries
to confirm their compatibility with ispell.el.

But perhaps switching from ispell to aspell was too hasty change.
According to aspell's TODO list, ispell.el is not quite ready for
using aspell.  The Info node (info "(aspell)Important Items") has:

  Things that need to be done
  ---------------------------

     * Enhance *ispell.el* so that it will work better with GNU Aspell.
       _[In Progress]_.

I don't know what is the current status of this TODO item.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: flyspell bug

[Richard Stallman]

      However, the installed change is too radical.
    It affects all users of ispell having aspell in exec-path.

Preferring aspell to ispell will inevitably do that.
It is the price that must be paid for a change that we will make
sooner or later.

We are going to switch to preferring aspell sometime.  If aspell is
not ready, we can't do it.  If aspell is ready now, sooner is better
than later.

    If switching from ispell to aspell was intentional (I can't find
    a discussion about this on emacs-devel) then we need to document
    it in the NEWS file with a warning about possible problems, to inform
    maintainers of ispell.el and aspell about this change asking them to
    update documentation, web pages, and to test aspell dictionaries
    to confirm their compatibility with ispell.el.

We're not building airplanes, you know.  Ideally in the absence of
resource constraints we would do all these things, but we don't have
to do them all.  We should not evaluate development choices as if we
were going to do them with the level of care that would be required
for building airplanes.

    But perhaps switching from ispell to aspell was too hasty change.
    According to aspell's TODO list, ispell.el is not quite ready for
    using aspell.

I would not rely on aspell's TODO list for the very latest information
on the status of ispell.el development.  The facts we are observing
directly about ispell are much more authoritative than a report
that might be old.