Review Request 109470: A patch which allows us to distinguish between cached lyrics and lyrics newly downloaded from the internet.

[Matěj Laitl]

> On March 28, 2013, 4:44 p.m., Matěj Laitl wrote:
> > Is there a wish request or any other motivation to implement this feature? I don't see many reasons why user should care that the lyrics have been just downloaded or already cached. On the other hand, user may want to know whether lyrics have been previously edited by her. Additionally, some file formats allow storing lyrics in tags, it would be nice if Amarok supported it. (then it would make sense to show her what is the source of lyrics being shown)
> 
> mayank jha wrote:
>     I thought perhaps this was needed in context/LyricsManager.cpp
>     230:        // TODO: add some sort of feedback that we could not fetch new ones
>     Thanks for your suggestions! Will try to work on the edited/unedited feature soon!
>
> 
> Matěj Laitl wrote:
>     > I thought perhaps this was needed in context/LyricsManager.cpp
>     > 230:        // TODO: add some sort of feedback that we could not fetch new ones
>     
>     Well, this is a valid TODO, but what you're implemented doesn't resolve it. The wanted feedback was we tried but failed to download lyrics, not to distinguish between cached and just-downloaded ones.
>     
>     > Thanks for your suggestions! Will try to work on the edited/unedited feature soon!
>     
>     Well, that may require a fair amount of new infrastructure and is not really suited for newcomers, but no-one will obviously prevent you from trying.
> 
> mayank jha wrote:
>     Is the feedback required to be displayed onto the screen ? or is it related to the code part ? Also doesnt showing cached lyrics indicate that we tried but could not find any new lyrics?

> Is the feedback required to be displayed onto the screen?

Yes, preferably using Amarok::Components::logger()->shortMessage()

> Also doesnt showing cached lyrics indicate that we tried but could not find any new lyrics?

Yes, but we don't expect our users to think that hard about it. They want to have fun playing their music, not a brain exercise.


- Matěj


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109470/#review30013
-----------------------------------------------------------


On March 27, 2013, 7:22 p.m., mayank jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109470/
> -----------------------------------------------------------
> 
> (Updated March 27, 2013, 7:22 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> It required modifications, when there is no change in the lyrics downloaded and lyrics retrieved from cache the title display of the lyrics browser changes to "Cached Lyrics" from "Lyrics" so we can tell the difference between old and new. 
> 
> 
> Diffs
> -----
> 
>   src/context/engines/lyrics/LyricsEngine.cpp 2befa91 
>   src/context/applets/lyrics/LyricsApplet.cpp 2394964 
>   src/context/engines/lyrics/LyricsEngine.h b187b73 
> 
> Diff: http://git.reviewboard.kde.org/r/109470/diff/
> 
> 
> Testing
> -------
> 
> Its working fine!
> 
> 
> Thanks,
> 
> mayank jha
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130402/88c1c840/attachment.html>