From 19e103fea2e55a3e3bc3a777b4c618e644b57760 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 28 Jun 2026 03:56:24 -0400 Subject: fix: stop the mode-line cleanly when disabled before startup fires The delayed initial fetch was scheduled with run-at-time but the one-shot timer was discarded, and when the mode is enabled before after-init the start is queued on after-init-hook. Disabling the mode before either fired could still start timers, hit the network, or mutate mode-line state after the user turned it off. Stop now cancels the stored startup timer and removes the after-init hook, and the startup callback no-ops when the mode is off. --- tests/test-wttrin--mode-line-start.el | 12 ++++-- tests/test-wttrin--mode-line-timer-lifecycle.el | 55 +++++++++++++++++++++++++ wttrin.el | 31 +++++++++++++- 3 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 tests/test-wttrin--mode-line-timer-lifecycle.el diff --git a/tests/test-wttrin--mode-line-start.el b/tests/test-wttrin--mode-line-start.el index 1b86689..21cdb24 100644 --- a/tests/test-wttrin--mode-line-start.el +++ b/tests/test-wttrin--mode-line-start.el @@ -21,7 +21,8 @@ (setq wttrin-mode-line-string nil) (setq wttrin--mode-line-cache nil) (setq wttrin--mode-line-timer nil) - (setq wttrin--buffer-refresh-timer nil)) + (setq wttrin--buffer-refresh-timer nil) + (setq wttrin--mode-line-startup-timer nil)) (defun test-wttrin--mode-line-start-teardown () "Teardown for mode-line-start tests." @@ -30,10 +31,13 @@ (cancel-timer wttrin--mode-line-timer)) (when (timerp wttrin--buffer-refresh-timer) (cancel-timer wttrin--buffer-refresh-timer)) + (when (timerp wttrin--mode-line-startup-timer) + (cancel-timer wttrin--mode-line-startup-timer)) (setq wttrin-mode-line-string nil) (setq wttrin--mode-line-cache nil) (setq wttrin--mode-line-timer nil) - (setq wttrin--buffer-refresh-timer nil)) + (setq wttrin--buffer-refresh-timer nil) + (setq wttrin--mode-line-startup-timer nil)) ;;; Normal Cases @@ -70,8 +74,10 @@ (null (plist-get call :repeat)))) scheduled-calls))) (should initial-fetch) + ;; The delayed fetch goes through the enabled-guard wrapper so a + ;; disable before it fires is a no-op. (should (eq (plist-get initial-fetch :func) - #'wttrin--mode-line-fetch-weather))))) + #'wttrin--mode-line-fetch-weather-if-enabled))))) (test-wttrin--mode-line-start-teardown))) (ert-deftest test-wttrin--mode-line-start-normal-creates-repeating-mode-line-timer () diff --git a/tests/test-wttrin--mode-line-timer-lifecycle.el b/tests/test-wttrin--mode-line-timer-lifecycle.el new file mode 100644 index 0000000..2cd5b9f --- /dev/null +++ b/tests/test-wttrin--mode-line-timer-lifecycle.el @@ -0,0 +1,55 @@ +;;; test-wttrin--mode-line-timer-lifecycle.el --- Mode-line timer lifecycle -*- lexical-binding: t; -*- + +;; Copyright (C) 2024-2026 Craig Jennings + +;;; Commentary: +;; Tests that disabling wttrin-mode-line-mode before the delayed startup fetch +;; fires cannot start the mode-line: the one-shot startup timer is cancelled and +;; the deferred after-init hook is removed, and the startup callback no-ops when +;; the mode is off. + +;;; Code: + +(require 'ert) +(require 'cl-lib) +(require 'wttrin) +(require 'testutil-wttrin) + +(ert-deftest test-wttrin--mode-line-fetch-weather-if-enabled-normal-runs-when-enabled () + "Normal: the delayed startup fetch runs when the mode is on." + (let ((called nil)) + (cl-letf (((symbol-function 'wttrin--mode-line-fetch-weather) + (lambda (&rest _) (setq called t)))) + (let ((wttrin-mode-line-mode t)) + (wttrin--mode-line-fetch-weather-if-enabled) + (should called))))) + +(ert-deftest test-wttrin--mode-line-fetch-weather-if-enabled-error-skips-when-disabled () + "Error: the delayed startup fetch is skipped when the mode was turned off +before it fired, so a disabled mode never hits the network or mutates state." + (let ((called nil)) + (cl-letf (((symbol-function 'wttrin--mode-line-fetch-weather) + (lambda (&rest _) (setq called t)))) + (let ((wttrin-mode-line-mode nil)) + (wttrin--mode-line-fetch-weather-if-enabled) + (should-not called))))) + +(ert-deftest test-wttrin--mode-line-stop-boundary-cancels-startup-timer-and-hook () + "Boundary: stop cancels a pending startup timer and removes the after-init +hook, so neither can start the mode-line after the user disabled it." + (let ((wttrin--mode-line-timer nil) + (wttrin--buffer-refresh-timer nil) + (wttrin--mode-line-startup-timer (run-at-time 9999 nil #'ignore)) + (wttrin-mode-line-string nil) + (wttrin--mode-line-cache nil) + (wttrin--mode-line-rendered-stale nil)) + (add-hook 'after-init-hook #'wttrin--mode-line-start) + (unwind-protect + (progn + (wttrin--mode-line-stop) + (should (null wttrin--mode-line-startup-timer)) + (should-not (memq #'wttrin--mode-line-start after-init-hook))) + (remove-hook 'after-init-hook #'wttrin--mode-line-start)))) + +(provide 'test-wttrin--mode-line-timer-lifecycle) +;;; test-wttrin--mode-line-timer-lifecycle.el ends here diff --git a/wttrin.el b/wttrin.el index 4bc1b3a..f109ed2 100644 --- a/wttrin.el +++ b/wttrin.el @@ -1614,6 +1614,13 @@ scheduled refresh." (defvar wttrin--buffer-refresh-timer nil "Timer object for proactive buffer cache refresh.") +(defvar wttrin--mode-line-startup-timer nil + "One-shot timer for the delayed initial mode-line fetch.") + +;; Defined by the `wttrin-mode-line-mode' minor mode below; declared here so +;; the startup guard can read it without a free-variable warning. +(defvar wttrin-mode-line-mode) + (defun wttrin--buffer-cache-refresh () "Proactively refresh the buffer cache for `wttrin-favorite-location'. Fetches fresh weather data and updates the buffer cache entry without @@ -1630,6 +1637,14 @@ geolocation has not yet resolved, this call is a no-op." (wttrin--cleanup-cache-if-needed) (puthash cache-key (cons (float-time) fresh-data) wttrin--cache)))))))) +(defun wttrin--mode-line-fetch-weather-if-enabled () + "Fetch mode-line weather only if `wttrin-mode-line-mode' is still enabled. +The initial fetch is delayed, so the mode can be turned off before it fires. +This guard keeps a disabled mode from hitting the network or mutating +mode-line state." + (when wttrin-mode-line-mode + (wttrin--mode-line-fetch-weather))) + (defun wttrin--mode-line-start () "Start mode-line weather display and refresh timer." (wttrin--debug-log "wttrin mode-line: Starting mode-line display (location=%s, interval=%s)" @@ -1641,8 +1656,14 @@ geolocation has not yet resolved, this call is a no-op." ;; on the next tick. (wttrin--resolve-favorite-location) (wttrin--mode-line-set-placeholder) - ;; Delay first fetch — network/daemon may not be ready at startup - (run-at-time wttrin-mode-line-startup-delay nil #'wttrin--mode-line-fetch-weather) + ;; Delay first fetch — network/daemon may not be ready at startup. + ;; Store the one-shot timer so stop can cancel it, and guard the callback + ;; so a disable before it fires is a no-op. + (when wttrin--mode-line-startup-timer + (cancel-timer wttrin--mode-line-startup-timer)) + (setq wttrin--mode-line-startup-timer + (run-at-time wttrin-mode-line-startup-delay nil + #'wttrin--mode-line-fetch-weather-if-enabled)) ;; Cancel existing timers to prevent duplicates on re-enable (when wttrin--mode-line-timer (cancel-timer wttrin--mode-line-timer)) @@ -1669,6 +1690,12 @@ geolocation has not yet resolved, this call is a no-op." (when wttrin--buffer-refresh-timer (cancel-timer wttrin--buffer-refresh-timer) (setq wttrin--buffer-refresh-timer nil)) + (when wttrin--mode-line-startup-timer + (cancel-timer wttrin--mode-line-startup-timer) + (setq wttrin--mode-line-startup-timer nil)) + ;; If the mode was enabled before after-init and disabled before the hook + ;; ran, the queued start would fire after disable — drop it. + (remove-hook 'after-init-hook #'wttrin--mode-line-start) (setq wttrin-mode-line-string nil) (setq wttrin--mode-line-cache nil) (setq wttrin--mode-line-rendered-stale nil) -- cgit v1.2.3