On Mon, 24 Sep 2018 at 21:37, Yoni Rabkin <[email protected]> wrote: > Fran Burstall <[email protected]> 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
_______________________________________________ Emms-help mailing list [email protected] https://lists.gnu.org/mailman/listinfo/emms-help
