From f18f1c90e362d84f4978b4365ae0a88807afb6e4 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sat, 8 Nov 2025 11:37:29 -0600 Subject: refactor: mode-line: extract inline keymap to shared defvar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace inline keymap construction with shared wttrin--mode-line-map defvar. Before (line 474-479): Keymap recreated on every mode-line update - (let ((map (make-sparse-keymap))) ...) - Allocates new keymap object each time - 6 lines of repetitive code After (line 183-190 + line 483): Shared keymap created once - (defvar wttrin--mode-line-map ...) - Reference: 'local-map wttrin--mode-line-map - Single allocation, no repeated construction Added comprehensive tests (8 tests, all passing): - Keymap existence and structure verification - Keybinding tests (mouse-1, mouse-3, no mouse-2) - Integration test verifying mode-line uses shared map Benefits: - More efficient (no allocation on every update) - Clearer code structure - Easier to add new keybindings - Self-documenting with inline documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- tests/test-wttrin--mode-line-map.el | 79 +++++++++++++++++++++++++++++++++++++ wttrin.el | 16 +++++--- 2 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 tests/test-wttrin--mode-line-map.el diff --git a/tests/test-wttrin--mode-line-map.el b/tests/test-wttrin--mode-line-map.el new file mode 100644 index 0000000..ed227cf --- /dev/null +++ b/tests/test-wttrin--mode-line-map.el @@ -0,0 +1,79 @@ +;;; test-wttrin--mode-line-map.el --- Tests for wttrin--mode-line-map -*- lexical-binding: t; -*- + +;; Copyright (C) 2024 Craig Jennings + +;;; Commentary: +;; Unit tests for wttrin--mode-line-map keymap. +;; Tests verify the keymap exists and has correct keybindings after refactoring. + +;;; Code: + +(require 'ert) +(require 'wttrin) + +;;; Normal Cases - Keymap Existence and Structure + +(ert-deftest test-wttrin--mode-line-map-exists () + "Test that wttrin--mode-line-map defvar exists after refactoring." + (should (boundp 'wttrin--mode-line-map))) + +(ert-deftest test-wttrin--mode-line-map-is-keymap () + "Test that wttrin--mode-line-map is actually a keymap." + (should (keymapp wttrin--mode-line-map))) + +(ert-deftest test-wttrin--mode-line-map-is-sparse-keymap () + "Test that wttrin--mode-line-map is a sparse keymap." + ;; Sparse keymaps start with 'keymap symbol + (should (eq 'keymap (car wttrin--mode-line-map)))) + +;;; Keybinding Tests + +(ert-deftest test-wttrin--mode-line-map-has-mouse-1-binding () + "Test that left-click (mouse-1) is bound to wttrin-mode-line-click." + (let ((binding (lookup-key wttrin--mode-line-map [mode-line mouse-1]))) + (should (eq binding 'wttrin-mode-line-click)))) + +(ert-deftest test-wttrin--mode-line-map-has-mouse-3-binding () + "Test that right-click (mouse-3) is bound to wttrin-mode-line-force-refresh." + (let ((binding (lookup-key wttrin--mode-line-map [mode-line mouse-3]))) + (should (eq binding 'wttrin-mode-line-force-refresh)))) + +;;; Boundary Cases - Verify No Unexpected Bindings + +(ert-deftest test-wttrin--mode-line-map-no-mouse-2-binding () + "Test that middle-click (mouse-2) has no binding." + (let ((binding (lookup-key wttrin--mode-line-map [mode-line mouse-2]))) + ;; Should return nil (unbound) or a prefix keymap + (should-not (and binding (symbolp binding))))) + +(ert-deftest test-wttrin--mode-line-map-minimal-bindings () + "Test that keymap has the expected bindings." + ;; Count non-nil entries in keymap + (let ((count 0)) + (map-keymap (lambda (_key _binding) + (setq count (1+ count))) + wttrin--mode-line-map) + ;; Should have at least 1 binding (could be 1 or 2 depending on how + ;; Emacs internally structures the keymap with [mode-line mouse-N]) + ;; The important thing is both mouse-1 and mouse-3 are accessible + (should (>= count 1)))) + +;;; Integration Test - Verify Mode-Line Uses the Keymap + +(ert-deftest test-wttrin-mode-line-display-uses-shared-keymap () + "Test that mode-line display uses wttrin--mode-line-map after refactoring. +This test verifies the refactoring eliminated inline keymap construction." + ;; Set up minimal mode-line state + (let ((wttrin-mode-line-favorite-location "Test, CA") + (wttrin--mode-line-tooltip-data "Test weather")) + ;; Update the mode-line display + (wttrin--mode-line-update-display "☀️") + + ;; Extract the keymap property from wttrin-mode-line-string + (let ((keymap-prop (get-text-property 0 'local-map wttrin-mode-line-string))) + ;; After refactoring, should use the shared wttrin--mode-line-map + ;; Not a freshly constructed keymap on each call + (should (eq keymap-prop wttrin--mode-line-map))))) + +(provide 'test-wttrin--mode-line-map) +;;; test-wttrin--mode-line-map.el ends here diff --git a/wttrin.el b/wttrin.el index afc0d2f..1ea8e11 100644 --- a/wttrin.el +++ b/wttrin.el @@ -180,6 +180,15 @@ Set this to t BEFORE loading wttrin, typically in your init file: (defvar wttrin--mode-line-tooltip-data nil "Cached full weather data for tooltip display.") +(defvar wttrin--mode-line-map + (let ((map (make-sparse-keymap))) + (define-key map [mode-line mouse-1] 'wttrin-mode-line-click) + (define-key map [mode-line mouse-3] 'wttrin-mode-line-force-refresh) + map) + "Keymap for mode-line weather widget interactions. +Left-click (mouse-1): refresh weather and open buffer. +Right-click (mouse-3): force-refresh cache and update tooltip.") + (defun wttrin-additional-url-params () "Concatenates extra information into the URL." (if wttrin-unit-system @@ -471,12 +480,7 @@ WEATHER-STRING format: \"Location: emoji temp conditions\" (e.g., \"Paris: ☀ (format "Weather for %s\nClick to refresh" wttrin-mode-line-favorite-location))) 'mouse-face 'mode-line-highlight - 'local-map (let ((map (make-sparse-keymap))) - (define-key map [mode-line mouse-1] - 'wttrin-mode-line-click) - (define-key map [mode-line mouse-3] - 'wttrin-mode-line-force-refresh) - map)))) + 'local-map wttrin--mode-line-map))) (force-mode-line-update t) (when (featurep 'wttrin-debug) (message "wttrin mode-line: Display updated, mode-line-string = %S, tooltip = %S" -- cgit v1.2.3