playlist overhaul day 2

[Ian Monroe]

On Tue, Oct 7, 2008 at 10:21 AM, Soren Harward <stharward at gmail.com> wrote:
> Thanks to help from Nikolaj, I've fixed the two major crasher bugs with my
> playlist overhaul: when Amarok is started with an empty playlist, and when all
> the playlist items are removed.  I've also renamed PMGroupingProxy to
> GroupingProxy and moved it into the main playlist dir, per Ian's suggestion.

Issues to fix before pushing:
Can you get rid of the using namespaces too? :) Its the sort of thing
that will be hard to fix later due to the risk of conflicts.

PlaylistActions.cpp needs a #include <typeinfo> for GCC 4.3.

Issues to worry about after pushing:
You also have ::instance() and The:: methods both. The The::'s method
just call the ::instance. Why not just get rid of ::instance.

You also have ::destroy methods which is a good topic to bikeshed on
so I don't really want to get into it, but I think its silly.

These are all the kind of issues that we would've been able to resolve
if you had been more open with the patch from the beginning.

> If I understand correctly, the most significant objection to my commit is the
> replacement of the playlist view, and the subsequent loss of the animations.
> My commit does not remove the existing GraphicsView, and I've already pretty
> much ported the GraphicsView to the new playlist model, so if the decision is
> made to return to the GraphicsView, it won't be terribly difficult to make the
> change in the code.
>
> Given that the primary objection is something that can fairly easily be undone,
> and considering the major improvements in other areas (such as undo/redo and
> the navigators), I'm going to go ahead and commit the code later this
> afternoon.  I know that it's not entirely bug-free, but at this point I think
> it's best to have more eyes and brains than just mine working on this code.
>
> --
> Soren Harward
> stharward at gmail.com
> _______________________________________________
> Amarok-devel mailing list
> Amarok-devel at kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>