Patch for KTabBar Close Buttons

[Aaron J. Seigo]

On Friday 23 November 2007, csh wrote:
> this is my first patch ever. It implements close buttons for tabs. The only
> thing I couldn't quite figure out is the formula for the icon postion, but
> besides that it should be fine.

first, thanks for the patch and working on kdelibs stuff =)

some comments:

* you don't need Q_DECLARE_FLAGS since this isn't a flag (e.g. something you | 
together)

* setCloseButton(int) should be setCloseButton(CloseButton); closeButton() 
should likewise return a CloseButton

* some better names might help clarify the API as well, e.g. 
setCloseButtonMode, closeButtonMode and ClostButtonMode or something similar 
that doesn't sound like the API actually sets a close button.

* does void setTabIcon(int index, const QIcon & icon); need to be public API 
or can that go into the private class? ditto for the protected methods.

* please follow the kdelibs coding style[1], e.g. this:

+   if(d->mCloseButtonSetting == KTabBar::NoCloseButton )
+      return;

should be:

+     if (d->mCloseButtonSetting == KTabBar::NoCloseButton) {
+        return;
+    }

* as for the findIconGeometry with the "there's gotta be a system behind 
it: --> insert here ;)" comment, you're right. there is a system: QStyle. the 
values you have there are for whatever specific widget style you are using 
(and whichever styles have the same sized tabs). you want to query the 
current style() for pixel metrics. take a look at the QStyle documentation, 
particularly for pixel metrics that start with PM_TabBar =) hth.

[1] http://techbase.kde.org/Policies/Kdelibs_Coding_Style

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20071124/82c25d8c/attachment.sig>