bug#74437: 30.0.92; completion-preview-idle-delay is delayed by flyspell

[Eshel Yaron]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: 74437@debbugs.gnu.org,  mail@alternateved.com
>> Date: Tue, 19 Nov 2024 19:40:07 +0100
>> 
>> +(defcustom flyspell-blocking-delay t
>
> I'd name this flyspell-use-timer or somesuch.

OK, will do.  Actually, how about flyspell-delay-use-timer?  Just
flyspell-use-timer feels a bit too general: Flyspell might use timers
for other purposes regardless of this option.

>> +  "Whether to block Emacs while Flyspell waits after a delayed command."
>> +  :type 'boolean
>> +  :version "30.1")
>      ^^^^^^^^^^^^^^
> You are an optimist ;-)

Oh well, 31.1 it is then :)

>> -                           (accept-process-output ispell-process)
>> +                           (accept-process-output ispell-process 1)
>
> Does this really work reliably from a timer?

I don't immediately see why it shouldn't, and the tests I tried so far
seem to work, but your skepticism makes me wonder if there's anything
I'm missing.  Do you have any particular potential issue in mind?

> You _are_ aware that this call to accept-process-output will run
> timers while we wait?

Yes.  If that's a cause for concern, we can inhibit running timers here
when we're calling flyspell-word from a timer, like in the patch below.

> Also, what happens if there are other async
> subprocesses running in parallel, like maybe Grep or compilation or
> url-retrieve?

They make progress, which seems to work as expected, at least with Grep.
That is if we use the previous patch, with the one below we pass non-nil
JUST-THIS-ONE argument to accept-process-output when called from a timer
so other processes shouldn't see new output during this call.  Either
way works, AFAICT.


diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index 7bf9cb1ae9d..5442b46cd1d 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -810,6 +810,13 @@ flyspell-check-changed-word-p
              (let ((pos (point)))
                (or (>= pos start) (<= pos stop) (= pos (1+ stop))))))))
 
+(defcustom flyspell-delay-use-timer nil
+  "Whether to use a timer for waiting after a delayed command."
+  :type 'boolean
+  :version "31.1")
+
+(defvar flyspell--timer nil)
+
 ;;*---------------------------------------------------------------------*/
 ;;*    flyspell-check-word-p ...                                        */
 ;;*---------------------------------------------------------------------*/
@@ -844,7 +851,16 @@ flyspell-check-word-p
        ;; The current command is not delayed, that
        ;; is that we must check the word now.
        (and (not unread-command-events)
-            (sit-for flyspell-delay)))
+             (if (not flyspell-delay-use-timer)
+                 (sit-for flyspell-delay)
+               (setq flyspell--timer
+                     (run-with-idle-timer
+                      flyspell-delay nil
+                      (lambda (buffer)
+                        (when (eq (current-buffer) buffer)
+                          (flyspell-word nil nil t)))
+                      (current-buffer)))
+               nil)))
        (t t)))
      (t t))))
 
@@ -955,6 +971,7 @@ flyspell-debug-signal-changed-checked
 (defun flyspell-post-command-hook ()
   "The `post-command-hook' used by flyspell to check a word on-the-fly."
   (interactive)
+  (when (timerp flyspell--timer) (cl-callf cancel-timer flyspell--timer))
   (when flyspell-mode
     (with-local-quit
       (let ((command this-command)
@@ -1095,13 +1112,14 @@ flyspell-word
 ;;*---------------------------------------------------------------------*/
 ;;*    flyspell-word ...                                                */
 ;;*---------------------------------------------------------------------*/
-(defun flyspell-word (&optional following known-misspelling)
+(defun flyspell-word (&optional following known-misspelling block-timers)
   "Spell check a word.
 If the optional argument FOLLOWING, or, when called interactively
 `ispell-following-word', is non-nil, checks the following (rather
 than preceding) word when the cursor is not over a word.  If
 optional argument KNOWN-MISSPELLING is non-nil considers word a
-misspelling and skips redundant spell-checking step.
+misspelling and skips redundant spell-checking step.  Non-nil optional
+argument BLOCK-TIMERS says not to run timers while spell-checking.
 
 See `flyspell-get-word' for details of how this finds the word to
 spell-check."
@@ -1179,7 +1197,8 @@ flyspell-word
                  (set-process-query-on-exit-flag ispell-process nil)
                   ;; Wait until ispell has processed word.
                   (while (progn
-                           (accept-process-output ispell-process)
+                           (accept-process-output ispell-process 1 nil
+                                                  (when block-timers 0))
                            (not (string= "" (car ispell-filter)))))
                   ;; (ispell-send-string "!\n")
                   ;; back to terse mode.