Re: Review Request 126308: KPushButton: respect SH_DialogButtonBox_ButtonsHaveIcons, also when drawing

[David Faure]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126308/#review89737
---



src/kdeui/kpushbutton.cpp (line 256)


This patch looks wrong because KPushButton can be used outside of "dialog 
button boxes", while the styleHint is supposed to be only about dialog button 
boxes.

QPushButton::sizeHint does this:
bool showButtonBoxIcons = 
qobject_cast(parentWidget())
  && 
style()->styleHint(QStyle::SH_DialogButtonBox_ButtonsHaveIcons);
which is a solution for testing the parent widget.

I still don't fully understand the issue though, at painting time both 
QPushButton and KPushButton call QStyle's CE_PushButton, so I don't see why 
these two would be working differently.
Is this a workaround for KPushButton which doesn't fix QPushButton?


- David Faure


On Dec. 11, 2015, 4:26 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126308/
> ---
> 
> (Updated Dec. 11, 2015, 4:26 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks, Qt KDE, Hugo 
> Pereira Da Costa, and Yichao Yu.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> KF5 applications have long had a habit of drawing icons on buttons even when 
> this feature was turned off in the user's setting. This was mostly noticeable 
> in applications built on kdelibs4support.
> 
> It seems that the actual culprit is in Qt's QPushButton implementation 
> (https://bugreports.qt.io/browse/QTBUG-49887), but it is possible to work 
> around it in `KPushButton::paintEvent`, by removing the icon (forcing it to 
> the null icon) in the option instance, before handing off control to the 
> painter.
> 
> 
> Diffs
> -
> 
>   src/kdeui/kdialogbuttonbox.cpp 0f6649b 
>   src/kdeui/kpushbutton.cpp 98534fa 
> 
> Diff: https://git.reviewboard.kde.org/r/126308/diff/
> 
> 
> Testing
> ---
> 
> On Kubuntu 14.04 and OS X 10.9.5 with Qt 5.5.1 and KF5 frameworks 5.16.0 .
> 
> I have not yet verified if there are other classes where this modification 
> would be relevant too.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

Re: Why is C90 enforced in KDE?

[Kevin Kofler]

Thomas Lübking wrote:
> as a build dep requires it, "we just need //" isn't true - the
> requirements are controlled outside

Well, in that case, this:

> - pipe flex/yacc results through c++ rather than the C compiler

won't do much good, because the C++ compiler will not necessarily grok any 
other C99isms in the code. This only really helps for "//" comments.

My suggestion is:
* do not ship Flex-generated code, and
* document the compatibility issue, and let it be the user's responsibility
  to use a version of Flex compatible with their compiler, which may be an
  older or patched one.

This is entirely a compatibility issue between 2 build tools (Flex and the 
compiler), I don't see how this should be our (KDE's) problem.

Kevin Kofler