refactor: move and test theme persistence behavior

3 files changed, 191 insertions, 36 deletions

diff --git a/init.el b/init.el
index 3f10da20..d66bea34 100644
--- a/init.el
+++ b/init.el
@@ -56,6 +56,7 @@
 
 (require 'ui-config)             ;; transparency, cursor color, icons, &c.
 (require 'ui-theme)              ;; themes and theme persistency
+(cj/load-theme-from-file)
 (require 'ui-navigation)         ;; the movement and navigation of windows
 (require 'font-config)           ;; font and emoji configuration
 (require 'selection-framework)   ;; menu config
diff --git a/modules/ui-theme.el b/modules/ui-theme.el
index bc526f38..f84d7231 100644
--- a/modules/ui-theme.el
+++ b/modules/ui-theme.el
@@ -17,10 +17,6 @@
 
 ;;; Code:
 
-(require 'user-constants)
-
-(eval-when-compile (defvar org-dir))
-
 ;; ----------------------------------- Themes ----------------------------------
 ;; theme choices and settings
 
@@ -39,8 +35,8 @@ Unloads any other applied themes before applying the chosen theme."
   (let ((chosentheme (completing-read "Load custom theme: "
                                       (mapcar #'symbol-name
                                               (custom-available-themes)))))
-    (mapc #'disable-theme custom-enabled-themes)
-    (load-theme (intern chosentheme) t))
+    (cj/theme-disable-all)
+    (cj/theme-load-name chosentheme))
   (cj/save-theme-to-file))
 
 (keymap-global-set "M-S-l" #'cj/switch-themes)  ;; was M-L, overrides downcase-word
@@ -48,18 +44,26 @@ Unloads any other applied themes before applying the chosen theme."
 ;; ----------------------------- Theme Persistence -----------------------------
 ;; persistence utility functions used by switch themes.
 
-(defvar theme-file (concat org-dir "emacs-theme.persist")
+(defgroup cj/ui-theme nil
+  "Theme persistence settings."
+  :group 'faces)
+
+(defcustom theme-file (expand-file-name ".emacs-theme" user-emacs-directory)
   "The location of the file to persist the theme name.
 If you want your theme change to persist across instances, put this in a
-directory that is sync'd across machines with this configuration.")
+directory that is sync'd across machines with this configuration."
+  :type 'file
+  :group 'cj/ui-theme)
 
-(defvar fallback-theme-name "modus-vivendi"
+(defcustom fallback-theme-name "modus-vivendi"
   "The name of the theme to fallback on.
 This is used when there's no file, or the theme name doesn't match
 any of the installed themes. This should be a built-in theme. If theme name is
-`nil', there will be no theme.")
+`nil', there will be no theme."
+  :type 'string
+  :group 'cj/ui-theme)
 
-(defun cj/read-file-contents (filename)
+(defun cj/theme-read-file-contents (filename)
   "Read FILENAME and return its content as a string.
 
 If FILENAME isn't readable, return nil."
@@ -68,20 +72,49 @@ If FILENAME isn't readable, return nil."
       (insert-file-contents filename)
       (string-trim (buffer-string)))))
 
-(defun cj/write-file-contents (content filename)
+(defun cj/theme-write-file-contents (content filename)
   "Write CONTENT to FILENAME.
 If FILENAME isn't writeable, return nil. If successful, return t."
   (when (file-writable-p filename)
     (condition-case err
         (progn
-          (with-temp-buffer
-            (insert content)
-            (write-file filename))
+          (write-region content nil filename nil 'silent)
           t)
       (error
        (message "Error writing to %s: %s" filename (error-message-string err))
        nil))))
 
+(defun cj/theme-disable-all ()
+  "Disable all currently enabled custom themes."
+  (mapc #'disable-theme (copy-sequence custom-enabled-themes)))
+
+(defun cj/theme-load-name (theme-name)
+  "Load THEME-NAME without confirmation."
+  (load-theme (intern theme-name) t))
+
+(defun cj/theme-load-fallback (msg)
+  "Display MSG and load `fallback-theme-name'."
+  (message "%s Loading fallback theme %s" msg fallback-theme-name)
+  (cj/theme-load-name fallback-theme-name))
+
+(defun cj/theme-apply-persisted-name (theme-name)
+  "Apply persisted THEME-NAME.
+Nil or empty THEME-NAME loads the fallback theme. The literal string
+\"nil\" disables all themes without loading a replacement."
+  (cj/theme-disable-all)
+  (cond
+   ((or (null theme-name) (string-empty-p theme-name))
+    (cj/theme-load-fallback "Theme file not found or empty."))
+   ((string= theme-name "nil")
+    nil)
+   (t
+    (condition-case err
+        (cj/theme-load-name theme-name)
+      (error
+       (cj/theme-load-fallback
+        (format "Error loading theme %s: %s."
+                theme-name (error-message-string err))))))))
+
 (defun cj/get-active-theme-name ()
   "Return the name of the active UI theme as a string.
 Returns fallback-theme-name if no theme is active."
@@ -91,38 +124,21 @@ Returns fallback-theme-name if no theme is active."
 
 (defun cj/save-theme-to-file ()
   "Save the string representing the current theme to the theme-file."
-  (if (not (cj/write-file-contents (cj/get-active-theme-name) theme-file))
+  (if (not (cj/theme-write-file-contents (cj/get-active-theme-name) theme-file))
       (message "Cannot save theme: %s is unwriteable" theme-file)
     (message "%s theme saved to %s" (cj/get-active-theme-name) theme-file)))
 
 (defun cj/load-fallback-theme (msg)
   "Display MSG and load ui-theme fallback-theme-name.
 Used to handle errors with loading persisted theme."
-  (message "%s Loading fallback theme %s" msg fallback-theme-name)
-  (load-theme (intern fallback-theme-name) t))
+  (cj/theme-disable-all)
+  (cj/theme-load-fallback msg))
 
 (defun cj/load-theme-from-file ()
   "Apply the theme name contained in theme-file as the active UI theme.
 If the theme is nil, it disables all current themes. If an error occurs
 loading the file name, the fallback-theme-name is applied and saved."
-  (let ((theme-name (cj/read-file-contents theme-file)))
-    ;; if theme-name is nil, unload all themes and load fallback theme
-    (if (not theme-name)
-        (progn
-          (mapc #'disable-theme custom-enabled-themes)
-          (cj/load-fallback-theme "Theme file not found or empty."))
-      ;; Check if theme is 'nil' string
-      (if (string= theme-name "nil")
-          (mapc #'disable-theme custom-enabled-themes)
-        ;; apply theme name or if error, load fallback theme
-        (condition-case err
-            (load-theme (intern theme-name) t)
-          (error
-           (cj/load-fallback-theme
-            (format "Error loading theme %s: %s."
-                    theme-name (error-message-string err)))))))))
-
-(cj/load-theme-from-file)
+  (cj/theme-apply-persisted-name (cj/theme-read-file-contents theme-file)))
 
 (provide 'ui-theme)
 ;;; ui-theme.el ends here
diff --git a/tests/test-ui-theme-persistence.el b/tests/test-ui-theme-persistence.el
new file mode 100644
index 00000000..86ed9de9
--- /dev/null
+++ b/tests/test-ui-theme-persistence.el
@@ -0,0 +1,138 @@
+;;; test-ui-theme-persistence.el --- Tests for UI theme persistence -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Smoke and unit coverage for ui-theme.el persistence behavior.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+
+(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
+
+(require 'ui-theme)
+
+(ert-deftest test-ui-theme-default-theme-file-is-emacs-dotfile ()
+  "The default theme file should live under `user-emacs-directory'."
+  (should (equal theme-file
+                 (expand-file-name ".emacs-theme" user-emacs-directory)))
+  (should-not (string-match-p "emacs-theme\\.persist\\'" theme-file)))
+
+(ert-deftest test-ui-theme-read-missing-file-returns-nil ()
+  "Reading a missing theme file should return nil."
+  (let ((theme-file (expand-file-name "missing-theme" temporary-file-directory)))
+    (should-not (cj/theme-read-file-contents theme-file))))
+
+(ert-deftest test-ui-theme-write-file-contents-writes-content ()
+  "Writing theme content should persist exactly that content."
+  (let ((file (make-temp-file "ui-theme-write-")))
+    (unwind-protect
+        (progn
+          (should (cj/theme-write-file-contents "modus-vivendi" file))
+          (should (equal (cj/theme-read-file-contents file)
+                         "modus-vivendi")))
+      (delete-file file))))
+
+(ert-deftest test-ui-theme-write-file-contents-uses-write-region ()
+  "Theme persistence should write directly instead of visiting the file."
+  (let ((file (make-temp-file "ui-theme-write-region-"))
+        write-region-args
+        write-file-called)
+    (unwind-protect
+        (cl-letf (((symbol-function 'write-region)
+               (lambda (&rest args)
+                 (setq write-region-args args)
+                 nil))
+              ((symbol-function 'write-file)
+               (lambda (&rest _args)
+                 (setq write-file-called t)
+                 (error "write-file should not be used"))))
+          (should (cj/theme-write-file-contents "dupre" file)))
+      (delete-file file))
+    (should (equal (list (car write-region-args)
+                         (cadr write-region-args)
+                         (nth 2 write-region-args))
+                   (list "dupre" nil file)))
+    (should-not write-file-called)))
+
+(ert-deftest test-ui-theme-load-valid-persisted-theme ()
+  "A valid persisted theme should disable current themes and load that theme."
+  (let ((file (make-temp-file "ui-theme-valid-"))
+        (custom-enabled-themes '(old-theme))
+        disabled
+        loaded)
+    (unwind-protect
+        (progn
+          (cj/theme-write-file-contents "modus-vivendi" file)
+          (let ((theme-file file))
+            (cl-letf (((symbol-function 'disable-theme)
+                       (lambda (theme) (push theme disabled)))
+                      ((symbol-function 'load-theme)
+                       (lambda (theme &optional _no-confirm _no-enable)
+                         (push theme loaded))))
+              (cj/load-theme-from-file)))
+          (should (equal disabled '(old-theme)))
+          (should (equal loaded '(modus-vivendi))))
+      (delete-file file))))
+
+(ert-deftest test-ui-theme-load-invalid-theme-falls-back ()
+  "An invalid persisted theme should disable current themes and load fallback."
+  (let ((file (make-temp-file "ui-theme-invalid-"))
+        (fallback-theme-name "modus-vivendi")
+        (custom-enabled-themes '(old-theme))
+        disabled
+        loaded)
+    (unwind-protect
+        (progn
+          (cj/theme-write-file-contents "missing-theme" file)
+          (let ((theme-file file))
+            (cl-letf (((symbol-function 'disable-theme)
+                       (lambda (theme) (push theme disabled)))
+                      ((symbol-function 'load-theme)
+                       (lambda (theme &optional _no-confirm _no-enable)
+                         (push theme loaded)
+                         (when (eq theme 'missing-theme)
+                           (error "missing theme")))))
+              (cj/load-theme-from-file)))
+          (should (equal disabled '(old-theme)))
+          (should (equal loaded '(modus-vivendi missing-theme))))
+      (delete-file file))))
+
+(ert-deftest test-ui-theme-load-missing-file-loads-fallback ()
+  "A missing theme file should disable current themes and load fallback."
+  (let ((theme-file (expand-file-name "missing-theme" temporary-file-directory))
+        (fallback-theme-name "modus-vivendi")
+        (custom-enabled-themes '(old-theme))
+        disabled
+        loaded)
+    (cl-letf (((symbol-function 'disable-theme)
+               (lambda (theme) (push theme disabled)))
+              ((symbol-function 'load-theme)
+               (lambda (theme &optional _no-confirm _no-enable)
+                 (push theme loaded))))
+      (cj/load-theme-from-file))
+    (should (equal disabled '(old-theme)))
+    (should (equal loaded '(modus-vivendi)))))
+
+(ert-deftest test-ui-theme-load-literal-nil-disables-themes ()
+  "A persisted literal nil should disable current themes and load nothing."
+  (let ((file (make-temp-file "ui-theme-nil-"))
+        (custom-enabled-themes '(old-theme newer-theme))
+        disabled
+        loaded)
+    (unwind-protect
+        (progn
+          (cj/theme-write-file-contents "nil" file)
+          (let ((theme-file file))
+            (cl-letf (((symbol-function 'disable-theme)
+                       (lambda (theme) (push theme disabled)))
+                      ((symbol-function 'load-theme)
+                       (lambda (theme &optional _no-confirm _no-enable)
+                         (push theme loaded))))
+              (cj/load-theme-from-file)))
+          (should (equal disabled '(newer-theme old-theme)))
+          (should-not loaded))
+      (delete-file file))))
+
+(provide 'test-ui-theme-persistence)
+;;; test-ui-theme-persistence.el ends here