D9808: fix incorrect emission of signals by kLineEdit

Dan Weatherill noreply at phabricator.kde.org
Thu Jan 11 13:22:01 UTC 2018


dweatherill added inline comments.

INLINE COMMENTS

> mwolff wrote in klineedit_unittest.cpp:87
> typo: if the text box is already clear*ed*

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

> anthonyfieroni wrote in klineedit.cpp:63
> You mean this is emitted by Qt and this is duplicate one or so?

@anthonyfieroni no, the textEdited signal should not be emitted when textChanged happens... at least according to the documentation, textEdited should only happen when the user changes the text. In this case, we are directly emitting textEdited exactly when the user has not changed the text.

But yes, it is the only real change.

> dfaure wrote in klineedit.cpp:184
> Why did you rename the slot? I don't understand "textEmitted", and the signal is of course still called textChanged. Isn't the slot renaming just noise?

you are absolutely right! My mistake, I intended to rename it to textEdited, as this is what the signal we actually want to emit from QLineEdit is, and the name textEmitted is spurious.

I do think it should be renamed however, because the entire bug is that it should not be emitting textChanged in this case...

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/053e18c3/attachment.html>


More information about the Kde-frameworks-devel mailing list