Implementation of the "Custom Tags" Feature

Maximilian Kossick maximilian.kossick at googlemail.com
Thu Nov 5 14:40:09 CET 2009


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.


More information about the Amarok-devel mailing list