Clean: emms-track-file-p replaces eq file type checks

There were about 10 checks throughout the code checking if a track is a
file type. File type tracks are specific enough for a predicate function.

Add: EMMS warning if no tagging program for audio file type

Warn the user when there is no tag writing program available to EMMS
for modifying the actual audio files. Current behavior allows the tags
to be edited, saved to the EMMS cache db, and then silently skips past
the part where the metadata would be applied to the file itself.

The changes here prevent the user from even opening the editor for a
file that has no tag writing program configured. This can be
overridden with the prefix arg when user wants to edit the tags
anyway, knowing they will only be saved in the emms-cache-db and not
written to the source audio files.

Patch by Grant Shangreaux

13 files changed, 60 insertions, 32 deletions

diff --git a/AUTHORS b/AUTHORS
index 04828cd..71e540c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -12,6 +12,7 @@ Daniel Brockman		 <daniel@brockman.se>
 Daniel Dehennin          <daniel.dehennin@baby-gnu.org>
 David Engster		 <deng@randomsample.de>
 Feng Shu                 <tumashu@163.com>
+Grant Shangreaux	 <grant@churls.world>
 Jean-Philippe Theberge	 <jphiltheberge@videotron.ca>
 Jorgen Schaefer		 <forcer@forcix.cx>
 Jose Antonio Ortega Ruiz <jao@gnu.org>
diff --git a/doc/emms.texinfo b/doc/emms.texinfo
index 77c27b2..d0e4570 100644
--- a/doc/emms.texinfo
+++ b/doc/emms.texinfo
@@ -1669,7 +1669,7 @@ from @file{emms-player-simple.el}
 @lisp
 (defun emms-mpg321-remote-playablep (track)
        "Return non-nil when we can play this track."
-       (and (eq 'file (emms-track-type track))
+       (and (emms-track-file-p track)
 @end lisp
 @noindent
 
diff --git a/emms-cache.el b/emms-cache.el
index 3e0d056..35b851c 100644
--- a/emms-cache.el
+++ b/emms-cache.el
@@ -163,7 +163,7 @@ regardless of whether they have been modified or not."
   (interactive "P")
   (let (removed)
     (maphash (lambda (path track)
-               (when (eq (emms-track-get track 'type) 'file)
+               (when (emms-track-file-p track)
                  ;; if no longer here, remove
                  (if (not (file-exists-p path))
                      (progn
diff --git a/emms-cue.el b/emms-cue.el
index 3828ee6..880d2d8 100644
--- a/emms-cue.el
+++ b/emms-cue.el
@@ -93,7 +93,7 @@ When PREVIOUS-P is t, get previous track info instead."
 (defun emms-info-cueinfo (track)
   "Add track information to TRACK.
 This is a useful element for `emms-info-functions'."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (string-match "\\.\\(ape\\|flac\\)\\'" (emms-track-name track)))
     (let ((cue (concat (file-name-sans-extension (emms-track-name track))
                        ".cue")))
diff --git a/emms-info-libtag.el b/emms-info-libtag.el
index 4269aba..fb9c5dd 100644
--- a/emms-info-libtag.el
+++ b/emms-info-libtag.el
@@ -81,7 +81,7 @@ Case is irrelevant."
   :type '(string))
 
 (defun emms-info-libtag (track)
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (let ((case-fold-search t))
                (string-match
                 emms-info-libtag-known-extensions
diff --git a/emms-info-metaflac.el b/emms-info-metaflac.el
index 79131cb..f03925f 100644
--- a/emms-info-metaflac.el
+++ b/emms-info-metaflac.el
@@ -70,7 +70,7 @@ external metaflac program"
 (defun emms-info-metaflac (track)
   "Get the FLAC tag of file TRACK, using `emms-info-metaflac-program'
 and return an emms-info structure representing it."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (string-match "\\.\\(flac\\|FLAC\\)\\'" (emms-track-name track)))
     (with-temp-buffer
       (when (zerop
diff --git a/emms-info-mp3info.el b/emms-info-mp3info.el
index 06624d5..5ebc964 100644
--- a/emms-info-mp3info.el
+++ b/emms-info-mp3info.el
@@ -72,7 +72,7 @@ This should be a list of info-flag=value lines."
 (defun emms-info-mp3info (track)
   "Add track information to TRACK.
 This is a useful element for `emms-info-functions'."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (string-match "\\.[Mm][Pp]3\\'" (emms-track-name track)))
     (with-temp-buffer
       (when (zerop
diff --git a/emms-info-ogginfo.el b/emms-info-ogginfo.el
index fbe64ec..68950ff 100644
--- a/emms-info-ogginfo.el
+++ b/emms-info-ogginfo.el
@@ -44,7 +44,7 @@ program"
 (defun emms-info-ogginfo (track)
   "Add track information to TRACK.
 This is a useful element for `emms-info-functions'."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (string-match "\\.[Oo][Gg][Gg]\\'" (emms-track-name track)))
 
     (with-temp-buffer
diff --git a/emms-info-opusinfo.el b/emms-info-opusinfo.el
index af1a9b8..7f28387 100644
--- a/emms-info-opusinfo.el
+++ b/emms-info-opusinfo.el
@@ -43,7 +43,7 @@ program"
 (defun emms-info-opusinfo (track)
   "Add track information to TRACK.
 This is a useful element for `emms-info-functions'."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
              (or (string-match "\\.[Oo][Gg][Gg]\\'" (emms-track-name track))
                  (string-match "\\.[Oo][Pp][Uu][Ss]\\'" (emms-track-name track))))
 
diff --git a/emms-info-tinytag.el b/emms-info-tinytag.el
index 32895f0..28a72de 100644
--- a/emms-info-tinytag.el
+++ b/emms-info-tinytag.el
@@ -80,7 +80,7 @@ Case is irrelevant."
 
 (defun emms-info-tinytag (track)
   "Set tags for TRACK using tinytag."
-  (when (and (eq 'file (emms-track-type track))
+  (when (and (emms-track-file-p track)
 	     (let ((case-fold-search t))
 	       (string-match
 		emms-info-tinytag-known-extensions
diff --git a/emms-player-mpd.el b/emms-player-mpd.el
index 39f23d5..e7a61a0 100644
--- a/emms-player-mpd.el
+++ b/emms-player-mpd.el
@@ -1158,7 +1158,7 @@ info from MusicPD.
 This is a useful addition to `emms-info-functions'."
   (if info
       (emms-info-mpd-process track info)
-    (when (and (eq 'file (emms-track-type track))
+    (when (and (emms-track-file-p track)
                (not (string-match "\\`http://" (emms-track-name track))))
       (let ((file (emms-player-mpd-get-mpd-filename (emms-track-name track))))
         (when (or emms-player-mpd-music-directory
diff --git a/emms-tag-editor.el b/emms-tag-editor.el
index c00a4fb..5b6e447 100644
--- a/emms-tag-editor.el
+++ b/emms-tag-editor.el
@@ -271,27 +271,52 @@ This string is suitable for inserting into the tags buffer."
     (goto-char (point-min))
     (emms-tag-editor-display-log-buffer-maybe)))
 
-(defun emms-tag-editor-edit-track (track)
-  "Edit the track at point, or TRACK."
+(defun emms-tag-editor--tagfile-function (track)
+  "Return value of `emms-tag-editor-tagfile-functions' for TRACK, or nil."
+  (assoc (file-name-extension (emms-track-get track 'name))
+         emms-tag-editor-tagfile-functions))
+
+(defun emms-tag-editor--track-editable-p (track)
+  "Return t if TRACK is not a file, or has a tagfile function defined."
+  (or (not (emms-track-file-p track))
+      (emms-tag-editor--tagfile-function track)))
+
+(defun emms-tag-editor-edit-track (track &optional edit-anyway)
+  "Edit the track at point, or TRACK.
+If EDIT-ANYWAY is true or TRACK is not a file type, it will be loaded
+in the tag editor. Otherwise, if EMMS does not have a program configured
+to actually write tags to the audio file, do not open the tag data in
+the editor."
   (interactive (list (emms-tag-editor-track-at)))
-  (if (null track)
-      (message "No track at point!")
-    (emms-tag-editor-insert-tracks (list track))))
-
-(defun emms-tag-editor-edit-marked-tracks ()
-  "Edit all tracks marked in the current buffer."
+  (cond
+   ((null track) (message "No track at point!"))
+   ((or (emms-tag-editor--track-editable-p track) edit-anyway)
+    (emms-tag-editor-insert-tracks (list track)))
+   (t (message "EMMS has no tag writing program configured for this file type!"))))
+
+(defun emms-tag-editor-edit-marked-tracks (&optional edit-anyway)
+  "Edit all tracks marked in the current buffer.
+If EDIT-ANYWAY is nil, filter out any file tracks that do not have a
+tagfile function defined."
   (interactive)
-  (let ((tracks (emms-mark-mapcar-marked-track 'emms-tag-editor-track-at t)))
+  (let* ((tracks (emms-mark-mapcar-marked-track 'emms-tag-editor-track-at t))
+         (funcs (mapcar #'emms-tag-editor--tagfile-function tracks)))
+    (when (seq-some #'null funcs)
+      (unless edit-anyway
+        (setq tracks (seq-filter #'emms-tag-editor--track-editable-p tracks))
+        (message "Skipped file tracks without a tag writing program configured.")))
     (if (null tracks)
-        (message "No track marked!")
+        (message "No writable track marked!")
       (emms-tag-editor-insert-tracks tracks))))
 
-(defun emms-tag-editor-edit ()
-  "Edit tags of either the track at point or all marked tracks."
-  (interactive)
+(defun emms-tag-editor-edit (&optional arg)
+  "Edit tags of either the track at point or all marked tracks.
+With a prefix argument, edits tags even if there is no external
+program for writing tags to the specified track or tracks."
+  (interactive "P")
   (if (emms-mark-has-markedp)
-      (emms-tag-editor-edit-marked-tracks)
-    (emms-tag-editor-edit-track (emms-tag-editor-track-at))))
+      (emms-tag-editor-edit-marked-tracks arg)
+    (emms-tag-editor-edit-track (emms-tag-editor-track-at) arg)))
 
 (defvar emms-tag-editor-mode-map
   (let ((map (make-sparse-keymap)))
@@ -617,7 +642,7 @@ With prefix argument, bury the tag edit buffer."
                 old (emms-track-get track 'orig-track))
           ;; rename local file
           (when (and (emms-track-get track 'newname)
-                     (eq (emms-track-get track 'type) 'file)
+                     (emms-track-file-p track)
                      (file-writable-p (emms-track-name track))
                      (y-or-n-p (format "Rename %s to %s? "
                                        (emms-track-name track)
@@ -646,10 +671,9 @@ With prefix argument, bury the tag edit buffer."
             (when (setq val (emms-track-get track (car tag)))
             (emms-track-set old (car tag) val)))
           ;; use external program to change tags in the file
-          (when (and (eq (emms-track-get track 'type) 'file)
+          (when (and (emms-track-file-p track)
                      (file-writable-p (emms-track-name track))
-                     (setq func (assoc (file-name-extension filename)
-                                       emms-tag-editor-tagfile-functions)))
+                     (setq func (emms-tag-editor--tagfile-function track)))
             (setq exit
                   (if (functionp (cdr func))
                       (funcall (cdr func) track)
@@ -755,7 +779,7 @@ value.
 If DONT-APPLY is non-nil the changes won't be applied directly.
 Then it's the callers job to apply them afterwards with
 `emms-tag-editor-apply'."
-  (if (eq (emms-track-get track 'type) 'file)
+  (if (emms-track-file-p track)
       (let* ((old-file (emms-track-name track))
              (path     (file-name-directory old-file))
              (suffix   (file-name-extension old-file))
@@ -829,7 +853,7 @@ A pipe is defined like below:
 
 (defun emms-tag-editor-track-pipe (track pipe-name)
   "Run command of pipe nameed PIPE-NAME to TRACK."
-  (if (eq (emms-track-get track 'type) 'file)
+  (if (emms-track-file-p track)
       (let* ((coding-system-for-read 'utf-8)
              (command (emms-tag-editor-pipe-get pipe-name :command))
              (arguments (emms-tag-editor-pipe-get pipe-name :arguments)))
@@ -877,6 +901,5 @@ A pipe is defined like below:
       (dolist (track tracks)
         (emms-tag-editor-track-pipe track pipe-name)))))
 
-
 (provide 'emms-tag-editor)
 ;;; Emms-tag-editor.el ends here
diff --git a/emms.el b/emms.el
index 8c950b1..6965e2a 100644
--- a/emms.el
+++ b/emms.el
@@ -676,6 +676,10 @@ before the error is signaled."
   "Return the type of TRACK."
   (emms-track-get track 'type))
 
+(defun emms-track-file-p (track)
+  "True if TRACK is a file type"
+  (eq 'file (emms-track-type track)))
+
 (defun emms-track-name (track)
   "Return the name of TRACK."
   (emms-track-get track 'name))