[Emms-patches] Re: completion in lastfm

[Thierry Volpiatto]

Thierry Volpiatto <address@hidden> writes:

> Hi Michael!
> Thanks you for your advices, i am not already use to emms and i discover
> the code, though i use emms since a long time now.
> That is the code i took from my emms config and port to emms.
> I will try to fix all that as soon as possible and send you another patch.
>
> Michael Olson <address@hidden> writes:
>
>> 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.
>
> ok
>
>>  - 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.
>
> ok i agree ,that easy to change.
>
>>  - What's the point of running a separate timer?
>
> I thought the original timer was in use for something else, so in the
> doubt, i create a new one.

And also the actual timer don't work, it is killed by
`emms-player-stopped-hook' :

The actual emms-timer is run in `emms-lastfm-radio-sentinel'
What happen is difficult to fix:
When it add to hook:

,----
| (add-hook 'emms-player-stopped-hook
|           'emms-lastfm-cancel-timer))
`----

That have a side effect that is killing the timer.

When the function is run in the debugger step by step,
that doesn't occur, but when i run it as normal,
the timer is killed and set to nil.

When i run this function without adding the cancel-timer to hook
everything work correctly.

I tried to (sit-for ?) before adding hook but it still not working.


>>  - Use emms-play-lastfm instead of emms-lastfm-radio.  I suspect that
>>    would remove the need for the timer.
>
> That would be good.
>
>>  - 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.
>
> Yes the list is all the global tags , but each global tag have
> "under-tags".
>
>>  - 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.
>
> ok.

-- 
A + Thierry Volpiatto
Location: Saint-Cyr-Sur-Mer - France