patches for custom styleelements, kcapacitybar

Thomas L├╝bking thomas.luebking at web.de
Sun Sep 28 16:27:04 BST 2008


Am Saturday 27 September 2008 schrieb Maksim Orlovich:
> I think you're also making it
> far more complex than needed. Why does one need to support all of the
> CC_/CE_/PE_ mess in the first place?
bases upon cooments in earlier discussions here. (initially there were only 
PE, CE and CC)
however, you can have this support for as few or many element types as you 
wish (as it could turn out a custom widget makes use of case dependent 
subcontrols, having CC would be pretty nice)
just for kcapacitybar, it would be complete overhead - yes.

> Further, reservations or not, there is no guarantee that someone else isn't 
> using the same primitive numbers. [....] qobject_cast<KStyle> is really the
> only safe option
though you need to hit two numbers (SH_ and SO_) in a row, it's fragile, yes.

my first attempt was (considered) more "hackish" but is rather solid as 
well and includes passing widgets with adjusted objectnames instead of a 
KStyleOptionCustomElement (if you don't worry about this, an upgraded kstyle 
patch is attached)
if then an application really uses this stylehint alongside a widget named 
PE_whatever and it will get an unexpected stylehint: selber schuld ;-P

i'd say it's even harder to break things then by checking kstyle inheritance 
AND it still works w/o invoking kstyle or linking kdeui at all

> And if you do that, then you can get rid of typo-prone string-based API in 
the first place.
if you get rid of the typo-prone string-based API, you'll be either stuck with 
elements predefined in KStyle (what i wanted to avoid) or depend on silent 
number agreements, what's imho little more prone than strings.

if you choose to allow runtime extension of the element list, you'll have 
typo-prone strings anyway (as whatever the strings are, style and widget will 
have to agree exactly on them... luckily you'll usually see that it doesn't 
work right in the first test after compiling)

...but i see your (api cleaning) point.
so i splitted the functions (this multiplies chances for a number conflict in 
the stylehint query, but this should be no problem using the harder objectname 
trick)
the requirement for a (documented) naming convention however remains and is 
used to harden the whole thing against accidental misuse

> Anyway, the addSupportFor method should return the ID; returning a single
yes, wasn't happy either. renamed the function now returning the value.

> (Oh, and signedness of enums is platform-dependent, and the range of the
> underlying type depends on the range of declared enumerants; they can
> easily be 8-bit).
o-k... nevertheless:
the PE_CustomBase differs from other styleelement bases 
for i don't see what reason. i however removed the inappropriate comment - 
thanks for the information.

> Please also write proper API docs, using the Doxygen tags. There is no
check (hopefully...)
> Please do not use #define's for constants. C++, unlike C has a proper
check (#define'ing function prototypes is hopefully ok, otherwise you're free 
to multiply the code yourself - i'm soooo lazy ;-P
> Please do not use names like element_counter, but rather elementCounter.
check
> Please do not write code that has multiple sideeffects in one line, e.g.:
check
(i hope "id = ++d->element_counter[KStylePrivate::PrimitiveElement]" is ok?
c'mon, that's not too complex + the code got simpler in that area anyway ;-)

some comment typos probably remained... or were added. you may safely svn 
blame me in case =D

Regards and thanks for advice,
Thomas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: customelements.oxygen.diff
Type: text/x-patch
Size: 1851 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080928/71f2bbd1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: customelements.kstyle.diff
Type: text/x-patch
Size: 12568 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080928/71f2bbd1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: customelements.kcapacitybar.diff
Type: text/x-patch
Size: 1998 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080928/71f2bbd1/attachment-0002.bin>


More information about the kde-core-devel mailing list