Fran Burstall <[email protected]> writes: > 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.
I agree with the above. > 3. Have a variable emms-playlist-limit-only-current so that users > can get the old behaviour back if they want it. Won't hurt to have; not a big deal to omit. > 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 We should modify this behavior where we find it so that Emms conforms with the rest of Emacs; when I hit C-x C-s it saves the buffer I'm looking at, and not some invisible buffer. I've added this to my general TODO. -- "Cut your own wood and it will warm you twice" _______________________________________________ Emms-help mailing list [email protected] https://lists.gnu.org/mailman/listinfo/emms-help
