bug#69733: [PATCH] Flyspell (flyspell-word): do not force 'save-excursion' on timers

[Daniel Pettersson]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, but I don't think I understand the relation between what you
> say above (and in the comments in your patch below) and the code
> change you propose:

flyspell-word wraps it's body inside an `save-excursion'.

(defun flyspell-word (&optional following known-misspelling)
...
  (save-excursion
...
>>                    (while (progn
>> -                           (accept-process-output ispell-process)
>> +                           ;; don't force save-excursion to timers.
>> +                           ;; only accept output from ispell-process.
>> +                           (accept-process-output ispell-process nil nil t)

> Where's save-excursion and point moving to which you allude here?

See above and for point moving I think that the stack trace at the
bottom of the email highlights my point.

> More importantly, why is it a good idea to stop running timers during
> this accept-process-output call and ignore output from other
> subprocesses?

I don't think that is the optimal solution.  But this does not ignore
output right, it just delays timers and filter functions.  If flyspell
is active in an comit buffer it might also screw up the point if process
outputs stuff during `accept-process-output'.

The best solution would be to run `accept-process-output' outside of the
body of save-excursion, but that would require a bit more mangling,
which might be worth the effort.

Let's create an simple example to illustrate the issue with current
implementation.  This is a highly construed example but there is always
the chance that flyspell has unexpected interactions with all buffers
that filter functions and timer functions modify the point of.

(defun timer-goto-point-min ()
  (goto-char (point-min)))

(defun some-command ()
  (interactive)
  ;; flyspell needs point to have been and at word.
  (goto-char (point-min))
  (re-search-forward "word.")
  ;; Goto top of buffer with timer
  (run-with-timer 0 0 'timer-goto-point-min))

(trace-function #'timer-goto-point-min)
(trace-function 'flyspell-word)
(trace-function 'accept-process-output)

;; Prepare buffer
(with-current-buffer (get-buffer-create "buffer")
  (save-excursion
    (insert "Some sentence with words."))
  (flyspell-mode))

;; Open buffer M-x some-command expects point to be at the start of
;; buffer 

;; 1 -> (flyspell-word)                    -- start of save-excursion
;; | 2 -> (accept-process-output #<process ispell>)
;; | | 3 -> (timer-goto-point-min)         -- moves point
;; | | 3 <- timer-goto-point-min: 1
;; | 2 <- accept-process-output: t
;; 1 <- flyspell-word: nil                 -- end of save-excursion
;;                                            resets the point


/Daniel