Khady / merlin-eldoc

Type and doc on hover for OCaml and Reason in emacs
GNU General Public License v3.0
43 stars 3 forks source link

Improve timer handling #13

Closed Khady closed 6 years ago

Khady commented 6 years ago

https://www.reddit.com/r/emacs/comments/8fbqxz/comment/dy32xwu

1- in minor modes, I've found it's a good practice to start by unconditionally running the "disable" part of the code (in your case merlin-auto-hl--cancel-timer) and then if the mode var is non-nil, run the activation code.

https://www.reddit.com/r/emacs/comments/8fbqxz/comment/dy2din3

After canceling the timer you should set the variable to nil. Otherwise you leave a stale timer struct sitting around, which can be misleading.

As far as Emacs is concerned, timers are global objects — e.g. they still run even if you don't hold onto the timer object. The timer struct is a handle to some otherwise internal Emacs object. If you want a timer to be "local" it's just a matter of handling it in a localized context. Since you're using an idle timer, you have a couple of different options.

First, you could make merlin-auto-hl--identifier-timer a buffer-local variable, and then each buffer with your minor mode enabled gets its own timer. You need to be careful to clean up this timer when the buffer is destroyed or when the mode is disabled. The timer callback must know which buffer it should act upon. This means either passing that buffer to the callback via the ARGS argument to run-with-idle-timer. Or you could capture the target buffer in a closure (when lexical binding is enabled). Either way, the callback needs to temporarily switch to its target buffer (with-current-buffer) before taking action. Timers don't keep track of the current buffer when they were created, and the callback could happen anywhere.

The way you've written it now, it will only act upon the current buffer, and only if that buffer is running your minor mode.

The second option, which I think is better, is to have a single, global idle timer. When a buffer has the minor mode enabled, you register it in some global list. If the registry list is empty, start the idle timer. When the mode is disabled, or the buffer is killed, remove it from the registry. If the registry list becomes empty, cancel the idle timer (or let the callback cancel it if the list is empty or only contains dead buffers). When the timer function runs, it goes through each registered buffer and, if "dirty" (e.g. modified since the timer callback last operated on it), does its thing in it, marking it "clean" in the process.

Khady commented 6 years ago

Use eldoc instead.