<table><tr><td style="">dweatherill added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D9808" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9808#inline-44539" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</span> wrote in <span style="color: #4b4d51; font-weight: bold;">klineedit_unittest.cpp:87</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">typo: if the text box is already clear*ed*</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">ok, it's unclear, but not a typo.<br />
According to QLineEdit documentation, clear() should only emit textChanged when there is actually text to clear.</p>
<p style="padding: 0; margin: 8px;">So the word "cleared" kind of implies "only after we've called clear()" I guess I should change it to</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">//if text box is already empty, calling clear() shouldn't emit</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9808#inline-44537" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">anthonyfieroni</span> wrote in <span style="color: #4b4d51; font-weight: bold;">klineedit.cpp:63</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">You mean this is emitted by Qt and this is duplicate one or so?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><a href="https://phabricator.kde.org/p/anthonyfieroni/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@anthonyfieroni</a> 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.</p>
<p style="padding: 0; margin: 8px;">But yes, it is the only real change.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9808#inline-44538" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">klineedit.cpp:184</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">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?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">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.</p>
<p style="padding: 0; margin: 8px;">I do think it should be renamed however, because the entire bug is that it should not be emitting textChanged in this case...</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R284 KCompletion</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9808" rel="noreferrer">https://phabricator.kde.org/D9808</a></div></div><br /><div><strong>To: </strong>dweatherill, Frameworks, dhaumann, cullmann<br /><strong>Cc: </strong>mwolff, dfaure, anthonyfieroni, iodelay, vbspam, njensen, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger<br /></div>