unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#50370: Fix bug in ispell-init-process error handling
       [not found] <e086ce83-5262-4007-aff3-56fa84427459@Spark>
@ 2021-09-04  3:53 ` Ian W
  2021-09-04 12:00   ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Ian W @ 2021-09-04  3:53 UTC (permalink / raw)
  To: 50370


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

This fixes the error handling in ispell-init-process. I encountered this bug, and also saw it mentioned here: https://mail.gnu.org/archive/html/auctex/2021-08/msg00007.html

The previous behavior involved the ispell process terminating
prematurely (correct behavior, invalid input) and then calling
ispell-accept-output. Because ispell-process had terminated,
ispell-accept-output threw its own error, which prevented the underlying
error from being displayed.

The bug resulted in the following interaction:
ispell-word would print out:
"Starting new Ispell process aspell with default dictionary...done"
"ispell-accept-output: No Ispell process to read output from!"

The correct behavior is:
"Starting new Ispell process aspell with default dictionary...done"
"cond: Error: ~/.emacs.d/.local/etc/ispell/.pws: The language "nil" is not known. This is probably because: the file "/usr/local/Cellar/aspell/0.60.8/lib/aspell-0.60/nil.dat" can not be opened for reading.""


In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.5.0, NS appkit-2022.50 Version 11.4 (Build 20F71))
 of 2021-08-15 built on Ians-MacBook-Pro-2.local
Windowing system distributor 'Apple', version 10.3.2022
System Description: macOS 11.5.2

Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs-plus@28/28.0.50/share/info/emacs
 --prefix=/usr/local/Cellar/emacs-plus@28/28.0.50 --with-xml2
 --with-gnutls --with-native-compilation --without-dbus
 --with-imagemagick --with-modules --with-rsvg --with-xwidgets --with-ns
 --disable-ns-self-contained 'CFLAGS=-I/usr/local/opt/gcc/include
 -I/usr/local/opt/libgccjit/include -I/usr/local/opt/gmp/include
 -I/usr/local/opt/jpeg/include' 'LDFLAGS=-L/usr/local/lib/gcc/11
 -I/usr/local/opt/gcc/include -I/usr/local/opt/libgccjit/include
 -I/usr/local/opt/gmp/include -I/usr/local/opt/jpeg/include''

The patch is attached.

Ian

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

[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 977 bytes --]

diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index c2f6b35df8..ce0653c434 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -2923,7 +2923,13 @@ Keeps argument list for future Ispell invocations for no async support."
 	     ;; But first wait to see if some more output is going to arrive.
 	     ;; Otherwise we get cool errors like "Can't open ".
 	     (sleep-for 1)
-	     (ispell-accept-output 3)
+             ;; To avoid `ispell-accept-output' throwing it's own
+             ;; error, we check that it will receive from a valid
+             ;; process.
+	     (when (if ispell-async-processp
+                         (process-live-p ispell-process)
+                       ispell-process)
+               (ispell-accept-output 3))
 	     (error "%s" (mapconcat #'identity ispell-filter "\n"))))
       (setq ispell-filter nil)		; Discard version ID line
       (let ((extended-char-mode (ispell-get-extended-character-mode)))

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#50370: Fix bug in ispell-init-process error handling
  2021-09-04  3:53 ` bug#50370: Fix bug in ispell-init-process error handling Ian W
@ 2021-09-04 12:00   ` Eli Zaretskii
       [not found]     ` <dc51f1a1-953c-45e9-90ea-6ca8c0288ed7@Spark>
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2021-09-04 12:00 UTC (permalink / raw)
  To: Ian W; +Cc: 50370

> Date: Fri, 3 Sep 2021 20:53:24 -0700
> From: Ian W <ian@wahbe.com>
> 
> This fixes the error handling in ispell-init-process. I encountered this bug, and also saw it mentioned here:
> https://mail.gnu.org/archive/html/auctex/2021-08/msg00007.html
> 
> The previous behavior involved the ispell process terminating
> prematurely (correct behavior, invalid input) and then calling
> ispell-accept-output. Because ispell-process had terminated,
> ispell-accept-output threw its own error, which prevented the underlying
> error from being displayed.
> 
> The bug resulted in the following interaction:
> ispell-word would print out:
> "Starting new Ispell process aspell with default dictionary...done"
> "ispell-accept-output: No Ispell process to read output from!"
> 
> The correct behavior is:
> "Starting new Ispell process aspell with default dictionary...done"
> "cond: Error: ~/.emacs.d/.local/etc/ispell/.pws: The language "nil" is not known. This is probably because: the
> file "/usr/local/Cellar/aspell/0.60.8/lib/aspell-0.60/nil.dat" can not be opened for reading.""

I'm not sure I agree that the "correct behavior" better than the
"incorrect".  The error message is not more self-explanatory, at
least.

Anyway, do you have a recipe for reproducing this problem?  The URL of
the AUCTeX discussion seems to say it happens every time?  I don't
think I understand what's going wrong to trigger the problem.

Thanks.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#50370: Fix bug in ispell-init-process error handling
       [not found]     ` <dc51f1a1-953c-45e9-90ea-6ca8c0288ed7@Spark>
@ 2021-09-04 18:40       ` Eli Zaretskii
  2021-09-04 19:38         ` Ian W
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2021-09-04 18:40 UTC (permalink / raw)
  To: Ian W; +Cc: 50370

[Please use Reply All to keep the bug address on the CC list.]

> Date: Sat, 4 Sep 2021 11:33:42 -0700
> From: Ian W <ian@wahbe.com>
> 
> I think it’s a better error message, but that is just my opinion. It tells me that ispell is trying to read an invalid
> dictionary file. In my case, I deleted that file and everything started working. I also think it’s the error message
> intended to be displayed (by the ispell program and ispell package), as it is the error message designed to
> help the user solve their problem.
> 
> (t
>  ;; Otherwise, it must be an error message. Show the user.
>  ;; But first wait to see if some more output is going to arrive.
>  ;; Otherwise we get cool errors like "Can't open ".
>  (sleep-for 1)
>  (ispell-accept-output 3) ;; <<< This is the line that errors >>>
>  (error "%s" (mapconcat #'identity ispell-filter "\n"))))
> 
> So the intended error message is never displayed. 
> 
> I’m sorry for not including a repo. I got it consistently because I had a bad file cached by aspell (or maybe
> doom). You also get the same message when you didn’t install your dictionaries correctly. This is a problem
> in how ispell displays errors, so it’s consistent but only if you already had a real problem with ispell.
> 
> To repo:
> (setq ispell-local-dictionary-overridden t
>           ispell-local-dictionary "not-a-dict")
> 
> This misconfigures your local dictionary, and ensures that the ispell process will error on start. Then run
> ispell-word on any word. Thanks for the prompt response. 

With the above recipe, I get:

  ispell-phaf: No matching entry for not-a-dict in ‘ispell-hunspell-dict-paths-alist’.

(My speller is Hunspell.)

So does this mean this problem doesn't happen with Hunspell?  Ir is
the recipe incomplete or something?





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#50370: Fix bug in ispell-init-process error handling
  2021-09-04 18:40       ` Eli Zaretskii
@ 2021-09-04 19:38         ` Ian W
  2021-09-05  7:32           ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Ian W @ 2021-09-04 19:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 50370

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

I think that hunspell is different. It validates that the dictionary really exists in the beginning of ispell-start-process:

(defun ispell-start-process ()
 "Start the Ispell process, with support for no asynchronous processes.
Keeps argument list for future Ispell invocations for no async support."
 ;; `ispell-current-dictionary' and `ispell-current-personal-dictionary'
 ;; are properly set in `ispell-internal-change-dictionary'.

 ;; Parse hunspell affix file if using hunspell and entry is uninitialized.
 (if ispell-really-hunspell
 (or (cadr (assoc ispell-current-dictionary ispell-dictionary-alist))
 (ispell-hunspell-fill-dictionary-entry ispell-current-dictionary)))
…)

Having seen this, I think that this will only be a problem for ispell and aspell. I just tested the recipe and it works on “emacs -Q”  (I have ispell installed correctly, and emacs defaults to that).
On Sep 4, 2021, 11:40 AM -0700, Eli Zaretskii <eliz@gnu.org>, wrote:
> [Please use Reply All to keep the bug address on the CC list.]
>
> > Date: Sat, 4 Sep 2021 11:33:42 -0700
> > From: Ian W <ian@wahbe.com>
> >
> > I think it’s a better error message, but that is just my opinion. It tells me that ispell is trying to read an invalid
> > dictionary file. In my case, I deleted that file and everything started working. I also think it’s the error message
> > intended to be displayed (by the ispell program and ispell package), as it is the error message designed to
> > help the user solve their problem.
> >
> > (t
> > ;; Otherwise, it must be an error message. Show the user.
> > ;; But first wait to see if some more output is going to arrive.
> > ;; Otherwise we get cool errors like "Can't open ".
> > (sleep-for 1)
> > (ispell-accept-output 3) ;; <<< This is the line that errors >>>
> > (error "%s" (mapconcat #'identity ispell-filter "\n"))))
> >
> > So the intended error message is never displayed.
> >
> > I’m sorry for not including a repo. I got it consistently because I had a bad file cached by aspell (or maybe
> > doom). You also get the same message when you didn’t install your dictionaries correctly. This is a problem
> > in how ispell displays errors, so it’s consistent but only if you already had a real problem with ispell.
> >
> > To repo:
> > (setq ispell-local-dictionary-overridden t
> > ispell-local-dictionary "not-a-dict")
> >
> > This misconfigures your local dictionary, and ensures that the ispell process will error on start. Then run
> > ispell-word on any word. Thanks for the prompt response.
>
> With the above recipe, I get:
>
> ispell-phaf: No matching entry for not-a-dict in ‘ispell-hunspell-dict-paths-alist’.
>
> (My speller is Hunspell.)
>
> So does this mean this problem doesn't happen with Hunspell? Ir is
> the recipe incomplete or something?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#50370: Fix bug in ispell-init-process error handling
  2021-09-04 19:38         ` Ian W
@ 2021-09-05  7:32           ` Eli Zaretskii
  0 siblings, 0 replies; 5+ messages in thread
From: Eli Zaretskii @ 2021-09-05  7:32 UTC (permalink / raw)
  To: Ian W; +Cc: 50370-done

> Date: Sat, 4 Sep 2021 12:38:40 -0700
> From: Ian W <ian@wahbe.com>
> Cc: 50370@debbugs.gnu.org
> 
> I think that hunspell is different. It validates that the dictionary really exists in the beginning of
> ispell-start-process:
> 
> (defun ispell-start-process ()
>  "Start the Ispell process, with support for no asynchronous processes.
> Keeps argument list for future Ispell invocations for no async support."
>  ;; `ispell-current-dictionary' and `ispell-current-personal-dictionary'
>  ;; are properly set in `ispell-internal-change-dictionary'.
> 
>  ;; Parse hunspell affix file if using hunspell and entry is uninitialized.
>  (if ispell-really-hunspell
>  (or (cadr (assoc ispell-current-dictionary ispell-dictionary-alist))
>  (ispell-hunspell-fill-dictionary-entry ispell-current-dictionary)))
> …)
> 
> Having seen this, I think that this will only be a problem for ispell and aspell. I just tested the recipe and it
> works on “emacs -Q”  (I have ispell installed correctly, and emacs defaults to that). 

Thanks, I installed the change, and I'm therefore closing this bug.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-05  7:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <e086ce83-5262-4007-aff3-56fa84427459@Spark>
2021-09-04  3:53 ` bug#50370: Fix bug in ispell-init-process error handling Ian W
2021-09-04 12:00   ` Eli Zaretskii
     [not found]     ` <dc51f1a1-953c-45e9-90ea-6ca8c0288ed7@Spark>
2021-09-04 18:40       ` Eli Zaretskii
2021-09-04 19:38         ` Ian W
2021-09-05  7:32           ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).