JuK - removing k3support libraries

[Michael Pyne]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20090509/714ad6c2/attachment.sig>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia