bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[dinkonin]

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

Hello,
I'm having some trouble with flyspell using the enchant backend
(setq ispell-program-name "enchant-2").

I have installed the latest enchant 2.2.12, without the
optional finish and hebrew backends. Flyspell refuses to work with the
above setting (no errors just does not detect anything ). Ive
narrowed it down to this:

when flyspell loads it adds the following line n top of
"ispell-enchant-dictionary-alist"

((nil "[[:alpha:]]" "[^[:alpha:]]" "[\n** (process:98935): WARNING **:
22:05:56.283: Error loading plugin: libvoikko.so.1: cannot open shared
object file: No such file or directory\n\n\n** (process:98935): WARNING
**: 22:05:56.286: Error loading plugin: libhspell.so.0: cannot open
shared object file: No such file or directory\n\n0123456789]" t nil nil
utf-8)

The problem disappears when I manually do:
(setq ispell-enchant-dictionary-alist
      '(("bg_BG" "[[:alpha:]]" "[^[:alpha:]]" "" t nil nil utf-8)
        ("en_US" "[[:alpha:]]" "[^[:alpha:]]" "" t nil nil utf-8)))

I've noticed that enchant-lsmod-2 also produces this error in the
terminal, maybe STDERR should be omitted when generating
ispell-enchant-dictionary-alist or shoud this be reported upstream to
enchant ?


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23,
cairo version 1.17.3)
 of 2020-10-21 built on srv
Repository revision: 3be93390fb6680d1e0c3256af72c86635a9eb327
Repository branch: makepkg
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man --with-gameuser=:games
 --with-sound=alsa --with-modules --without-gconf --without-gsettings
 --with-nativecomp --with-x-toolkit=gtk3 --without-xaw3d
 --without-m17n-flt --with-cairo --without-compress-install
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -g
 -fuse-ld=gold -g -fuse-ld=gold' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GLIB NOTIFY INOTIFY ACL
GNUTLS LIBXML2 FREETYPE HARFBUZZ LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3
X11 XDBE XIM MODULES NATIVE_COMP THREADS LIBSYSTEMD JSON PDUMPER LCMS2

Important settings:
  value of $LC_COLLATE: bg_BG.UTF-8
  value of $LC_MONETARY: bg_BG.UTF-8
  value of $LC_NUMERIC: bg_BG.UTF-8
  value of $LC_TIME: bg_BG.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Org

Minor modes in effect:
  auto-dim-other-buffers-mode: t
  persistent-scratch-autosave-mode: t
  reverse-im-mode: t
  auto-insert-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  flycheck-inline-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  buffer-face-mode: t
  org-indent-mode: t
  company-box-mode: t
  amx-mode: t
  diredfl-global-mode: t
  treemacs-icons-dired-mode: t
  treemacs-tag-follow-mode: t
  treemacs-filewatch-mode: t
  treemacs-git-mode: deferred
  treemacs-fringe-indicator-mode: t
  gcmh-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  global-auto-revert-mode: t
  global-evil-collection-unimpaired-mode: t
  evil-collection-unimpaired-mode: t
  electric-pair-mode: t
  show-paren-mode: t
  savehist-mode: t
  global-company-mode: t
  company-mode: t
  evil-org-mode: t
  save-place-mode: t
  which-key-mode: t
  evil-goggles-mode: t
  evil-lion-mode: t
  global-evil-surround-mode: t
  evil-surround-mode: t
  shell-dirtrack-mode: t
  evil-mode: t
  evil-local-mode: t
  ivy-posframe-mode: t
  override-global-mode: t
  all-the-icons-ivy-rich-mode: t
  ivy-rich-mode: t
  ivy-mode: t
  minions-mode: t
  delete-selection-mode: t
  general-override-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  straight-live-modifications-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: dinkonin <dinkonin@gmail.com>
> Date: Thu, 29 Oct 2020 22:25:47 +0200
> 
> I'm having some trouble with flyspell using the enchant backend
> (setq ispell-program-name "enchant-2").
> 
> I have installed the latest enchant 2.2.12, without the
> optional finish and hebrew backends. Flyspell refuses to work with the
> above setting (no errors just does not detect anything ). Ive
> narrowed it down to this:
> 
> when flyspell loads it adds the following line n top of "ispell-enchant-dictionary-alist"
> 
> ((nil "[[:alpha:]]" "[^[:alpha:]]" "[\n** (process:98935): WARNING **:
> 22:05:56.283: Error loading plugin: libvoikko.so.1: cannot open shared
> object file: No such file or directory\n\n\n** (process:98935): WARNING
> **: 22:05:56.286: Error loading plugin: libhspell.so.0: cannot open
> shared object file: No such file or directory\n\n0123456789]" t nil nil
> utf-8)

These look like local configuration problems on your system, or some
bug in Enchant: it tells you that some shared libraries are missing.

> I've noticed that enchant-lsmod-2 also produces this error in the
> terminal, maybe STDERR should be omitted when generating
> ispell-enchant-dictionary-alist or shoud this be reported upstream to
> enchant ?

The latter, I think.  Reuben, can you please comment?

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Fri, 30 Oct 2020 at 07:38, Eli Zaretskii <eliz@gnu.org> wrote:

>
> > I've noticed that enchant-lsmod-2 also produces this error in the
> > terminal, maybe STDERR should be omitted when generating
> > ispell-enchant-dictionary-alist or shoud this be reported upstream to
> > enchant ?
>
> The latter, I think.  Reuben, can you please comment?
>

It looks like Enchant has been built with the voikko (Finnish) provider,
but libvoikko is not installed.

I don't think it's possible to build Enchant's voikko provider without
libvoikko installed, because the provider needs to link against libvoikko.
So, this looks like a configuration problem on the user's system. To
convince me otherwise would require a recipe to reproduce the problem, I
think!

If there is a bug, it would definitely be in Enchant, and should be
reported to Enchant's bug tracker (details in the package). Thanks!

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Fri, 30 Oct 2020 22:56:27 +0000
> Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> 
> It looks like Enchant has been built with the voikko (Finnish) provider, but libvoikko is not installed.
> 
> I don't think it's possible to build Enchant's voikko provider without libvoikko installed, because the provider
> needs to link against libvoikko. So, this looks like a configuration problem on the user's system. To convince
> me otherwise would require a recipe to reproduce the problem, I think!
> 
> If there is a bug, it would definitely be in Enchant, and should be reported to Enchant's bug tracker (details in
> the package). Thanks!

Thanks, I'm therefore closing this bug report.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[dinkonin]

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

Thanks for looking into this ,

I am able to reproduce it on a new ArchLinux installation with `emacs -Q
--eval '(setq ispell-program-name "enchant-2")'` both with Emacs 27 from
arch repositories and Emacs 28 built from source.
So I think the problem is in the Arch Linux version of the enchant package,
It's built with support for all backends but they are not Installed by the
package manager because they are marked as optional by the package
maintainer.

I still think this should not break spell checking in the languages
supported by the installed enchant providers.

On Sat, Oct 31, 2020 at 12:56 AM Reuben Thomas <rrt@sc3d.org> wrote:

> On Fri, 30 Oct 2020 at 07:38, Eli Zaretskii <eliz@gnu.org> wrote:
>
>>
>> > I've noticed that enchant-lsmod-2 also produces this error in the
>> > terminal, maybe STDERR should be omitted when generating
>> > ispell-enchant-dictionary-alist or shoud this be reported upstream to
>> > enchant ?
>>
>> The latter, I think.  Reuben, can you please comment?
>>
>
> It looks like Enchant has been built with the voikko (Finnish) provider,
> but libvoikko is not installed.
>
> I don't think it's possible to build Enchant's voikko provider without
> libvoikko installed, because the provider needs to link against libvoikko.
> So, this looks like a configuration problem on the user's system. To
> convince me otherwise would require a recipe to reproduce the problem, I
> think!
>
> If there is a bug, it would definitely be in Enchant, and should be
> reported to Enchant's bug tracker (details in the package). Thanks!
>
> --
> https://rrt.sc3d.org
>

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: dinkonin <dinkonin@gmail.com>
> Date: Sat, 31 Oct 2020 09:32:19 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, 44318@debbugs.gnu.org
> 
> I still think this should not break spell checking in the languages supported by the installed enchant
> providers.

That might be so, but that's not an Emacs problem, is it?  If the
speller refuses to run correctly due to misconfigured installation,
how can you expect Emacs to fix that problem?  Your wish should be
directed at the maintainers of the Enchant package on that system.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[dinkonin]

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

I totally agree with you that this is an upstream problem and I have
reported it there. But maybe I have not worded my bug correctly (my English
is lacking, sorry). Enchant is working everywhere else on the same system
i.e. in AbiWord, Vim, GtkSpell and gedit. It does not work only in Emacs,
that's why I reported it here, for the benefit of other Arch users
using the package and Emacs.

Again I'm sorry for wasting your time.

On Sat, Oct 31, 2020 at 9:50 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: dinkonin <dinkonin@gmail.com>
> > Date: Sat, 31 Oct 2020 09:32:19 +0200
> > Cc: Eli Zaretskii <eliz@gnu.org>, 44318@debbugs.gnu.org
> >
> > I still think this should not break spell checking in the languages
> supported by the installed enchant
> > providers.
>
> That might be so, but that's not an Emacs problem, is it?  If the
> speller refuses to run correctly due to misconfigured installation,
> how can you expect Emacs to fix that problem?  Your wish should be
> directed at the maintainers of the Enchant package on that system.
>

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: dinkonin <dinkonin@gmail.com>
> Date: Sat, 31 Oct 2020 10:37:49 +0200
> Cc: Reuben Thomas <rrt@sc3d.org>, 44318@debbugs.gnu.org
> 
> I totally agree with you that this is an upstream problem and I have reported it there. But maybe I have not
> worded my bug correctly (my English is lacking, sorry). Enchant is working everywhere else on the same
> system i.e. in AbiWord, Vim, GtkSpell and gedit. It does not work only in Emacs, that's why I reported it here,
> for the benefit of other Arch users using the package and Emacs.

I don't know how it succeeds working in those other environments, but
I don't think Emacs should work around clear problems in installing
the spell-checker.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Sat, 31 Oct 2020 at 09:17, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: dinkonin <dinkonin@gmail.com>
> > Date: Sat, 31 Oct 2020 10:37:49 +0200
> > Cc: Reuben Thomas <rrt@sc3d.org>, 44318@debbugs.gnu.org
> >
> > I totally agree with you that this is an upstream problem and I have
> reported it there. But maybe I have not
> > worded my bug correctly (my English is lacking, sorry). Enchant is
> working everywhere else on the same
> > system i.e. in AbiWord, Vim, GtkSpell and gedit. It does not work only
> in Emacs, that's why I reported it here,
> > for the benefit of other Arch users using the package and Emacs.
>
> I don't know how it succeeds working in those other environments, but
> I don't think Emacs should work around clear problems in installing
> the spell-checker.
>

I've had a bit more time to think about this now, and I think I understand
better what is going on.

First, it's pretty obvious why this isn't a problem for programs other than
Emacs: they ignore the error messages.

I think Enchant is correct to generate the errors, as it shows that it has
been mis-installed. I looked at the Arch package, and it depends on
libvoikko, so I'm surprised you're getting these errors in the first place.

However, I think Emacs should be able (like other Enchant-using programs)
to cope with this problem, particularly as all it should have to do is
ignore stderr.

I think this can be achieved by changing the definition of
ispell--call-enchant-lsmod to:

(defun ispell--call-enchant-lsmod (&rest args)
  "Call enchant-lsmod with ARGS and return the output as string."
  (with-output-to-string
    (apply #'ispell-call-process
           (replace-regexp-in-string "enchant\\(-[0-9]\\)?\\'"
                                     "enchant-lsmod\\1"
                                     ispell-program-name)
           nil '(t nil) nil args)))

(I can't see any reason to use `with-current-buffer` either; am I missing
something?)

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Sun, 1 Nov 2020 22:23:44 +0000
> Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> 
> I think Enchant is correct to generate the errors, as it shows that it has been mis-installed. I looked at the
> Arch package, and it depends on libvoikko, so I'm surprised you're getting these errors in the first place.
> 
> However, I think Emacs should be able (like other Enchant-using programs) to cope with this problem,
> particularly as all it should have to do is ignore stderr.
> 
> I think this can be achieved by changing the definition of ispell--call-enchant-lsmod to:

Thanks, but I'm not interested in working around installation problems
of programs we invoke.  It's a slippery slope, with never-ending
additional requests we will have to honor.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[dinkonin]

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

I agree with Eli Zaretskii,
 after some further investigation I've narrowed it down to an issue with
the arch packaging that does not exist in other non arch-based
distributions. I reported the issue in https://bugs.archlinux.org/task/68499,
and I hope this will be resolved by the package maintainer.
Again sorry for wasting your time and thank you both for the swift
responses.

On Mon, Nov 2, 2020 at 5:34 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Sun, 1 Nov 2020 22:23:44 +0000
> > Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> >
> > I think Enchant is correct to generate the errors, as it shows that it
> has been mis-installed. I looked at the
> > Arch package, and it depends on libvoikko, so I'm surprised you're
> getting these errors in the first place.
> >
> > However, I think Emacs should be able (like other Enchant-using
> programs) to cope with this problem,
> > particularly as all it should have to do is ignore stderr.
> >
> > I think this can be achieved by changing the definition of
> ispell--call-enchant-lsmod to:
>
> Thanks, but I'm not interested in working around installation problems
> of programs we invoke.  It's a slippery slope, with never-ending
> additional requests we will have to honor.
>

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Mon, 2 Nov 2020 at 03:34, Eli Zaretskii <eliz@gnu.org> wrote:

>
> Thanks, but I'm not interested in working around installation problems
> of programs we invoke.  It's a slippery slope, with never-ending
> additional requests we will have to honor.
>

The reporter and I seem to have swapped positions on this!

I believe that the ispell.el code (which I wrote) is buggy: it should not
be incorporating warnings into its output.

Also, the patch I offered is a simplification of the original code. So, I
don't think we are losing here.

Eli, I quite agree with your sentiment, and I would certainly not advocate
installing a workaround in Emacs unless there were compelling reasons.
However, I do not see this as a workaround, and as it is also a
simplification, I don't see a problem.

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[dinkonin]

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

Thanks you Reuben Thomas, I hope this gets resolved in the best way
possible,

Unfortunately Arch maintainers have declined looking at this with the
following Catch 22 comment:

>> FS#68499 - [enchant] Warnings about optional dependencies.>> Reason for
closing: Not a bug
>> Additional comments about closing: Warnings are not errors. If emacs
Flyspell doesn't work upon seeing enchant *warnings*, Flyspell should be
fixed

As for me, I think I will refrain from reporting more bugs for now.
Be safe.

On Mon, Nov 2, 2020 at 10:35 AM Reuben Thomas <rrt@sc3d.org> wrote:

> On Mon, 2 Nov 2020 at 03:34, Eli Zaretskii <eliz@gnu.org> wrote:
>
>>
>> Thanks, but I'm not interested in working around installation problems
>> of programs we invoke.  It's a slippery slope, with never-ending
>> additional requests we will have to honor.
>>
>
> The reporter and I seem to have swapped positions on this!
>
> I believe that the ispell.el code (which I wrote) is buggy: it should not
> be incorporating warnings into its output.
>
> Also, the patch I offered is a simplification of the original code. So, I
> don't think we are losing here.
>
> Eli, I quite agree with your sentiment, and I would certainly not advocate
> installing a workaround in Emacs unless there were compelling reasons.
> However, I do not see this as a workaround, and as it is also a
> simplification, I don't see a problem.
>
> --
> https://rrt.sc3d.org
>

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: dinkonin <dinkonin@gmail.com>
> Date: Mon, 2 Nov 2020 10:14:41 +0200
> Cc: Reuben Thomas <rrt@sc3d.org>, 44318@debbugs.gnu.org
> 
> Again sorry for wasting your time and thank you both for the swift responses.

No need to apologize.  Thank you for your interest in Emacs.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 2 Nov 2020 08:35:13 +0000
> Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> 
> I believe that the ispell.el code (which I wrote) is buggy: it should not be incorporating warnings into its
> output.
> 
> Also, the patch I offered is a simplification of the original code. So, I don't think we are losing here.
> 
> Eli, I quite agree with your sentiment, and I would certainly not advocate installing a workaround in Emacs
> unless there were compelling reasons. However, I do not see this as a workaround, and as it is also a
> simplification, I don't see a problem.

I have a problem with ignoring stderr because it can provide some
useful information.  Moreover, no one said that some client of
Enchant, current or future, won't decide to produce non-error output
on stderr.

Sorry.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Mon, 2 Nov 2020 at 15:43, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Mon, 2 Nov 2020 08:35:13 +0000
> > Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> >
> > I believe that the ispell.el code (which I wrote) is buggy: it should
> not be incorporating warnings into its
> > output.
> >
> > Also, the patch I offered is a simplification of the original code. So,
> I don't think we are losing here.
> >
> > Eli, I quite agree with your sentiment, and I would certainly not
> advocate installing a workaround in Emacs
> > unless there were compelling reasons. However, I do not see this as a
> workaround, and as it is also a
> > simplification, I don't see a problem.
>
> I have a problem with ignoring stderr because it can provide some
> useful information.


It is not so much a question of ignoring stderr (for example, this output
could be shown to the Emacs user), it is a question of not treating it as
part of the output of enchant-lsmod.


> Moreover, no one said that some client of Enchant, current or future,
> won't decide to produce non-error output on stderr.
>

I'm not sure what you mean here. "enchant-lsmod" simply queries its own
providers (spelling back-ends). I maintain all the code involved (there are
currently no third-party providers for Enchant that I'm aware of), and, for
what it's worth, decide how it should work.

In this case, the providers should not be outputting anything, either to
stderr or stdout, and indeed, they do not. It is enchant-lsmod that emits
the warning when it cannot load a provider. This is a legitimate warning,
but it does not form part of its output.

So I, as upstream maintainer, am telling you that indeed, it is not correct
that Emacs should take enchant-lsmod's stderr as part of its output! (As I
said above, it would be possible to capture stderr separately and display
it to the user, though I don't recommend that currently, because the
warnings are not written for users. Other programs that use Enchant do not
show these warnings, they simply omit providers that do not load from the
list of languages/spelling checkers that are available.)

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 2 Nov 2020 15:49:19 +0000
> Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> 
> So I, as upstream maintainer, am telling you that indeed, it is not correct that Emacs should take
> enchant-lsmod's stderr as part of its output! (As I said above, it would be possible to capture stderr
> separately and display it to the user, though I don't recommend that currently, because the warnings are not
> written for users. Other programs that use Enchant do not show these warnings, they simply omit providers
> that do not load from the list of languages/spelling checkers that are available.)

OK, I agree to this change, under protest.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1.1: Type: text/plain, Size: 2123 bytes --]

On Mon, 2 Nov 2020 at 16:10, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Mon, 2 Nov 2020 15:49:19 +0000
> > Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> >
> > So I, as upstream maintainer, am telling you that indeed, it is not
> correct that Emacs should take
> > enchant-lsmod's stderr as part of its output! (As I said above, it would
> be possible to capture stderr
> > separately and display it to the user, though I don't recommend that
> currently, because the warnings are not
> > written for users. Other programs that use Enchant do not show these
> warnings, they simply omit providers
> > that do not load from the list of languages/spelling checkers that are
> available.)
>
> OK, I agree to this change, under protest.
>

Thanks Eli, I have installed it; of course, contact me if it causes any
problems. After installing that patch, I discovered that in
ispell--call-enchant-lsmod, it did need to use with-current-buffer, so I
have installed a patch that restores the use of with-current-buffer;
apologies for getting that wrong first time.

In fact, this change is not sufficient to fix the original problem, because
the enchant program outputs the same warning when a provider module cannot
be loaded. In this case, stderr is added to the list of suggested words.
Again, I believe Enchant is correct to output a warning, and again, I
believe Emacs is wrong to amalgamate stderr and stdout.

I attach three patches that do the following:

1. Simplify ispell-call-process{,-region} by factoring out the macro
ispell-with-safe-default-directory.

2. I also found that ispell-check-version can be simplified slightly:
aspell has accepted -vv since 2004, so use it always.
3. When spell-checking, collect only standard output. This leaves some
spell-checker-specific calls to ispell-call-process to collect stderr as
well, which as far as I can tell is needed in only one case,
ispell-find-hunspell-dictionaries; but it doesn't hurt to leave the rest
unchanged. I have tested this patch with all supported spellcheckers
(ispell, aspell, hunspell, enchant).

[-- Attachment #1.2: Type: text/html, Size: 4051 bytes --]

[-- Attachment #2: 0002-Factor-out-some-common-code-in-ispell.el.patch --]
[-- Type: text/x-patch, Size: 1982 bytes --]

From 1aa6c9f5e7ce89f9440a2a5b4c591e65c8b4e5a0 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 2 Nov 2020 21:45:40 +0000
Subject: [PATCH 2/3] Factor out some common code in ispell.el

* lisp/textmodes/ispell.el (ispell-with-safe-default-directory): Add
  macro.
  (ispell-call-process, ispell-call-process-region): Use it.
---
 lisp/textmodes/ispell.el | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 9e56923d60..49da3d1ed4 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -769,18 +769,23 @@ ispell-check-version
 	    (setq ispell-really-hunspell nil))))))
     result))
 
+(defmacro ispell-with-safe-default-directory (&rest body)
+  "Execute the forms in BODY with a reasonable
+`default-directory'."
+  (declare (indent 0) (debug t))
+  `(let ((default-directory default-directory))
+     (unless (file-accessible-directory-p default-directory)
+       (setq default-directory (expand-file-name "~/")))
+     ,@body))
+
 (defun ispell-call-process (&rest args)
-  "Like `call-process' but defend against bad `default-directory'."
-  (let ((default-directory default-directory))
-    (unless (file-accessible-directory-p default-directory)
-      (setq default-directory (expand-file-name "~/")))
+  "Like `call-process', but defend against bad `default-directory'."
+  (ispell-with-safe-default-directory
     (apply 'call-process args)))
 
 (defun ispell-call-process-region (&rest args)
-  "Like `call-process-region' but defend against bad `default-directory'."
-  (let ((default-directory default-directory))
-    (unless (file-accessible-directory-p default-directory)
-      (setq default-directory (expand-file-name "~/")))
+  "Like `call-process-region', but defend against bad `default-directory'."
+  (ispell-with-safe-default-directory
     (apply 'call-process-region args)))
 
 (defvar ispell-debug-buffer)
-- 
2.25.1


[-- Attachment #3: 0001-Simplify-ispell-check-version-s-use-of-vv-flag.patch --]
[-- Type: text/x-patch, Size: 1333 bytes --]

From c83ffcbe1ba950b9270bcde56d117c2c50e35112 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 2 Nov 2020 21:41:05 +0000
Subject: [PATCH 1/3] =?UTF-8?q?Simplify=20ispell-check-version=E2=80=99s?=
 =?UTF-8?q?=20use=20of=20-vv=20flag?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/textmodes/ispell.el (ispell-check-version): All supported spell
checker programs accept -vv, including aspell for many years, so use
it.
---
 lisp/textmodes/ispell.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index da3c3f4d6f..9e56923d60 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -684,13 +684,11 @@ ispell-check-version
     (with-temp-buffer
       (setq status (ispell-call-process
 		    ispell-program-name nil t nil
-		    ;; aspell doesn't accept the -vv switch.
 		    (let ((case-fold-search
 			   (memq system-type '(ms-dos windows-nt)))
 			  (speller
 			   (file-name-nondirectory ispell-program-name)))
-		      ;; Assume anything that isn't `aspell' is Ispell.
-		      (if (string-match "\\`aspell" speller) "-v" "-vv"))))
+		      "-vv")))
       (goto-char (point-min))
       (if interactivep
 	  ;; Report version information of ispell
-- 
2.25.1


[-- Attachment #4: 0003-Prevent-ispell-being-confused-by-spellchecker-output.patch --]
[-- Type: text/x-patch, Size: 1471 bytes --]

From 5d772177536dcfed607159802eeadcd22eab1969 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 2 Nov 2020 21:45:52 +0000
Subject: [PATCH 3/3] Prevent `ispell' being confused by spellchecker output on
 stderr

* lisp/textmodes/ispell.el (ispell-send-string, ispell-lookup-words):
Capture only stdout. This avoids warning messages, e.g. from Enchant,
from being interpreted as output from the spell-checker process.
---
 lisp/textmodes/ispell.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index 49da3d1ed4..8d90cab653 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -1861,7 +1861,7 @@ ispell-send-string
 		    (apply 'ispell-call-process-region
 			   (point-min) (point-max)
 			   ispell-program-name nil
-			   output-buf nil
+			   '(output-buf nil) nil
 			   "-a"
 			   ;; hunspell -m option means something different
 			   (if ispell-really-hunspell "" "-m")
@@ -2566,7 +2566,7 @@ ispell-lookup-words
         (while (search-backward "*" nil t) (insert "."))
         (setq word (buffer-string))
         (erase-buffer))
-      (setq status (apply 'ispell-call-process prog nil t nil
+      (setq status (apply 'ispell-call-process prog nil '(t nil) nil
                           (nconc (if (and args (> (length args) 0))
                                      (list args)
                                    (if look-p nil
-- 
2.25.1

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 2 Nov 2020 21:49:33 +0000
> Cc: dinkonin <dinkonin@gmail.com>, 44318@debbugs.gnu.org
> 
> 1. Simplify ispell-call-process{,-region} by factoring out the macro ispell-with-safe-default-directory.
> 
> 2. I also found that ispell-check-version can be simplified slightly: aspell has accepted -vv since 2004, so use
> it always.
> 3. When spell-checking, collect only standard output. This leaves some spell-checker-specific calls to
> ispell-call-process to collect stderr as well, which as far as I can tell is needed in only one case,
> ispell-find-hunspell-dictionaries; but it doesn't hurt to leave the rest unchanged. I have tested this patch with
> all supported spellcheckers (ispell, aspell, hunspell, enchant).

I'm okay with the first 2, but I'm less comfortable with the 3rd one.
It is wrong to assume that nothing but warnings come through stderr:
for example "hunspell -D" emits the important information to stderr,
at least on my system.  It could be that we don't currently use the 2
functions you suggest to change for such cases, but I think ignoring
stderr in some calls and not the others is a slippery slope of
confusion and subtle bugs.  Since the important case -- that of
enchanty-lsmod -- is already solved, why do we need to make changes
that are not really required, and currently don't give us any gains?

Thanks.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

[Apologies Eli, re-sending to list rather than just you.]

On Tue, 3 Nov 2020 at 16:45, Eli Zaretskii <eliz@gnu.org> wrote:

>
> I'm okay with the first 2,


Great, I'll install those!

but I'm less comfortable with the 3rd one.
> It is wrong to assume that nothing but warnings come through stderr:
> for example "hunspell -D" emits the important information to stderr,
> at least on my system.


Exactly, I found this while testing a more ambitious patch that never
collected stderr (and indeed versions of Enchant until last night's release
of 2.2.13 printed their --version output on stderr).

Hence, all of those uses still collect stderr.

It could be that we don't currently use the 2
> functions you suggest to change for such cases, but I think ignoring
> stderr in some calls and not the others is a slippery slope of
> confusion and subtle bugs.


The two functions I changed implement a long-running communication with the
spellchecker using the ispell protocol, notionally over a pipe. There's no
reason to mix two streams, and in any case that would only work
fortuitously, since the spellchecker doesn't know how those streams will be
combined. In other words, a source of confusion and subtle bugs.

Fortunately, none of the current spellcheckers we support tries to do this.


>   Since the important case -- that of
> enchanty-lsmod -- is already solved, why do we need to make changes
> that are not really required, and currently don't give us any gains?
>

Unfortunately, as I mentioned before, it's not completely solved, as the
"enchant" program outputs warnings on stderr, just like enchant-lsmod. This
is interpreted by Emacs as additions to the suggestions or corrections
list, which is clearly wrong.

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Tue, 3 Nov 2020 at 17:04, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 3 Nov 2020 16:55:13 +0000
> >
> >    Since the important case -- that of
> >  enchanty-lsmod -- is already solved, why do we need to make changes
> >  that are not really required, and currently don't give us any gains?
> >
> > Unfortunately, as I mentioned before, it's not completely solved, as the
> "enchant" program outputs warnings
> > on stderr, just like enchant-lsmod. This is interpreted by Emacs as
> additions to the suggestions or
> > corrections list, which is clearly wrong.
>
> Then I suggest that you fix that upstream.  It is not nice for Enchant
> to output stuff to stderr while communicating with a front-end via a
> pipe, under the -a switch.  It's a violation of the protocol of sorts:
> anything you want to say in that mode should be said via stdout.
>

I'm not sure that's right: the warning is emitted at start-up, before
enchant starts executing the protocol. In any case, as I said before, I
don't think it makes sense for a client of a pipe protocol like this to
combine two streams (which cannot safely make sense).

I admit that Emacs is the only client I know of that uses Enchant (or any
of the spellchecker programs we support other than ispell); much of its
command-line interface is designed specifically to support Emacs. On the
other hand, I can't see a solution to this problem that doesn't involve
simply suppressing the warning message altogether, where the user will
never see it, including in manual testing. I guess it would be possible
only to emit warnings when stderr is a tty, for example, but that seems
like a hack; the solution I proposed is at worst a slight tightening of the
protocol that is already in agreement with all known implementations, of
which ispell is obsolete, aspell is obsolescent, and hunspell is mature:
it's unlikely any of them will act differently in future.

A longer-term solution would be to drop support for spellcheckers other
than Enchant. Enchant supports aspell and hunspell, as well as other
spell-checkers not otherwise available to Emacs, including newer, more
capable spellcheckers such as nuspell. This would eventually permit the
removal of a great deal of code and complexity from ispell.el.

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 3 Nov 2020 17:19:56 +0000
> Cc: 44318@debbugs.gnu.org, dinkonin <dinkonin@gmail.com>
> 
> I'm not sure that's right: the warning is emitted at start-up, before enchant starts executing the protocol. In
> any case, as I said before, I don't think it makes sense for a client of a pipe protocol like this to combine two
> streams (which cannot safely make sense).

It worked until now.  Enchant is the new kid on the block, so I
respectfully request that it behaves itself.  Yes, it probably means
it should suppress the warning when invoked with -a, but I see no
problem with that.  (You could even consider suppressing the warning
entirely, and only emitting it when some feature which requires the
problematic trait is requested.  But I won't tell you how to develop
Enchant's UI.)

> A longer-term solution would be to drop support for spellcheckers other than Enchant.

I think it would be wrong for Emacs to do that, as that would put all
the eggs in a single basket, something that is not safe in Free
Software world, where packages become unmaintained outside of our
control.  Not having to deal with idiosyncratic behavior of several
spellers is an advantage, but I think the risk of being left without
an actively maintained spell-checking engine is so serious that it
tramps that advantage many times over.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Tue, 3 Nov 2020 at 17:32, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 3 Nov 2020 17:19:56 +0000
> > Cc: 44318@debbugs.gnu.org, dinkonin <dinkonin@gmail.com>
> >
> > I'm not sure that's right: the warning is emitted at start-up, before
> enchant starts executing the protocol. In
> > any case, as I said before, I don't think it makes sense for a client of
> a pipe protocol like this to combine two
> > streams (which cannot safely make sense).
>
> It worked until now.  Enchant is the new kid on the block, so I
> respectfully request that it behaves itself.  Yes, it probably means
> it should suppress the warning when invoked with -a, but I see no
> problem with that.  (You could even consider suppressing the warning
> entirely, and only emitting it when some feature which requires the
> problematic trait is requested.  But I won't tell you how to develop
> Enchant's UI.)


I'll leave it for now—it doesn't seem to have been a serious problem, and
the warnings are only emitted for an incorrectly-installed Enchant, as we
observed. I am thinking about a major overhaul/rewrite of Enchant (internal
changes only!), so that may be an issue to address then.

I think it would be wrong for Emacs to do that, as that would put all
> the eggs in a single basket, something that is not safe in Free
> Software world, where packages become unmaintained outside of our
> control.


I don't understand this: Emacs, to this day, is happy to import large
quantities of code from other project; let alone the option of
forking/maintaining free software, which is one of its great benefits. And
because Enchant has such a similar interface to the other supported
spell-checkers, the cost of switching is low. For myself, I'd be more
concerned with bugs or missing functionality in Enchant as a reason to be
cautious (i.e. I would want to see a phased transition) than about the
long-term prospects.

In any case, the principal reason I'm thinking about rewriting Enchant is
to make it easier to maintain, so hopefully I'm moving towards making it
more palatable in your terms too.

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 3 Nov 2020 18:27:32 +0000
> Cc: 44318@debbugs.gnu.org, dinkonin <dinkonin@gmail.com>
> 
>  I think it would be wrong for Emacs to do that, as that would put all
>  the eggs in a single basket, something that is not safe in Free
>  Software world, where packages become unmaintained outside of our
>  control.
> 
> I don't understand this: Emacs, to this day, is happy to import large quantities of code from other project; let
> alone the option of forking/maintaining free software, which is one of its great benefits. And because
> Enchant has such a similar interface to the other supported spell-checkers, the cost of switching is low. For
> myself, I'd be more concerned with bugs or missing functionality in Enchant as a reason to be cautious (i.e.
> I would want to see a phased transition) than about the long-term prospects.

We are miscommunicating.  My point is that if Emacs will depend on
Enchant and won't be able to use the existing spellers without Enchant
being in-between, then we will be in a dire situation if Enchant stops
being developed and bit-rots.  By contrast, with the current code, we
can always tell users to use aspell/hunspell directly.

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Tue, 3 Nov 2020 at 18:50, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 3 Nov 2020 18:27:32 +0000
> > Cc: 44318@debbugs.gnu.org, dinkonin <dinkonin@gmail.com>
> >
> >  I think it would be wrong for Emacs to do that, as that would put all
> >  the eggs in a single basket, something that is not safe in Free
> >  Software world, where packages become unmaintained outside of our
> >  control.
> >
> > I don't understand this: Emacs, to this day, is happy to import large
> quantities of code from other project; let
> > alone the option of forking/maintaining free software, which is one of
> its great benefits. And because
> > Enchant has such a similar interface to the other supported
> spell-checkers, the cost of switching is low. For
> > myself, I'd be more concerned with bugs or missing functionality in
> Enchant as a reason to be cautious (i.e.
> > I would want to see a phased transition) than about the long-term
> prospects.
>
> We are miscommunicating.  My point is that if Emacs will depend on
> Enchant and won't be able to use the existing spellers without Enchant
> being in-between, then we will be in a dire situation if Enchant stops
> being developed and bit-rots.  By contrast, with the current code, we
> can always tell users to use aspell/hunspell directly.
>

What I was trying to say is that it would be very easy to re-add support
for the other spell-checkers, since they and Enchant operate in the same
way, and they would not have changed in the mean time. There is little to
rot in Enchant, I intend to reduce that amount, and in the end it should be
less effort to maintain Enchant than to maintain multiple back-ends in
ispell.el.

-- 
https://rrt.sc3d.org

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

bug#44318: 28.0.50; Problem with ispell/flyspell and ""enchant"" backend

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 3 Nov 2020 18:53:54 +0000
> Cc: 44318@debbugs.gnu.org, dinkonin <dinkonin@gmail.com>
> 
>  We are miscommunicating.  My point is that if Emacs will depend on
>  Enchant and won't be able to use the existing spellers without Enchant
>  being in-between, then we will be in a dire situation if Enchant stops
>  being developed and bit-rots.  By contrast, with the current code, we
>  can always tell users to use aspell/hunspell directly.
> 
> What I was trying to say is that it would be very easy to re-add support for the other spell-checkers, since
> they and Enchant operate in the same way, and they would not have changed in the mean time.

If we switch to supporting only Enchant, the compatibility will soon
disappear.  And re-adding back deleted code is just waste of
resources.  So I still think this is not a good idea, sorry.

> in the end it should be less effort to maintain Enchant than to
> maintain multiple back-ends in ispell.el.

That's an advantage, indeed, but as I said before, the downsides
overwhelm it.