On July 26th, 2011, 10:46 p.m., Aleix Pol Gonzalez wrote:

kdeui/widgets/klineedit.cpp (Diff revision 1)
void KLineEdit::updateClearButtonIcon(const QString& text)
		349	if (d->wideEnoughForClear && text.length() > 0) {

Wouldn't it be better to put it this way? Just saying...

d->clearButton->animateVisible(d->wideEnoughForClear && text.length() > 0);

On July 27th, 2011, 10:52 p.m., Nicolas Alvarez wrote:

I think the original is clearer, to be honest.

On July 27th, 2011, 11:45 p.m., Hugo Pereira Da Costa wrote:

I agree with Nicolas.
I have nothing against putting boolean test inside the method call, *in principle*, but I believe it's convenient only if the boolean condition is "short enough" to write. 

Other than that, anyone willing to go for a "ship it" ? 
It was reported on bug 268898 that the patch actually works, and that no regression has been found so far (which is also my own experience)

- Hugo

On July 26th, 2011, 9:54 p.m., Hugo Pereira Da Costa wrote:

Details:
- fixes the somewhat incorrect logic in KLineEditButton::animateVisible
- simplifies KLineEdit::updateClearButtonIcon consequently.

tested with klineedittest found in kdelibs/kdeui/tests, this with and without the patch attached to comment #1 of bug 268898, used to actually trigger the mentionned bug. Also tested with other klineEdit implementation such as Dolphin's location bar.