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

Reply via email to