merge: playlist UI improvements from feat/music-playlist-ui

3 files changed, 454 insertions, 2 deletions

diff --git a/modules/music-config.el b/modules/music-config.el
index 6d7ce132..9542075f 100644
--- a/modules/music-config.el
+++ b/modules/music-config.el
@@ -436,7 +436,7 @@ Intended for use on `emms-player-finished-hook'."
 
 
 (defun cj/music-playlist-toggle ()
-  "Toggle the EMMS playlist buffer in a right side window."
+  "Toggle the EMMS playlist buffer in a bottom side window."
   (interactive)
   (let* ((buf-name cj/music-playlist-buffer-name)
          (buffer (get-buffer buf-name))
@@ -448,7 +448,7 @@ Intended for use on `emms-player-finished-hook'."
       (progn
         (cj/emms--setup)
         (setq buffer (cj/music--ensure-playlist-buffer))
-        (setq win (display-buffer-in-side-window buffer '((side . right) (window-width . 0.35))))
+        (setq win (display-buffer-in-side-window buffer '((side . bottom) (window-height . 0.5))))
         (select-window win)
         (with-current-buffer buffer
           (if (and (fboundp 'emms-playlist-current-selected-track)
@@ -591,6 +591,186 @@ Dirs added recursively."
   (advice-remove 'emms-playlist-clear #'cj/music--after-playlist-clear)
   (advice-add 'emms-playlist-clear :after #'cj/music--after-playlist-clear)
 
+  ;;; Playlist display
+
+  ;; Track description: show "Artist - Title [M:SS]" instead of file paths
+  (defun cj/music--format-duration (seconds)
+    "Convert SECONDS to a \"M:SS\" string."
+    (when (and seconds (numberp seconds) (> seconds 0))
+      (format "%d:%02d" (/ seconds 60) (mod seconds 60))))
+
+  (defun cj/music--track-description (track)
+    "Return a human-readable description of TRACK.
+For tagged tracks: \"Artist - Title [M:SS]\".
+For file tracks without tags: filename without path or extension.
+For URL tracks: decoded URL."
+    (let ((type (emms-track-type track))
+          (title (emms-track-get track 'info-title))
+          (artist (emms-track-get track 'info-artist))
+          (duration (emms-track-get track 'info-playing-time))
+          (name (emms-track-name track)))
+      (cond
+       ;; Tagged track with title
+       (title
+        (let ((dur-str (cj/music--format-duration duration))
+              (parts '()))
+          (when artist (push artist parts))
+          (push title parts)
+          (let ((desc (string-join (nreverse parts) " - ")))
+            (if dur-str (format "%s  [%s]" desc dur-str) desc))))
+       ;; File without tags — show clean filename
+       ((eq type 'file)
+        (file-name-sans-extension (file-name-nondirectory name)))
+       ;; URL — decode percent-encoded characters
+       ((eq type 'url)
+        (decode-coding-string (url-unhex-string name) 'utf-8))
+       ;; Fallback
+       (t (emms-track-simple-description track)))))
+
+  (setq emms-track-description-function #'cj/music--track-description)
+
+  ;; Playlist faces
+  (defface cj/music-header-face
+    '((((class color) (background dark))
+       (:foreground "#969385"))
+      (((class color) (background light))
+       (:foreground "gray50")))
+    "Face for playlist header labels.")
+
+  (defface cj/music-header-value-face
+    '((((class color) (background dark))
+       (:foreground "#d0cbc0"))
+      (((class color) (background light))
+       (:foreground "gray30")))
+    "Face for playlist header values.")
+
+  (defface cj/music-mode-on-face
+    '((((class color) (background dark))
+       (:foreground "#d7af5f"))
+      (((class color) (background light))
+       (:foreground "DarkGoldenrod")))
+    "Face for active mode indicators in the playlist header.")
+
+  (defface cj/music-mode-off-face
+    '((((class color) (background dark))
+       (:foreground "#58574e"))
+      (((class color) (background light))
+       (:foreground "gray70")))
+    "Face for inactive mode indicators in the playlist header.")
+
+  (defface cj/music-keyhint-face
+    '((((class color) (background dark))
+       (:foreground "#8a9496"))
+      (((class color) (background light))
+       (:foreground "gray50")))
+    "Face for keybinding hints in the playlist header.")
+
+  (custom-set-faces
+   '(emms-playlist-track-face
+     ((((class color) (background dark))
+       (:foreground "#8a9496"))
+      (((class color) (background light))
+       (:foreground "gray50"))))
+   '(emms-playlist-selected-face
+     ((((class color) (background dark))
+       (:foreground "#d7af5f" :weight bold))
+      (((class color) (background light))
+       (:foreground "DarkGoldenrod" :weight bold)))))
+
+  ;; Multi-line header overlay
+  (defvar-local cj/music--header-overlay nil
+    "Overlay displaying the playlist header.")
+
+  (defun cj/music--header-text ()
+    "Build a multi-line header string for the playlist buffer overlay."
+    (let* ((pl-name (if cj/music-playlist-file
+                        (file-name-sans-extension
+                         (file-name-nondirectory cj/music-playlist-file))
+                      "Untitled"))
+           (track-count (count-lines (point-min) (point-max)))
+           (now-playing (cond
+                         ((not emms-player-playing-p) "Stopped")
+                         (emms-player-paused-p "Paused")
+                         (t (let ((track (emms-playlist-current-selected-track)))
+                              (if track
+                                  (cj/music--track-description track)
+                                "Playing")))))
+           (mode-indicator
+            (lambda (key label active)
+              (let ((face (if active 'cj/music-mode-on-face 'cj/music-mode-off-face)))
+                (propertize (format "[%s] %s" key label) 'face face)))))
+      (concat
+       (propertize "Playlist" 'face 'cj/music-header-face)
+       (propertize " : " 'face 'cj/music-header-face)
+       (propertize (format "%s (%d)" pl-name track-count) 'face 'cj/music-header-value-face)
+       "\n"
+       (propertize "Current " 'face 'cj/music-header-face)
+       (propertize " : " 'face 'cj/music-header-face)
+       (propertize now-playing 'face 'cj/music-header-value-face)
+       "\n"
+       (propertize "Mode    " 'face 'cj/music-header-face)
+       (propertize " : " 'face 'cj/music-header-face)
+       (funcall mode-indicator "r" "repeat" (bound-and-true-p emms-repeat-playlist))
+       "  "
+       (funcall mode-indicator "t" "single" (bound-and-true-p emms-repeat-track))
+       "  "
+       (funcall mode-indicator "z" "random" (bound-and-true-p emms-random-playlist))
+       "  "
+       (funcall mode-indicator "x" "consume" cj/music-consume-mode)
+       "\n"
+       (propertize "Keys    " 'face 'cj/music-header-face)
+       (propertize " : " 'face 'cj/music-header-face)
+       (propertize "a:add  c:clear  L:load  S:save  SPC:pause  <>:skip  ↑↓:move  C-↑↓:reorder  q:dismiss"
+                   'face 'cj/music-keyhint-face)
+       "\n\n")))
+
+  (defun cj/music--update-header ()
+    "Insert or update the multi-line header overlay in the playlist buffer."
+    (when-let ((buf (get-buffer cj/music-playlist-buffer-name)))
+      (with-current-buffer buf
+        (unless cj/music--header-overlay
+          (setq cj/music--header-overlay (make-overlay (point-min) (point-min)))
+          (overlay-put cj/music--header-overlay 'priority 100))
+        (move-overlay cj/music--header-overlay (point-min) (point-min))
+        (overlay-put cj/music--header-overlay 'before-string
+                     (cj/music--header-text)))))
+
+  (defvar-local cj/music--bg-remap-cookie nil
+    "Cookie for the active-window background face remapping.")
+
+  (defun cj/music--update-active-bg (&rest _)
+    "Toggle playlist buffer background based on whether its window is selected."
+    (when-let ((buf (get-buffer cj/music-playlist-buffer-name)))
+      (with-current-buffer buf
+        (let ((active (eq buf (window-buffer (selected-window)))))
+          (cond
+           ((and active (not cj/music--bg-remap-cookie))
+            (setq cj/music--bg-remap-cookie
+                  (face-remap-add-relative 'default :background "#1d1b19")))
+           ((and (not active) cj/music--bg-remap-cookie)
+            (face-remap-remove-relative cj/music--bg-remap-cookie)
+            (setq cj/music--bg-remap-cookie nil)))))))
+
+  (defun cj/music--setup-playlist-display ()
+    "Set up header overlay and focus tracking in the playlist buffer."
+    (setq header-line-format nil)
+    (cj/music--update-header)
+    (add-hook 'window-selection-change-functions #'cj/music--update-active-bg nil t))
+
+  (add-hook 'emms-playlist-mode-hook #'cj/music--setup-playlist-display)
+  (add-hook 'emms-player-started-hook #'cj/music--update-header)
+  (add-hook 'emms-player-stopped-hook #'cj/music--update-header)
+  (add-hook 'emms-player-paused-hook #'cj/music--update-header)
+  (add-hook 'emms-player-finished-hook #'cj/music--update-header)
+  (add-hook 'emms-playlist-cleared-hook #'cj/music--update-header)
+
+  ;; Refresh header immediately when toggling modes
+  (dolist (fn '(emms-toggle-repeat-playlist
+                emms-toggle-repeat-track
+                emms-toggle-random-playlist
+                cj/music-toggle-consume))
+    (advice-add fn :after (lambda (&rest _) (cj/music--update-header))))
+
   :bind
   (:map emms-playlist-mode-map
         ;; Playback
diff --git a/tests/test-music-config--format-duration.el b/tests/test-music-config--format-duration.el
new file mode 100644
index 00000000..7b107049
--- /dev/null
+++ b/tests/test-music-config--format-duration.el
@@ -0,0 +1,91 @@
+;;; test-music-config--format-duration.el --- Tests for duration formatting -*- coding: utf-8; lexical-binding: t; -*-
+;;
+;; Author: Craig Jennings <c@cjennings.net>
+;;
+;;; Commentary:
+;; Unit tests for cj/music--format-duration function.
+;; Tests the pure helper that converts seconds to "M:SS" display strings.
+;;
+;; Test organization:
+;; - Normal Cases: Typical durations, exact minutes, seconds only
+;; - Boundary Cases: Zero, one second, large values, float input
+;; - Error Cases: Nil, negative, non-numeric input
+;;
+;;; Code:
+
+(require 'ert)
+
+;; Stub missing dependencies before loading music-config
+(defvar-keymap cj/custom-keymap
+  :doc "Stub keymap for testing")
+
+;; Add EMMS elpa directory to load path for batch testing
+(let ((emms-dir (car (file-expand-wildcards
+                      (expand-file-name "elpa/emms-*" user-emacs-directory)))))
+  (when emms-dir
+    (add-to-list 'load-path emms-dir)))
+
+(require 'emms)
+(require 'emms-playlist-mode)
+(require 'music-config)
+
+;;; Normal Cases
+
+(ert-deftest test-music-config--format-duration-normal-typical-song ()
+  "Validate typical 3:45 song duration."
+  (should (string= (cj/music--format-duration 225) "3:45")))
+
+(ert-deftest test-music-config--format-duration-normal-exact-minutes ()
+  "Validate exact minute boundary produces M:00."
+  (should (string= (cj/music--format-duration 180) "3:00")))
+
+(ert-deftest test-music-config--format-duration-normal-seconds-only ()
+  "Validate sub-minute duration produces 0:SS."
+  (should (string= (cj/music--format-duration 45) "0:45")))
+
+(ert-deftest test-music-config--format-duration-normal-single-digit-seconds ()
+  "Validate seconds are zero-padded to two digits."
+  (should (string= (cj/music--format-duration 62) "1:02")))
+
+(ert-deftest test-music-config--format-duration-normal-long-track ()
+  "Validate duration over 10 minutes."
+  (should (string= (cj/music--format-duration 622) "10:22")))
+
+;;; Boundary Cases
+
+(ert-deftest test-music-config--format-duration-boundary-one-second ()
+  "Validate minimum positive duration."
+  (should (string= (cj/music--format-duration 1) "0:01")))
+
+(ert-deftest test-music-config--format-duration-boundary-zero-returns-nil ()
+  "Validate zero seconds returns nil (no useful duration)."
+  (should (null (cj/music--format-duration 0))))
+
+(ert-deftest test-music-config--format-duration-boundary-very-large ()
+  "Validate very long duration (over an hour) formats correctly."
+  (should (string= (cj/music--format-duration 3661) "61:01")))
+
+(ert-deftest test-music-config--format-duration-boundary-float-input ()
+  "Validate float seconds are truncated by integer division."
+  (should (string= (cj/music--format-duration 125.7) "2:05")))
+
+(ert-deftest test-music-config--format-duration-boundary-59-seconds ()
+  "Validate max seconds before minute rollover."
+  (should (string= (cj/music--format-duration 59) "0:59")))
+
+;;; Error Cases
+
+(ert-deftest test-music-config--format-duration-error-nil-returns-nil ()
+  "Validate nil input returns nil."
+  (should (null (cj/music--format-duration nil))))
+
+(ert-deftest test-music-config--format-duration-error-negative-returns-nil ()
+  "Validate negative input returns nil."
+  (should (null (cj/music--format-duration -5))))
+
+(ert-deftest test-music-config--format-duration-error-string-returns-nil ()
+  "Validate non-numeric input returns nil."
+  (should (null (cj/music--format-duration "3:45"))))
+
+(provide 'test-music-config--format-duration)
+;;; test-music-config--format-duration.el ends here
diff --git a/tests/test-music-config--track-description.el b/tests/test-music-config--track-description.el
new file mode 100644
index 00000000..a1a1cc6d
--- /dev/null
+++ b/tests/test-music-config--track-description.el
@@ -0,0 +1,181 @@
+;;; test-music-config--track-description.el --- Tests for track description -*- coding: utf-8; lexical-binding: t; -*-
+;;
+;; Author: Craig Jennings <c@cjennings.net>
+;;
+;;; Commentary:
+;; Unit tests for cj/music--track-description function.
+;; Tests the custom track description that replaces EMMS's default file-path display
+;; with human-readable formats based on track type and available metadata.
+;;
+;; Track construction: EMMS tracks are alists created with `emms-track' and
+;; populated with `emms-track-set'. No playlist buffer or player state needed.
+;;
+;; Test organization:
+;; - Normal Cases: Tagged tracks (artist+title+duration), partial metadata, file fallback, URL
+;; - Boundary Cases: Empty strings, missing fields, special characters, long names
+;; - Error Cases: Unknown track type fallback
+;;
+;;; Code:
+
+(require 'ert)
+
+;; Stub missing dependencies before loading music-config
+(defvar-keymap cj/custom-keymap
+  :doc "Stub keymap for testing")
+
+;; Add EMMS elpa directory to load path for batch testing
+(let ((emms-dir (car (file-expand-wildcards
+                      (expand-file-name "elpa/emms-*" user-emacs-directory)))))
+  (when emms-dir
+    (add-to-list 'load-path emms-dir)))
+
+(require 'emms)
+(require 'emms-playlist-mode)
+(require 'music-config)
+
+;;; Test helpers
+
+(defun test-track-description--make-file-track (path &optional title artist duration)
+  "Create a file TRACK with PATH and optional metadata TITLE, ARTIST, DURATION."
+  (let ((track (emms-track 'file path)))
+    (when title (emms-track-set track 'info-title title))
+    (when artist (emms-track-set track 'info-artist artist))
+    (when duration (emms-track-set track 'info-playing-time duration))
+    track))
+
+(defun test-track-description--make-url-track (url &optional title artist duration)
+  "Create a URL TRACK with URL and optional metadata TITLE, ARTIST, DURATION."
+  (let ((track (emms-track 'url url)))
+    (when title (emms-track-set track 'info-title title))
+    (when artist (emms-track-set track 'info-artist artist))
+    (when duration (emms-track-set track 'info-playing-time duration))
+    track))
+
+;;; Normal Cases — Tagged tracks (artist + title + duration)
+
+(ert-deftest test-music-config--track-description-normal-full-metadata ()
+  "Validate track with artist, title, and duration shows all three."
+  (let ((track (test-track-description--make-file-track
+                "/music/Kind of Blue/01 - So What.flac"
+                "So What" "Miles Davis" 562)))
+    (should (string= (cj/music--track-description track)
+                     "Miles Davis - So What  [9:22]"))))
+
+(ert-deftest test-music-config--track-description-normal-title-and-artist-no-duration ()
+  "Validate track with artist and title but no duration omits bracket."
+  (let ((track (test-track-description--make-file-track
+                "/test/uncached-nodur.mp3" "Blue in Green" "Miles Davis")))
+    (should (string= (cj/music--track-description track)
+                     "Miles Davis - Blue in Green"))))
+
+(ert-deftest test-music-config--track-description-normal-title-only ()
+  "Validate track with title but no artist shows title alone."
+  (let ((track (test-track-description--make-file-track
+                "/test/uncached-noartist.mp3" "Flamenco Sketches" nil 566)))
+    (should (string= (cj/music--track-description track)
+                     "Flamenco Sketches  [9:26]"))))
+
+(ert-deftest test-music-config--track-description-normal-title-only-no-duration ()
+  "Validate track with only title shows just the title."
+  (let ((track (test-track-description--make-file-track
+                "/test/uncached-titleonly.mp3" "All Blues")))
+    (should (string= (cj/music--track-description track)
+                     "All Blues"))))
+
+;;; Normal Cases — File tracks without tags
+
+(ert-deftest test-music-config--track-description-normal-file-no-tags ()
+  "Validate untagged file shows filename without path or extension."
+  (let ((track (test-track-description--make-file-track
+                "/music/Kind of Blue/02 - Freddie Freeloader.flac")))
+    (should (string= (cj/music--track-description track)
+                     "02 - Freddie Freeloader"))))
+
+(ert-deftest test-music-config--track-description-normal-file-nested-path ()
+  "Validate deeply nested path still shows only the filename."
+  (let ((track (test-track-description--make-file-track
+                "/music/Jazz/Miles Davis/Kind of Blue/01 - So What.mp3")))
+    (should (string= (cj/music--track-description track)
+                     "01 - So What"))))
+
+;;; Normal Cases — URL tracks
+
+(ert-deftest test-music-config--track-description-normal-url-plain ()
+  "Validate plain URL is shown as-is."
+  (let ((track (test-track-description--make-url-track
+                "https://radio.example.com/stream")))
+    (should (string= (cj/music--track-description track)
+                     "https://radio.example.com/stream"))))
+
+(ert-deftest test-music-config--track-description-normal-url-percent-encoded ()
+  "Validate percent-encoded URL characters are decoded."
+  (let ((track (test-track-description--make-url-track
+                "https://radio.example.com/my%20station%21")))
+    (should (string= (cj/music--track-description track)
+                     "https://radio.example.com/my station!"))))
+
+(ert-deftest test-music-config--track-description-normal-url-with-tags ()
+  "Validate URL track with tags uses tag display, not URL."
+  (let ((track (test-track-description--make-url-track
+                "https://radio.example.com/stream"
+                "Jazz FM" "Radio Station" 0)))
+    ;; Duration 0 → nil from format-duration, so no bracket
+    (should (string= (cj/music--track-description track)
+                     "Radio Station - Jazz FM"))))
+
+;;; Boundary Cases
+
+(ert-deftest test-music-config--track-description-boundary-empty-title-string ()
+  "Validate empty title string is still truthy, shows empty result."
+  (let ((track (test-track-description--make-file-track
+                "/music/track.mp3" "" "Artist")))
+    ;; Empty string is non-nil, so title branch is taken
+    (should (string= (cj/music--track-description track)
+                     "Artist - "))))
+
+(ert-deftest test-music-config--track-description-boundary-file-no-extension ()
+  "Validate file without extension shows full filename."
+  (let ((track (test-track-description--make-file-track "/music/README")))
+    (should (string= (cj/music--track-description track)
+                     "README"))))
+
+(ert-deftest test-music-config--track-description-boundary-file-multiple-dots ()
+  "Validate file with multiple dots strips only the final extension."
+  (let ((track (test-track-description--make-file-track
+                "/music/disc.1.track.03.flac")))
+    (should (string= (cj/music--track-description track)
+                     "disc.1.track.03"))))
+
+(ert-deftest test-music-config--track-description-boundary-unicode-title ()
+  "Validate unicode characters in metadata are preserved."
+  (let ((track (test-track-description--make-file-track
+                "/music/track.mp3" "夜に駆ける" "YOASOBI" 258)))
+    (should (string= (cj/music--track-description track)
+                     "YOASOBI - 夜に駆ける  [4:18]"))))
+
+(ert-deftest test-music-config--track-description-boundary-url-utf8-percent-encoded ()
+  "Validate percent-encoded UTF-8 in URL is decoded correctly."
+  (let ((track (test-track-description--make-url-track
+                "https://example.com/caf%C3%A9")))
+    (should (string= (cj/music--track-description track)
+                     "https://example.com/café"))))
+
+(ert-deftest test-music-config--track-description-boundary-short-duration ()
+  "Validate 1-second track formats correctly in bracket."
+  (let ((track (test-track-description--make-file-track
+                "/music/t.mp3" "Beep" nil 1)))
+    (should (string= (cj/music--track-description track)
+                     "Beep  [0:01]"))))
+
+;;; Error Cases
+
+(ert-deftest test-music-config--track-description-error-unknown-type-fallback ()
+  "Validate unknown track type uses emms-track-simple-description fallback."
+  (let ((track (emms-track 'streamlist "https://example.com/playlist.m3u")))
+    ;; Should not error; falls through to simple-description
+    (let ((result (cj/music--track-description track)))
+      (should (stringp result))
+      (should (string-match-p "example\\.com" result)))))
+
+(provide 'test-music-config--track-description)
+;;; test-music-config--track-description.el ends here