D9808: fix incorrect emission of signals by kLineEdit

David Faure noreply at phabricator.kde.org
Thu Jan 11 13:37:31 UTC 2018


dfaure added inline comments.

INLINE COMMENTS

> dweatherill wrote in klineedit_unittest.cpp:87
> ok, it's unclear, but not a typo.
> According to QLineEdit documentation, clear() should only emit textChanged when there is actually text to clear.
> 
> So the word "cleared" kind of implies "only after we've called clear()" I guess I should change it to
> 
>   //if text box is already empty, calling clear() shouldn't emit

Agreed, that's clearer (haha).

> dweatherill wrote in klineedit.cpp:184
> actually, reading more carefully, I agree it should not be renamed, just the spurious emission of textEdited should go

yes, it's connected to textChanged, so it SHOULD be called _k_textChanged, otherwise it gets really confusing.

Please submit an updated patch so we can reason about it.

Is this "spurious emission" there since 5.0? I'm worried about breaking apps due to the behaviour change... (not saying that's a reason to reject this, just needs additional thinking/research about the possible breakage)

REPOSITORY
  R284 KCompletion

REVISION DETAIL
  https://phabricator.kde.org/D9808

To: dweatherill, #frameworks, dhaumann, cullmann
Cc: mwolff, dfaure, anthonyfieroni, iodelay, vbspam, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180111/fde3e360/attachment.html>


More information about the Kde-frameworks-devel mailing list