[Emms-patches] Re: completion in lastfm

[Michael Olson]

Thierry Volpiatto <address@hidden> writes:

> Hi emms,
> here a little patch to have some completion in lastfm for global-tag
> and similar-artist.
> I didn't touch these functions, i just create new ones with an
> independent timer to not interfer with the actual one.
> This patch apply fine on revision:

Thanks for the patch.  Some comments follow.

 - emms-completing-read should go into emms.el, likewise its variable.
   It is useful in other places in the code, too.

 - Please don't use loop, unless it actually does something nontrivial.
   It's not good form to make a file that does not require 'cl to
   require it, especially when a simple dolist will suffice.

 - What's the point of running a separate timer?

 - Use emms-play-lastfm instead of emms-lastfm-radio.  I suspect that
   would remove the need for the timer.

 - Is there some URL we can hit to get a list of all available global
   tags, perhaps caching that somewhere?  The list provided in this
   patch seems a bit small.

 - The new functions that prompt the user for a tag or similar artist
   should be called `emms-play-lastfm-tag' and `emms-play-lastfm-artist'
   respectively.

-- 
|       Michael Olson  |  FSF Associate Member #652     |
| http://mwolson.org/  |  Hobbies: Lisp, HCoop          |
| Projects: Emacs, Muse, ERC, EMMS, ErBot, DVC, Planner |
`-------------------------------------------------------'