Fran Burstall <address@hidden> writes:
> Have a look at branch emms-playlist-limit for a re-write of
> emms-playlist-limit.el. This is my first piece of elisp for public
> consumption so criticism very welcome and almost certainly necessary.
>
This is all good. I'm fine with you merging this into master so that
everyone can play with it (especially considering that it already
compiles with no errors or warnings.)
Once you do we need to:
* write documentation (I'm volunteering to do that)
* document the change in one sentence in the NEWS file (you do that)
* add you to the AUTHORS file (you do that too)
Will do.
> As with the previous implementation, limiting always applies to the
> current playlist and makes the new limited playlist current. I
> wonder if that is the optimal behaviour?
>
> ---Fran
That's a good question. The confusion here would be that you could be
trying to limit a playlist buffer and keep on getting an error that no
tracks are being found because the code is looking for tracks from
_this_ buffer in the _current_ buffer:
1. create two playlist buffers "classical" and "prog rock"
2. make "classical" the current playlist buffer
3. switch to "prog rock" (note we are just switching to it, not
making it current)
4. try to limit to a track from "prog rock" (say, "21st Centry
Schizoid Man")
5. get an error because "21st Centry Schizoid Man" can't be found in
the "classical" buffer
The behavior isn't technically an error, but I think it falls short of
the Do What I Mean principle.
What do you think?
I agree that it falls short: I am likely to call the limiting function from a playlist buffer and expect it to limit what I am seeing. This is not the only place where emms has this slightly unexpected behaviour: hit C-x C-s in any playlist buffer and what is saved is the _current_ playlist: in yr example, I hit C-x C-s in the "prog rock" buffer so I can listen to King Crimson on my phone but the saved m3u (say) file is full of Beethoven!
My preference for emms-playlist-limit would be:
1. Limit the playlist buffer it is called from (and error out if it is called from a non-playlist buffer).
2. If the calling buffer is the current playlist, make the limited playlist current, otherwise not.
3. Have a variable emms-playlist-limit-only-current so that users can get the old behaviour back if they want it.
Similar remarks apply to emms-playlist-save and, indeed, all other commands in the emms-playlist-mode-map: it violates the Principle of Least Surprise when they act on another, possibly invisible, buffer. (That said, most commands in the mode-map do apply to the calling playlist but the limit fns and save are definite exceptions!). If emms-playlist-save saved the calling buffer, this would go a long way towards making emms a comfortable environment for _editing_ playlists (which is one of the reasons I was attracted to emms in the first place) as well as playing them. Thoughts?
---Fran