[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [emms-help] [Patch] Fix bugs in xxx-clear/current-kill functions

From: Carlos Pita
Subject: Re: [emms-help] [Patch] Fix bugs in xxx-clear/current-kill functions
Date: Wed, 9 Jan 2019 14:14:12 -0300

> emms-playlist-current-clear. AFAICS is plainly wrong for
> emms-playlist-clear.

The behavior of the function is correct, so I fixed the doc string.

I think so. My patch removes a redundant check though, besides fixing the docstring.

(kbd "C") can call `emms-playlist-clear' directly, and since nobody
calls `emms-playlist-mode-clear' it can be removed. I changed that.

If your intention is to remove the ability to invoke playlist-mode-clear altogether from outside a playlist buffer, which seems a reasonable alternative to me, I believe you should do the same with playlist-mode-current-kill, for the sake of consistency, splitting playlist-current-kill into playlist-current-kill and playlist-kill. Then K would call playlist-kill to kill the current buffer if it's a playlist, period.

If that's not your intention, then take into account that playlist-mode clear and kill functions should act on the *current buffer* if it's a playlist but on the *current playlist* if it's not. Otherwise the behaviour would be very surprising. That's why I renamed it to playlist-mode-current-clear which is consistent both in name and behavior with playlist-mode-current-kill. My function simply calls playlist-clear if in a playlist buffer but playlist-current-clear if not.

That said, I like the simpler approach of disallowing playlist-mode-kill/clear from outside a playlist buffer, in that case I'm just pointing out that it's desirable to keep consistency between kill and clear.

I don't know that the behavior of `emms-playlist-current-kill' makes
sense. I'll have to review it.

In general I think it makes sense, but the "always keep at least one playlist" behaviour may be unnecessary. Althought it seems to be related to theĀ "create a new playlist if there is no one" feature of playlist-current-clear.

Best regards

reply via email to

[Prev in Thread] Current Thread [Next in Thread]