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

[Basil L. Contovounesios]

stefankangas@gmail.com (Stefan Kangas) writes:

> branch: master
> commit d37861535dfd452f7c2255ae5edcf7686b75fe5a
> Author: Stefan Kangas <stefankangas@gmail.com>
> Commit: Stefan Kangas <stefankangas@gmail.com>
>
>     Cancel timer when world-clock buffer is killed
>     
>     * lisp/time.el (world-clock-timer): New variable.
>     (world-clock-cancel-timer): New defun.
>     (world-clock): Add 'world-clock-cancel-timer' to 'kill-buffer-hook'.
> ---
>  lisp/time.el | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/time.el b/lisp/time.el
> index 1ab992a..5ced920 100644
> --- a/lisp/time.el
> +++ b/lisp/time.el
> @@ -523,6 +523,8 @@ See `world-clock'."
>    (setq-local revert-buffer-function #'world-clock-update)
>    (setq show-trailing-whitespace nil))
>  
> +(defvar world-clock-timer nil)

Should this be given an internal name (and docstring)?

>  (defun world-clock-display (alist)
>    "Replace current buffer text with times in various zones, based on ALIST."
>    (let ((inhibit-read-only t)
> @@ -561,12 +563,20 @@ To turn off the world time display, go to the window 
> and type `\\[quit-window]'.
>    (interactive)
>    (when (and world-clock-timer-enable
>               (not (get-buffer world-clock-buffer-name)))
> -    (run-at-time t world-clock-timer-second #'world-clock-update))
> +    (setq world-clock-timer
> +          (run-at-time t world-clock-timer-second #'world-clock-update))
> +    (add-hook 'kill-buffer-hook #'world-clock-cancel-timer))

Should this be hooked buffer-locally?

>    (pop-to-buffer world-clock-buffer-name)
>    (world-clock-display (time--display-world-list))
>    (world-clock-mode)
>    (fit-window-to-buffer))
>  
> +(defun world-clock-cancel-timer ()
> +  "Cancel the world clock timer."
> +  (when world-clock-timer
> +    (cancel-timer world-clock-timer)
> +    (setq world-clock-timer nil)))
> +
>  (defun world-clock-update (&optional _arg _noconfirm)
>    "Update the `world-clock' buffer."
>    (if (get-buffer world-clock-buffer-name)

Actually, doesn't world-clock-update already try to cancel itself when
world-clock-buffer-name no longer names an existing buffer?

It seems to me that world-clock-cancel-timer and world-clock-update are
duplicating each other.  Or am I missing something?

Thanks,

-- 
Basil