JuK - removing k3support libraries

[Georg Grabler]

Already recoded those parts. Will update the patch until wednesday (a bit
hard during the week when I'm working .. to be ture, it's sunday and I'm
currently at work :p).

On Sun, May 10, 2009 at 1:27 PM, Georg Grabler <ggrabler at gmail.com> wrote:

> Hi Michael,
>
> Well, first of all - i dropped it here before I could the reviewboard
> working :p ... had some troubles finding Scott or you in there as a
> reviewer.
>
> Secondly, the swapping of the lines does not really improve anything, they
> got turned when I edited the parts. I'll try to update this accordingly, so
> the patch is smaller.
>
> I've never looked at QStringListModel (to be true, I didn't know it even
> exists, since the documentation had a stringlistmodel as an example, i
> though there isn't anything like that). I'll take a look at those parts, and
> will send a updated version of the patch (a link this time, had troubles
> getting this patch in the lists due to being taller than 40kb) and update it
> on the reviewboard.
>
> Anyway, I learned about models, means I can continue with parts where I
> definitely will need my own models and delegates (covermanager, playlist).
>
> For the directorylist .. well, since I didn't know about QStringListModel,
> it was likely that the solution is the easiest I did know. Will also take a
> look at this.
>
> Kind regards,
> Georg
>
> 2009/5/10 Michael Pyne <mpyne at purinchu.net>
>
>> On Saturday 09 May 2009 13:39:00 Georg Grabler wrote:
>> > Hello,
>> >
>> > First of all - hi Michael, I'm still alive (pest plants never die).
>> >
>> > I've started porting some JuK parts which still contain k3support
>> libraries
>> > to qt4. I started with the easier parts before i get to the harder
>> things
>> > as the playlist and the covermanager.
>> >
>> > I thought, I'd better send you what I did until now, for two reasons:
>> > - Review
>> > - Possible conflicts if somebody of you guys updates JuK
>>
>> I see you've also put it up on review board.  I'd review it there
>> (assuming it
>> works in either Konq or Arora yet...) but my password was lost in the
>> Great
>> Kernel Panic of 2009 since it was newer than my last $KDEHOME backup, and
>> there is no way I can see to reset review board passwords.
>>
>> So I'll just do it the old-fashioned way. ;)
>>
>> Starting from tagguesserconfigdlg.cpp:
>>
>> Removing the <kicon.h> include is probably not a good idea, even if it
>> still
>> compiles, as the KIcon constructor is used.
>>
>> Also, some lines are re-arranged in the TagGuesserConfigDlg constructor,
>> for no
>> reason I can see.  Does this improve anything?  Reason I ask is because it
>> makes it harder to figure out why you removed some lines and then oh-wait,
>> they
>> just moved down somewhere else.
>>
>> +    else if (m_child->bMoveUp->isEnabled() == false)
>> +        m_child->bMoveUp->setEnabled(true);
>>
>> This can just be m_child->bMoveUp->setEnabled(true), doing the specific
>> check
>> isn't necessary.  If it is important that the result not be duplicated
>> then
>> typically the class with the setFoo method should ensure that, not calling
>> code.
>>
>> +    if (item == model->index(model->rowCount(QModelIndex())-1, 0,
>> QModelIndex()))
>> +        m_child->bMoveDown->setEnabled(false);
>>
>> There is probably a bug here if there are no schemes in the model, be sure
>> to
>> check for that as well.  Also my advice about checking for enabledness
>> applies
>> here as well.
>>
>> On an overall note the ::accept() method is a bit convoluted trying to
>> extract
>> a QStringList out of the model.  But Qt already provides a
>> QStringListModel,
>> which you should either use directly and remove TagGuesserTreeModel, or
>> make
>> TagGuesserTreeModel a subclass of, that way your way of getting the
>> QStringList could be simply model->stringList().
>>
>> Likewise the DirectoryList changes would probably easier using a
>> QStringListModel since it just holds a list of folder names.
>>
>> I haven't actually had a chance to compile and run the patched version,
>> I'll
>> let you know if there are problems that weren't already present. ;)
>>
>> Regards,
>>  - Michael Pyne
>>
>> _______________________________________________
>> kde-multimedia mailing list
>> kde-multimedia at kde.org
>> https://mail.kde.org/mailman/listinfo/kde-multimedia
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20090510/218e8a48/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia