search

tmalsburg / guess-language.el

Emacs minor mode that detects the language you're typing in. Automatically switches spell checker. Supports multiple languages per document.
115 stars 14 forks source link

run-at-time #26

Closed malb closed 4 years ago

malb commented 4 years ago

I was wondering what the rationale is behind the run-at-time at https://github.com/tmalsburg/guess-language.el/blob/master/guess-language.el#L247 ? I'm getting a lot of "Blocking call to accept-process-output with quit inhibited" as a result of this. This isn't so bad, but Emacs would also occasionally hang because two of those run-at-time at the same time seem to cause problems (?)

Just calling the body of that function locally seems to work, but I didn't extensively test it.

joostkremers commented 4 years ago

Alternatively, wrap the call to flyspell-region inside run-at-time in with-local-quit? Since that seems to be what the Elisp manual suggests: https://www.gnu.org/software/emacs/manual/html_node/elisp/Timers.html#Timers.

tmalsburg commented 4 years ago

Never noticed this but I'm getting these messages as well. The run-at-time was introduced to make sure the word under point is correctly highlighted after a switch of dictionaries. I tried using with-local-quit but that doesn't work as desired. Not sure what the underlying issue is and how to fix it. :(

joostkremers commented 4 years ago

Never noticed this but I'm getting these messages as well. The run-at-time was introduced to make sure the word under point is correctly highlighted after a switch of dictionaries.

How exactly does run-at-time accomplish that, if I may ask? Doesn't passing 0 as the time to run the function mean that it gets run immediately?

I tried using with-local-quit but that doesn't work as desired.

What do you expect it to do? I added a with-local-quit around (flyspell-region beginning end) and it seems to work fine. guess-language still changes the dictionary according to the language of the current paragraph and I'm not getting any "Blocking call to accept-process-output with quit inhibited" errors anymore.

Not sure what the underlying issue is and how to fix it. :(

From what I gather through Google, a timer shouldn't call an external process, because when the timer's function is run, inhibit-quit is set to t. If the external process hangs, Emacs hangs as well and because inhibit-quit is set, the user cannot use C-g to regain control.

tmalsburg commented 4 years ago

How exactly does run-at-time accomplish that, if I may ask? Doesn't passing 0 as the time to run the function mean that it gets run immediately?

Remember that Emacs is single-threaded. If you run-at-time 0, the function is scheduled to be executed as soon as possible. But at the earliest after the current execution thread has finished. Example:

(progn
  (message "First")
  (run-at-time 0 nil (lambda () (message "This is run at time 0.")))
  (message "Finish"))

This produces:

First
Finish
This is run at time 0.

So the run-at-time 0 makes sure flyspell is run only after we finish switching the dictionary. As to why this is necessary, I'm not sure because my understanding of flyspell and Emacs isn't deep enough. Perhaps @wentasah can clarily this (he contributed the relevant code). I understand that the run-at-time solution is perhaps not optimal, but it fixes a bug and largely seems to work.

tmalsburg commented 4 years ago

p.s: The commit message actually explains why run-at-time 0 is necessary. Quoting:

When the dictionary is switched as a result of detecting different language, the word under point remains highlighted as incorrect (according to the previously used dictionary) until the word is flyspelled again, for example by moving the point out of the word and back. This is annoying.

This commit fixes that by ensuring that flyspelling the current paragraph with the new dictionary happens after the return from flyspell-incorrect-hook, which highlights the word under point according to the old dictionary.

wentasah commented 4 years ago

I was not aware that timer handlers are called with inhibit-quit set to t. From my reading of the manual, I'd say that with-local-quit should be added around flyspell-region as suggested by @joostkremers.

tmalsburg commented 4 years ago

Seems to work. @joostkremers, based on your question about the function of run-at-time I misinterpreted you as suggesting to replace run-at-time with with-local-quit. Sorry.

joostkremers commented 4 years ago

Seems to work. @joostkremers, based on your question about the function of run-at-time I misinterpreted you as suggesting to replace run-at-time with with-local-quit. Sorry.

No problem, glad it got fixed!