Re: master d378615: Cancel timer when world-clock buffer is killed

[Basil L. Contovounesios]

Stefan Kangas <stefankangas@gmail.com> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>>> +    (add-hook 'kill-buffer-hook #'world-clock-cancel-timer))
>>
>> Should this be hooked buffer-locally?
>
> Yup.
>
>>> +(defvar world-clock-timer nil)
>>
>> Should this be given an internal name (and docstring)?
>>
>>> +(defun world-clock-cancel-timer ()
>>> +  "Cancel the world clock timer."
>>> +  (when world-clock-timer
>>> +    (cancel-timer world-clock-timer)
>>> +    (setq world-clock-timer nil)))
>>> +
>>
>> It seems to me that world-clock-cancel-timer and world-clock-update are
>> duplicating each other.  Or am I missing something?
>
> Oh, right...  I've pushed a fixed version where I removed the above new
> variable and moved the code to cancel the timer from
> `world-clock-update' to `world-clock-cancel-timer'.

Why not keep the (internal) variable and pass it to cancel-timer
directly instead of searching through timer-list for a particular entry?

> (I couldn't just add `world-clock-cancel-timer' to `kill-buffer-hook'
> because the buffer still exists when the hook is run.)
>
> Thanks for the review!

Thanks for improving world-clock! :)

-- 
Basil