Changeset View
Standalone View
src/klineedit.cpp
Context not available. | |||||
60 | #ifndef KCOMPLETION_NO_DEPRECATED | 60 | #ifndef KCOMPLETION_NO_DEPRECATED | ||
---|---|---|---|---|---|
61 | emit q->userTextChanged(text); | 61 | emit q->userTextChanged(text); | ||
62 | #endif | 62 | #endif | ||
63 | emit q->textEdited(text); | | |||
anthonyfieroni: You mean this is emitted by Qt and this is duplicate one or so? | |||||
I agree with dfaure, isn't this line here the only "real" change? The extended unit test is good too of course. mwolff: I agree with dfaure, isn't this line here the only "real" change? The extended unit test is… | |||||
@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. dweatherill: @anthonyfieroni no, the textEdited signal should not be emitted when textChanged happens... at… | |||||
64 | } | 63 | } | ||
65 | } | 64 | } | ||
66 | 65 | | |||
Context not available. | |||||
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? dfaure: Why did you rename the slot? I don't understand "textEmitted", and the signal is of course… | |||||
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... dweatherill: you are absolutely right! My mistake, I intended to rename it to textEdited, as this is what… | |||||
actually, reading more carefully, I agree it should not be renamed, just the spurious emission of textEdited should go dweatherill: actually, reading more carefully, I agree it should not be renamed, just the spurious emission… | |||||
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) dfaure: yes, it's connected to textChanged, so it SHOULD be called _k_textChanged, otherwise it gets… |
You mean this is emitted by Qt and this is duplicate one or so?