Implementation of the "Custom Tags" Feature
Dan Meltzer
parallelgrapefruit at gmail.com
Sun Nov 8 08:41:48 CET 2009
I had a bout of insomnia this evening, and so I sat down and rewrote
the customlabels feature as we've kind of discussed. There is now a
ReadLabelCapability and a WriteLabelCapability (still debating on if
we want to merge these... I don't think so though). I've moved the
sql code over, and modified them to be asynchronous. I am currently
working on a lastfm ReadLabelCapability as well, because I kind of
like the idea :). I tried to add it to the playlist, but I can't
figure out how to get it in the layout...
I'm going to keep the work in my clone for now, dmeltzers-playground
on gitorious, but I'll try and track you down soon to see about
collaborating.
Dan,
On Thu, Nov 5, 2009 at 9:02 AM, Nikolaj Hald Nielsen
<nhnfreespirit at gmail.com> wrote:
> So, irregardless of where the actual code/ui should go: Well done! :-)
>
> Labels is something from the Amarok 1.4 days that I have been missing
> and it is good to see them making a comeback.
>
> Integrating labels into the playlist should be pretty simple. You just
> have to add a label "row" to PlaylistDefines.h (careful, the correct
> order needs to be maintained through multiple lists in that file), add
> a case statement to the data() function in the PlaylistModel to return
> the actual string to show and then finally add a token to the
> PlaylsitLayoutManager so it can be added to new layouts (that is a 1
> liner once the other stuff is done). All this should be trivial once
> you look at how it already works for the other possible values.
>
> Oh, and then a little code might be needed in the inline editor to
> "split up" the string into seperate labels if the user changes it (I
> guess a comma seperated list will be shown?)
>
> Integrating it into the QueryMakers is going to be more... interesting... :-)
>
> - Nikolaj
>
>
> On Thu, Nov 5, 2009 at 2:40 PM, Maximilian Kossick
> <maximilian.kossick at googlemail.com> wrote:
>> On Thu, Nov 5, 2009 at 1:49 PM, Daniel Dewald
>> <Daniel.Dewald at time-shift.de> wrote:
>>> On Thursday, 5. November 2009 08:42:59 Maximilian Kossick wrote:
>>>> I agree with Jeff. Adding the labels SQL code to TagDialog is definitely
>>>> wrong.
>>>>
>>>> the logical place to put the label management code is a Capability in
>>>> my opinion. Adding support for labels to querymaker is going to be
>>>> interesting to say the least, especially as we probably should move
>>>> the label queries for SQL based collections into the actual SQL
>>>> queries for performance reasons. And as MySQL supports sub queries
>>>> (unlike Sqlite) we can use sub queries. Please do not use the left
>>>> joins of the A1 labels queries as an example, those never worked right
>>>> for complex queries.
>>>
>>> To be honest I put it where it is now because I simply didn't understand the
>>> Meta Stuff let alone the Query Maker. And since there is not really any
>>> documentation on this stuff (is there?) changes in that way will have to be
>>> made by someone who actually understands that stuff.
>>
>> Hm, these are two parts of Amarok that are actually documented. Well,
>> at least they have api documentation. There is no design
>> documentation. I'd suggest that you grep for usage of Meta::Capability
>> (and subclasses) in Amarok. For example EditCapability in TagDialog.
>>
>> My suggestion would be to add a new capability that reads and writes
>> labels for a single track.
>>
>>>> On Wed, Nov 4, 2009 at 7:58 PM, Jeff Mitchell <mitchell at kde.org> wrote:
>>>> > OK, some comments.
>>>> >
>>>> > First, I'm worried about adding all of this to the Tag Dialog when a lot
>>>> > of us are feeling like the Tag Dialog is crufty and needs to be
>>>> > rewritten anyways.
>>>> >
>>>> > Second, those various SQL statements should not live in the Tag Dialog.
>>>> > They need to be moved to the Sql Collection. There are other places
>>>> > around Amarok that have embedded SQL, but they're in the wrong. I think
>>>> > we should attempt to have new code do it right.
>>>>
>>>> there are places where embedding SQL makes sense (e.g.
>>>> ServiceSqlCollection). TagDialog is not one of them, I agree.
>>>
>>> Same as above. Putting it somewhere else would simply have meant for me to not
>>> put it anywhere at all and let the label stuff to someone else. As I know that
>>> the devs are very busy with other stuff I decided to give it a try (Since a
>>> lot of people requested this feature) and saw it that way: Better a working
>>> feature that's misplaced then none at all.
>>>
>>
>> I most empathicallly disagree.
>> _______________________________________________
>> Amarok-devel mailing list
>> Amarok-devel at kde.org
>> https://mail.kde.org/mailman/listinfo/amarok-devel
>>
> _______________________________________________
> Amarok-devel mailing list
> Amarok-devel at kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>
More information about the Amarok-devel
mailing list