<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109470/">http://git.reviewboard.kde.org/r/109470/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 28th, 2013, 4:44 p.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
</blockquote>
<p>On March 29th, 2013, 4:28 p.m. UTC, <b>mayank jha</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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!
</pre>
</blockquote>
<p>On March 30th, 2013, 11:13 a.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
<br />
<p>- mayank</p>
<br />
<p>On March 27th, 2013, 7:22 p.m. UTC, mayank jha wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By mayank jha.</div>
<p style="color: grey;"><i>Updated March 27, 2013, 7:22 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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. </pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Its working fine!</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/context/engines/lyrics/LyricsEngine.cpp <span style="color: grey">(2befa91)</span></li>
<li>src/context/applets/lyrics/LyricsApplet.cpp <span style="color: grey">(2394964)</span></li>
<li>src/context/engines/lyrics/LyricsEngine.h <span style="color: grey">(b187b73)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109470/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>