Hi, There are some bugs in the family of functions mentioned in the subject:
1. emms-playlist-clear doesn't create a new playlist as announced in the docstring. Indeed, the docstring seems to be copy-pasted from emms-playlist-current-clear. AFAICS is plainly wrong for emms-playlist-clear. 2. emms-playlist-mode-clear unnecessarily and faulty repeats code in emms-playlist-clear, forgetting to call emms-playlist-cleared-hook in some cases. 3. Despite its documentation, emms-playlist-mode-clear will fail when the current buffer is not an emms playlist one, since it calls emms-playlist-clear which starts by ensuring the current buffer is a playlist. Probably the intention was to call emms-playlist-current-clear instead (and probably emms-playlist-mode-clear should be renamed to emms-playlist-mode-current-clear for consistency). Also, there is an usability issue I discussed in [1]: 4. Killing a playlist while in in playlist mode should move to the next one, if any, instead of "closing" emms. If there is no next one, just bury the current one (here I'm being conservative, it's the current behavior, intended or not). This improves usability and more closely aligns the behavior of emms-playlist-mode-current-kill to that of emms-playlist-current-kill. I'm sending a patch fixing all the above. I've taken special care of documenting the exact behavior of all functions, specially for corner cases. All in all, the code has been simplified. Every single change has been documented in the commit message. Fill free to amend it if you don't agree with everything. Best regards -- Carlos [1] http://lists.gnu.org/archive/html/emms-help/2019-01/msg00010.html
From 8486678cec14542ef8a169b80433dfaf016e654d Mon Sep 17 00:00:00 2001 From: memeplex <[email protected]> Date: Sun, 6 Jan 2019 17:08:49 -0300 Subject: [PATCH] Fix bugs in xxx-clear/current-kill functions emms-playlist-mode-current-kill: - make docstring reflect actual behavior - make it more usable and consistent with emms-playlist-current-kill by switching to the next playlist if possible emms-playlist-mode-clear: - rename to emms-playlist-mode-current-clear for consistency - avoid code duplication - don't forget to call emms-playlist-cleared-hook - make it call emms-playlist-current-clear when the current buffer is not a playlist (instead of failing) emms-playlist-mode-clear: - remove redundant check emms-playlist-clear: - rewrite docstring since it was plainly wrong --- lisp/emms-playlist-mode.el | 23 ++++++++++++----------- lisp/emms.el | 8 ++++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lisp/emms-playlist-mode.el b/lisp/emms-playlist-mode.el index 4ac5b3c..1e0cc09 100644 --- a/lisp/emms-playlist-mode.el +++ b/lisp/emms-playlist-mode.el @@ -146,7 +146,7 @@ This is true for every invocation of `emms-playlist-mode-go'." (define-key map (kbd "K") 'emms-playlist-mode-current-kill) (define-key map (kbd "?") 'describe-mode) (define-key map (kbd "r") 'emms-random) - (define-key map (kbd "C") 'emms-playlist-mode-clear) + (define-key map (kbd "C") 'emms-playlist-mode-current-clear) (define-key map (kbd "d") 'emms-playlist-mode-goto-dired-at-point) (define-key map (kbd "<mouse-2>") 'emms-playlist-mode-play-current-track) (define-key map (kbd "RET") 'emms-playlist-mode-play-smart) @@ -184,23 +184,24 @@ FUN should be a function." (defun emms-playlist-mode-current-kill () "If the current buffer is an EMMS playlist buffer, kill it. -Otherwise, kill the current EMMS playlist buffer." +Otherwise, kill the current EMMS playlist buffer. +In any case, switch to the next playlist buffer. +If there is a single playlist, bury the buffer instead of killing it." (interactive) (if (and emms-playlist-buffer-p - (not (eq (current-buffer) emms-playlist-buffer))) - (kill-buffer (current-buffer)) + (cdr (emms-playlist-buffer-list))) + (let ((buf (current-buffer))) + (call-interactively #'emms-playlist-mode-next) + (kill-buffer buf)) (emms-playlist-current-kill))) -(defun emms-playlist-mode-clear () +(defun emms-playlist-mode-current-clear () "If the current buffer is an EMMS playlist buffer, clear it. Otherwise, clear the current EMMS playlist buffer." (interactive) - (if (and emms-playlist-buffer-p - (not (eq (current-buffer) emms-playlist-buffer))) - (let ((inhibit-read-only t)) - (widen) - (delete-region (point-min) (point-max))) - (emms-playlist-clear))) + (if emms-playlist-buffer-p + (emms-playlist-clear) + (emms-playlist-current-clear))) (defun emms-playlist-mode-last () "Move to directly after the last track in the current buffer." diff --git a/lisp/emms.el b/lisp/emms.el index 0da19d5..a83aa11 100644 --- a/lisp/emms.el +++ b/lisp/emms.el @@ -857,7 +857,8 @@ other EMMS buffers. The list will be in newest-first order." emms-playlist-buffers) (defun emms-playlist-current-kill () - "Kill the current EMMS playlist buffer and switch to the next one." + "Kill the current EMMS playlist buffer and switch to the next one. +If there is no next one, just bury the current EMMS playlist buffer." (interactive) (when (buffer-live-p emms-playlist-buffer) (let ((new (cadr (emms-playlist-buffer-list)))) @@ -874,14 +875,13 @@ other EMMS buffers. The list will be in newest-first order." "Clear the current playlist. If no current playlist exists, a new one is generated." (interactive) - (if (or (not emms-playlist-buffer) - (not (buffer-live-p emms-playlist-buffer))) + (if (not (buffer-live-p emms-playlist-buffer)) (setq emms-playlist-buffer (emms-playlist-new)) (with-current-buffer emms-playlist-buffer (emms-playlist-clear)))) (defun emms-playlist-clear () - "Clear the current buffer. + "Clear the current buffer if it's an EMMS playlist buffer. If no playlist exists, a new one is generated." (interactive) (emms-playlist-ensure-playlist-buffer) -- 2.20.1
_______________________________________________ Emms-help mailing list [email protected] https://lists.gnu.org/mailman/listinfo/emms-help
