feat(music): playing-aware playlist landing and music-only adds

Opening the playlist used to jump to EMMS's selected track, which stays set while stopped, so the view opened deep in the list with point mid-row. It now lands on the playing track's row (beginning of line, upper third of the window) when a song is playing, and at the top of the list when stopped. Both entry points share the logic, and the playlist buffer gets hl-line so the current row stays findable on album art.

Also fixed: directory adds handed the raw tree to emms-add-directory-tree, which adds every file it finds. Cover art and liner notes became playlist rows. All three add paths now route through a filtered walk that keeps only accepted music extensions. The m3u loader applies the same filter to local paths (stream URLs pass through), so old playlists with saved cover lines stop re-importing them.

6 files changed, 345 insertions, 30 deletions

diff --git a/modules/music-config.el b/modules/music-config.el
index b57d910a..9174bea3 100644
--- a/modules/music-config.el
+++ b/modules/music-config.el
@@ -322,6 +322,38 @@ modification date so marginalia can show them."
             (completion-ignore-case . t))
         (complete-with-action action candidates string pred)))))
 
+(defun cj/music--playlist-open-position (buffer)
+  "Return where point should land when the playlist BUFFER is displayed.
+The beginning of the playing track's line when a song is playing (during
+playback the selected track is the playing one), else the top of the
+list.  Keying off the selected track alone is wrong: EMMS keeps a stale
+selection while stopped, which used to open the playlist deep in the list
+at whatever played last."
+  (with-current-buffer buffer
+    (if (and (boundp 'emms-player-playing-p) emms-player-playing-p
+             (boundp 'emms-playlist-selected-marker)
+             (markerp emms-playlist-selected-marker)
+             (marker-position emms-playlist-selected-marker)
+             (eq (marker-buffer emms-playlist-selected-marker) (current-buffer)))
+        (save-excursion
+          (goto-char emms-playlist-selected-marker)
+          (line-beginning-position))
+      (point-min))))
+
+(defun cj/music--playlist-land-point (win buffer)
+  "Move WIN's point in BUFFER per the open-position rule and settle the view.
+When a song is playing its row lands in the window's upper third, so the
+upcoming tracks fill the space below it.  When stopped, the view starts at
+the top of the list.  Point sits at the beginning of its line either way,
+so the row reads left-to-right from its number."
+  (let ((pos (cj/music--playlist-open-position buffer)))
+    (set-window-point win pos)
+    (if (> pos (with-current-buffer buffer (point-min)))
+        (with-selected-window win
+          (with-current-buffer buffer
+            (recenter (max 1 (/ (window-body-height) 3)))))
+      (set-window-start win pos))))
+
 (defvar-local cj/music--renumber-timer nil
   "Pending idle timer for the playlist row renumber, or nil.")
 
@@ -372,14 +404,21 @@ inserts or kills into one renumber pass."
         (emms-playlist-mode))
       (setq emms-playlist-buffer-p t)
       ;; Row numbering: renumber after every playlist change, debounced.
-      (add-hook 'after-change-functions #'cj/music--schedule-renumber nil t))
+      (add-hook 'after-change-functions #'cj/music--schedule-renumber nil t)
+      ;; The highlighted row stays findable even when the cursor sits on
+      ;; album art (pairs with the row-number prefixes).
+      (hl-line-mode 1))
     (cj/music--renumber-rows buffer)
     ;; Set this as the current EMMS playlist buffer
     (setq emms-playlist-buffer buffer)
     buffer))
 
 (defun cj/music--m3u-file-tracks (m3u-file)
-  "Return list of absolute track paths from M3U-FILE. Ignore # comment lines."
+  "Return list of absolute track paths from M3U-FILE. Ignore # comment lines.
+Stream URLs pass through untouched; a local path must carry an accepted
+music extension (`cj/music--valid-file-p') -- old playlists saved before
+directory adds were filtered can carry cover.jpg lines, and loading one
+would put the cover right back in the playlist."
   (when (and m3u-file (file-exists-p m3u-file))
     (with-temp-buffer
       (insert-file-contents m3u-file)
@@ -389,11 +428,12 @@ inserts or kills into one renumber pass."
         (while (re-search-forward "^[^#].*$" nil t)
           (let ((line (string-trim (match-string 0))))
             (unless (string-empty-p line)
-              (push (if (or (file-name-absolute-p line)
-                            (string-match-p "\\`\\(https?\\|mms\\)://" line))
-                        line
-                      (expand-file-name line dir))
-                    tracks))))
+              (let* ((url-p (string-match-p "\\`\\(https?\\|mms\\)://" line))
+                     (path (cond (url-p line)
+                                 ((file-name-absolute-p line) line)
+                                 (t (expand-file-name line dir)))))
+                (when (or url-p (cj/music--valid-file-p path))
+                  (push path tracks))))))
         (nreverse tracks)))))
 
 (defun cj/music--playlist-track-objects ()
@@ -482,15 +522,34 @@ Returns the full path to the selected file, or nil if cancelled."
 
 ;;; Commands: add/select
 
+(defun cj/music--music-files-recursive (directory)
+  "Return sorted absolute paths of the music files under DIRECTORY.
+Only files passing `cj/music--valid-file-p' (the accepted extensions in
+`cj/music-file-extensions') come back; hidden files and hidden
+directories are skipped.  This is the filter the directory-add commands
+route through -- handing the raw tree to EMMS added every file it found,
+so cover art and liner notes ended up as playlist rows."
+  (sort (seq-filter #'cj/music--valid-file-p
+                    (directory-files-recursively
+                     directory "\\`[^.]" nil
+                     (lambda (dir)
+                       (not (string-prefix-p "." (file-name-nondirectory dir))))))
+        #'string-lessp))
+
 (defun cj/music-add-directory-recursive (directory)
-  "Add all music files under DIRECTORY recursively to the EMMS playlist."
+  "Add all music files under DIRECTORY recursively to the EMMS playlist.
+Only files with accepted music extensions are added; cover art and other
+non-music files in album directories stay out."
   (interactive
    (list (read-directory-name "Add directory recursively: " cj/music-root nil t)))
   (unless (file-directory-p directory)
     (user-error "Not a directory: %s" directory))
   (cj/music--ensure-playlist-buffer)
-  (emms-add-directory-tree directory)
-  (message "Added recursively: %s" directory))
+  (let ((files (cj/music--music-files-recursive directory)))
+    (dolist (f files)
+      (emms-add-file f))
+    (message "Added %d music file%s from %s"
+             (length files) (if (= (length files) 1) "" "s") directory)))
 
 
 (defun cj/music-fuzzy-select-and-add ()
@@ -856,11 +915,7 @@ resized and toggled off this session, it reopens at that remembered height."
                    buffer 'bottom 'cj/--music-playlist-height
                    cj/music-playlist-window-height))
         (select-window win)
-        (with-current-buffer buffer
-          (if (and (fboundp 'emms-playlist-current-selected-track)
-                   (emms-playlist-current-selected-track))
-              (emms-playlist-mode-center-current)
-            (goto-char (point-min))))
+        (cj/music--playlist-land-point win buffer)
         (let ((count (with-current-buffer buffer
                        (count-lines (point-min) (point-max)))))
           (message (if (> count 0)
@@ -878,7 +933,9 @@ Initializes EMMS if needed."
     (when buffer-exists
       (with-current-buffer cj/music-playlist-buffer-name
         (setq has-content (> (point-max) (point-min)))))
-    (switch-to-buffer (cj/music--ensure-playlist-buffer))
+    (let ((buffer (cj/music--ensure-playlist-buffer)))
+      (switch-to-buffer buffer)
+      (cj/music--playlist-land-point (selected-window) buffer))
     (cond
      ((not emms-was-loaded) (message "EMMS started. Current playlist empty"))
      ((and buffer-exists has-content) (message "EMMS running. Displaying current playlist"))
diff --git a/tests/test-music-config--m3u-file-tracks.el b/tests/test-music-config--m3u-file-tracks.el
index badc9817..e3cbd72e 100644
--- a/tests/test-music-config--m3u-file-tracks.el
+++ b/tests/test-music-config--m3u-file-tracks.el
@@ -189,5 +189,23 @@
   "Parse nil input returns nil gracefully."
   (should (null (cj/music--m3u-file-tracks nil))))
 
+;;; Non-music filtering
+
+(ert-deftest test-music-config--m3u-file-tracks-filters-non-music-local-files ()
+  "Normal: a local non-music path (a saved cover.jpg line) is dropped;
+music files and stream URLs pass through."
+  (test-music-config--m3u-file-tracks-setup)
+  (unwind-protect
+      (let* ((content (concat "/home/user/music/track1.mp3\n"
+                              "/home/user/music/album/cover.jpg\n"
+                              "https://somafm.com/stream\n"
+                              "/home/user/music/track2.flac\n"))
+             (m3u-file (cj/create-temp-test-file-with-content content "test.m3u"))
+             (tracks (cj/music--m3u-file-tracks m3u-file)))
+        (should (equal tracks '("/home/user/music/track1.mp3"
+                                "https://somafm.com/stream"
+                                "/home/user/music/track2.flac"))))
+    (test-music-config--m3u-file-tracks-teardown)))
+
 (provide 'test-music-config--m3u-file-tracks)
 ;;; test-music-config--m3u-file-tracks.el ends here
diff --git a/tests/test-music-config--music-files-recursive.el b/tests/test-music-config--music-files-recursive.el
new file mode 100644
index 00000000..f5f5fc5b
--- /dev/null
+++ b/tests/test-music-config--music-files-recursive.el
@@ -0,0 +1,88 @@
+;;; test-music-config--music-files-recursive.el --- Tests for filtered directory collection -*- coding: utf-8; lexical-binding: t; -*-
+;;
+;; Author: Craig Jennings <c@cjennings.net>
+;;
+;;; Commentary:
+;; Directory adds used to hand the whole tree to emms-add-directory-tree,
+;; which adds every file it finds -- cover.jpg and friends ended up as
+;; playlist rows.  The filtered walk returns only files passing
+;; cj/music--valid-file-p, skipping hidden dirs/files, sorted.  Real temp-dir
+;; fixtures; the EMMS boundary is mocked only in the command-level test.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+
+(defvar cj/custom-keymap (make-sparse-keymap)
+  "Stub keymap for testing.")
+
+(require 'music-config)
+
+(defmacro test-music-files--with-fixture (var &rest body)
+  "Run BODY with VAR bound to a temp music-directory fixture."
+  (declare (indent 1))
+  `(let ((,var (make-temp-file "music-test-" t)))
+     (unwind-protect
+         (progn
+           (make-directory (expand-file-name "album" ,var))
+           (make-directory (expand-file-name ".hidden" ,var))
+           (dolist (f '("song.mp3" "cover.jpg" "album/track.flac"
+                        "album/folder.png" "album/notes.txt"
+                        ".hidden/secret.mp3" ".stray.ogg"))
+             (write-region "" nil (expand-file-name f ,var)))
+           ,@body)
+       (delete-directory ,var t))))
+
+;;; Normal Cases
+
+(ert-deftest test-music-files-recursive-music-only ()
+  "Normal: only files with accepted music extensions come back, sorted;
+cover art, text files, and hidden entries stay out."
+  (test-music-files--with-fixture root
+    (should (equal (mapcar (lambda (f) (file-relative-name f root))
+                           (cj/music--music-files-recursive root))
+                   '("album/track.flac" "song.mp3")))))
+
+(ert-deftest test-music-add-directory-recursive-adds-only-music ()
+  "Normal: the directory-add command feeds only music files to EMMS."
+  (test-music-files--with-fixture root
+    (let (added)
+      (cl-letf (((symbol-function 'cj/music--ensure-playlist-buffer)
+                 (lambda () (current-buffer)))
+                ((symbol-function 'emms-add-file)
+                 (lambda (f) (push f added))))
+        (cj/music-add-directory-recursive root))
+      (should (equal (mapcar (lambda (f) (file-relative-name f root))
+                             (nreverse added))
+                     '("album/track.flac" "song.mp3"))))))
+
+;;; Boundary Cases
+
+(ert-deftest test-music-files-recursive-case-insensitive-extensions ()
+  "Boundary: extensions match case-insensitively (Song.MP3 counts)."
+  (let ((root (make-temp-file "music-test-case-" t)))
+    (unwind-protect
+        (progn
+          (write-region "" nil (expand-file-name "Song.MP3" root))
+          (should (= 1 (length (cj/music--music-files-recursive root)))))
+      (delete-directory root t))))
+
+(ert-deftest test-music-files-recursive-empty-directory ()
+  "Boundary: a directory with no music files returns nil."
+  (let ((root (make-temp-file "music-test-empty-" t)))
+    (unwind-protect
+        (progn
+          (write-region "" nil (expand-file-name "readme.txt" root))
+          (should-not (cj/music--music-files-recursive root)))
+      (delete-directory root t))))
+
+;;; Error Cases
+
+(ert-deftest test-music-add-directory-recursive-not-a-directory-errors ()
+  "Error: a non-directory argument signals user-error."
+  (should-error (cj/music-add-directory-recursive "/nonexistent/nowhere")
+                :type 'user-error))
+
+(provide 'test-music-config--music-files-recursive)
+;;; test-music-config--music-files-recursive.el ends here
diff --git a/tests/test-music-config--playlist-open-position.el b/tests/test-music-config--playlist-open-position.el
new file mode 100644
index 00000000..cd83e65b
--- /dev/null
+++ b/tests/test-music-config--playlist-open-position.el
@@ -0,0 +1,147 @@
+;;; test-music-config--playlist-open-position.el --- Tests for playlist landing position -*- coding: utf-8; lexical-binding: t; -*-
+;;
+;; Author: Craig Jennings <c@cjennings.net>
+;;
+;;; Commentary:
+;; Opening the playlist lands point by one rule: the beginning of the playing
+;; track's line when a song is playing, else the top of the list.  The old
+;; behavior keyed off EMMS's selected track, which stays set while stopped, so
+;; the playlist opened deep in the list at a stale position.  The decision is
+;; a pure helper; the window landing (point + upper-third recenter) is tested
+;; with recenter stubbed at the display boundary.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+
+(defvar cj/custom-keymap (make-sparse-keymap)
+  "Stub keymap for testing.")
+
+;; Declare the emms vars special HERE too: the module's bare (defvar
+;; emms-player-playing-p) marks them special only for code compiled in
+;; that file, so a plain `let' in this lexical-binding test file would
+;; bind them lexically and the module would never see the value (the
+;; scope-shadowing trap from the testing rules).
+(defvar emms-player-playing-p)
+(defvar emms-playlist-selected-marker)
+
+(require 'music-config)
+
+(defmacro test-music-open-pos--with-buffer (var &rest body)
+  "Run BODY with VAR bound to a temp 3-track playlist-shaped buffer."
+  (declare (indent 1))
+  `(let ((,var (generate-new-buffer " *test-open-pos*")))
+     (unwind-protect
+         (progn
+           (with-current-buffer ,var
+             (insert "track one\ntrack two\ntrack three\n"))
+           ,@body)
+       (when (buffer-live-p ,var) (kill-buffer ,var)))))
+
+(defun test-music-open-pos--marker (buffer line offset)
+  "Marker in BUFFER at LINE (1-based) plus OFFSET chars."
+  (with-current-buffer buffer
+    (save-excursion
+      (goto-char (point-min))
+      (forward-line (1- line))
+      (forward-char offset)
+      (point-marker))))
+
+;;; Normal Cases
+
+(ert-deftest test-music-playlist-open-position-playing-lands-on-playing-line-start ()
+  "Normal: playing -> the playing track's line, at its beginning (even when
+the marker sits mid-line)."
+  (test-music-open-pos--with-buffer buf
+    (let ((emms-player-playing-p t)
+          (emms-playlist-selected-marker (test-music-open-pos--marker buf 2 4)))
+      (should (= (cj/music--playlist-open-position buf)
+                 (with-current-buffer buf
+                   (save-excursion (goto-char (point-min)) (forward-line 1) (point))))))))
+
+(ert-deftest test-music-playlist-open-position-stopped-lands-at-top ()
+  "Normal: not playing -> top of the list, even though EMMS still has a
+stale selected track."
+  (test-music-open-pos--with-buffer buf
+    (let ((emms-player-playing-p nil)
+          (emms-playlist-selected-marker (test-music-open-pos--marker buf 3 0)))
+      (should (= (cj/music--playlist-open-position buf) 1)))))
+
+;;; Boundary Cases
+
+(ert-deftest test-music-playlist-open-position-playing-no-marker-lands-at-top ()
+  "Boundary: playing but no usable marker -> top of the list."
+  (test-music-open-pos--with-buffer buf
+    (let ((emms-player-playing-p t)
+          (emms-playlist-selected-marker nil))
+      (should (= (cj/music--playlist-open-position buf) 1)))))
+
+(ert-deftest test-music-playlist-open-position-marker-in-other-buffer-lands-at-top ()
+  "Boundary: a marker pointing into a different buffer is ignored."
+  (test-music-open-pos--with-buffer buf
+    (with-temp-buffer
+      (insert "elsewhere\n")
+      (let ((emms-player-playing-p t)
+            (emms-playlist-selected-marker (point-marker)))
+        (should (= (cj/music--playlist-open-position buf) 1))))))
+
+(ert-deftest test-music-playlist-open-position-empty-buffer ()
+  "Boundary: an empty playlist lands at point-min without error."
+  (let ((buf (generate-new-buffer " *test-open-pos-empty*")))
+    (unwind-protect
+        (let ((emms-player-playing-p nil)
+              (emms-playlist-selected-marker nil))
+          (should (= (cj/music--playlist-open-position buf) 1)))
+      (kill-buffer buf))))
+
+;;; Landing (window boundary stubbed)
+
+(ert-deftest test-music-playlist-land-point-playing-recenter-upper-third ()
+  "Normal: landing on a playing row sets window point to its line start and
+recenters into the upper third."
+  (test-music-open-pos--with-buffer buf
+    (let ((emms-player-playing-p t)
+          (emms-playlist-selected-marker (test-music-open-pos--marker buf 2 4))
+          (recenter-arg 'not-called))
+      (save-window-excursion
+        (set-window-buffer (selected-window) buf)
+        (cl-letf (((symbol-function 'recenter)
+                   (lambda (&optional arg &rest _) (setq recenter-arg arg))))
+          (cj/music--playlist-land-point (selected-window) buf))
+        (should (= (window-point (selected-window))
+                   (with-current-buffer buf
+                     (save-excursion (goto-char (point-min)) (forward-line 1) (point)))))
+        (should (integerp recenter-arg))
+        (should (>= recenter-arg 1))))))
+
+(ert-deftest test-music-playlist-land-point-stopped-top-no-recenter ()
+  "Normal: landing while stopped puts window point at the top; no recenter."
+  (test-music-open-pos--with-buffer buf
+    (let ((emms-player-playing-p nil)
+          (emms-playlist-selected-marker nil)
+          (recenter-called nil))
+      (save-window-excursion
+        (set-window-buffer (selected-window) buf)
+        (cl-letf (((symbol-function 'recenter)
+                   (lambda (&rest _) (setq recenter-called t))))
+          (cj/music--playlist-land-point (selected-window) buf))
+        (should (= (window-point (selected-window)) 1))
+        (should-not recenter-called)))))
+
+;;; hl-line in the playlist buffer
+
+(ert-deftest test-music-playlist-ensure-enables-hl-line ()
+  "Normal: the playlist buffer gets hl-line-mode so the current row is
+findable even when the cursor sits on album art."
+  (let (created)
+    (cl-letf (((symbol-function 'emms-playlist-mode) #'ignore))
+      (unwind-protect
+          (progn
+            (setq created (cj/music--ensure-playlist-buffer))
+            (with-current-buffer created
+              (should hl-line-mode)))
+        (when (buffer-live-p created) (kill-buffer created))))))
+
+(provide 'test-music-config--playlist-open-position)
+;;; test-music-config--playlist-open-position.el ends here
diff --git a/tests/test-music-config-commands.el b/tests/test-music-config-commands.el
index 3c585d0b..40fb2d81 100644
--- a/tests/test-music-config-commands.el
+++ b/tests/test-music-config-commands.el
@@ -30,17 +30,21 @@
 ;;; cj/music-add-directory-recursive
 
 (ert-deftest test-music-add-directory-recursive-passes-dir-to-emms ()
-  "Normal: add-directory-recursive routes through emms-add-directory-tree."
+  "Normal: add-directory-recursive feeds the directory's music files (and
+only those) to emms-add-file -- the raw-tree path added cover art too."
   (let* ((tmp (file-name-as-directory (make-temp-file "cj-music-add-" t)))
-         called)
+         added)
     (unwind-protect
-        (cl-letf (((symbol-function 'cj/music--ensure-playlist-buffer) #'ignore)
-                  ((symbol-function 'emms-add-directory-tree)
-                   (lambda (dir) (setq called dir)))
-                  ((symbol-function 'message) #'ignore))
-          (cj/music-add-directory-recursive tmp))
+        (progn
+          (write-region "" nil (expand-file-name "song.mp3" tmp))
+          (write-region "" nil (expand-file-name "cover.jpg" tmp))
+          (cl-letf (((symbol-function 'cj/music--ensure-playlist-buffer) #'ignore)
+                    ((symbol-function 'emms-add-file)
+                     (lambda (f) (push f added)))
+                    ((symbol-function 'message) #'ignore))
+            (cj/music-add-directory-recursive tmp)))
       (delete-directory tmp t))
-    (should (equal called tmp))))
+    (should (equal (mapcar #'file-name-nondirectory added) '("song.mp3")))))
 
 (ert-deftest test-music-add-directory-recursive-errors-on-non-directory ()
   "Error: passing a regular file (not a directory) signals user-error."
diff --git a/tests/test-music-config-helpers-untested.el b/tests/test-music-config-helpers-untested.el
index bfdb2634..87aa210c 100644
--- a/tests/test-music-config-helpers-untested.el
+++ b/tests/test-music-config-helpers-untested.el
@@ -154,17 +154,18 @@ test prelude inserts filler with `inhibit-read-only' bound."
 ;;; ---------- cj/music-add-directory-recursive ----------
 
 (ert-deftest test-mc-add-directory-recursive-normal-calls-emms ()
-  "Normal: with an existing directory, the recursive add reaches emms."
+  "Normal: with an existing directory, the recursive add reaches emms with
+each music file individually (the filtered walk, not the raw tree)."
   (test-mc-untested--setup)
   (unwind-protect
       (let* ((dir cj/test-base-dir)
-             (called-with nil))
-        (cl-letf (((symbol-function 'emms-add-directory-tree)
-                   (lambda (d) (setq called-with d)))
+             (added nil))
+        (write-region "" nil (expand-file-name "one.mp3" dir))
+        (cl-letf (((symbol-function 'emms-add-file)
+                   (lambda (f) (push f added)))
                   ((symbol-function 'message) #'ignore))
           (cj/music-add-directory-recursive dir))
-        (should (equal (file-name-as-directory called-with)
-                       (file-name-as-directory dir))))
+        (should (member "one.mp3" (mapcar #'file-name-nondirectory added))))
     (test-mc-untested--teardown)))
 
 (ert-deftest test-mc-add-directory-recursive-error-not-a-directory ()