[emms-help] Re: [PATCH] BUG: Playing empty directory tree

[Daniel Brockman]

Jorgen Schaefer <address@hidden> writes:

> Daniel Brockman <address@hidden> writes:
>
>> Note the removal of the final period in the error message, as per the
>> documentation of `error':
>>
>>     ``In Emacs, the convention is that error messages start with a
>>     capital letter but *do not* end with a period.  Please follow this
>>     convention for the sake of consistency.''
>>
>> Oh, and you get another minor fix for free.
>
> Applied in CVS. Thanks for spotting this!

You like that fix?  Here's a bunch of similar ones.

Most of the patch removes trailing periods, but in some cases, like
the following, the error messages are made more to-the-point:

  - (error "No tracks found for playing! Look at `emms-source-list'."))
  + (error "No tracks found in `emms-source-list'"))

While less ``friendly'', I think the latter version is more consistent
with other Emacs error messages.


Note also this change:

  - (unless (featurep 'emms-info)
  -   (error "You have to load emms-info before using ...."))
  + (require 'emms-info)

Duplicating the functionality of `require' seems unnecessary. :-)


Finally, here's the reason I sent in the previous patch to begin with:

  - (error "No tracks found" source))
  + (error "No tracks found"))

There's nothing called `source' in scope there.  Maybe you meant to
say `lst'?  Then again, I don't see what that is supposed to do.


Apologies for lumping everything in one patch, but these changes are
so small I figured multiple patches would actually be less convenient.

By the way, I'm getting annoyed by the phrase ``player playing''. :-)
Wouldn't ``player running'' be better?  ``A player is already
playing.'' vs. ``A player is already running.''  Unfortunately, this
language is used in code, too (`emms-player-playing-p', etc.), so the
change might not be worth it.


Again, thanks for an awesome piece of software!

Regards,

-- 
Daniel Brockman
address@hidden

cvs diff: Diffing .
Index: emms-pl-manip.el
===================================================================
RCS file: /cvsroot/emms/emms/emms-pl-manip.el,v
retrieving revision 1.15
diff -u -r1.15 emms-pl-manip.el
--- emms-pl-manip.el    16 Feb 2004 20:16:06 -0000      1.15
+++ emms-pl-manip.el    1 Oct 2004 14:09:04 -0000
@@ -116,8 +116,7 @@
 (defun emms-pl-manip-sort-by-info-artist ()
   "Sort the playlist, using "
   (interactive)
-  (unless (featurep 'emms-info)
-    (error "You have to load emms-info before using 
emms-pl-manip-sort-by-info-artist."))
+  (require 'emms-info)
   (emms-pl-manip-sort (lambda (entry)
                         (emms-info-artist (emms-info-get entry)))
                       'string<))
Index: emms.el
===================================================================
RCS file: /cvsroot/emms/emms/emms.el,v
retrieving revision 1.33
diff -u -r1.33 emms.el
--- emms.el     1 Oct 2004 13:40:36 -0000       1.33
+++ emms.el     1 Oct 2004 14:09:04 -0000
@@ -180,7 +180,7 @@
     (emms-stop))
   (if (emms-playlist-next)
       (emms-start)
-    (error "No next track in playlist.")))
+    (error "No next track in playlist")))
 
 (defun emms-next-noerror ()
   "Play the next track in the playlist, but don't signal an error when
@@ -188,14 +188,14 @@
 This is a suitable function to put in `emms-player-finished-hook'."
   (interactive)
   (when emms-player-playing-p
-    (error "A track is already playing."))
+    (error "A track is already being played"))
   (cond ((emms-playlist-next)
         (emms-start))
        (emms-repeat-playlist
         (setq emms-playlist-current 0)
         (emms-start))
        (t
-        (message "No next track in playlist."))))
+        (message "No next track in playlist"))))
 
 (defun emms-previous ()
   "Play the previous track in the playlist."
@@ -204,7 +204,7 @@
     (emms-stop))
   (if (emms-playlist-previous)
       (emms-start)
-    (error "No previous track in playlist.")))
+    (error "No previous track in playlist")))
 
 ;;;###autoload
 (defun emms-play-all ()
@@ -219,7 +219,7 @@
                                                (cdr source)))
                                       emms-source-list))))))
     (when (null new)
-      (error "No tracks found for playing! Look at `emms-source-list'."))
+      (error "No tracks found in `emms-source-list'"))
     (emms-stop)
     (emms-playlist-set-playlist (vconcat new))
     (emms-playlist-set-current 0)
@@ -480,7 +480,7 @@
   "Play the tracks in LIS."
   (let ((new (vconcat lis)))
     (when (zerop (length new))
-      (error "No tracks found" source))
+      (error "No tracks found"))
     (emms-stop)
     (emms-playlist-set-playlist new)
     (emms-playlist-set-current 0)
@@ -541,10 +541,10 @@
 (defun emms-player-start (track)
   "Start playing TRACK."
   (if emms-player-playing-p
-      (error "A player is already playing - stop it first.")
+      (error "A player is already playing")
     (let ((player (emms-player-for track)))
       (if (not player)
-          (error "I can't play %S!" track)
+          (error "Don't know how to play file: " track)
         (funcall (emms-player-get player 'start)
                  track)
         (setq emms-player-playing-p player)