<table><tr><td style="">ahmadsamir added a comment.
</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/D18793">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D18793#437220" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D18793#437220</a>, <a href="https://phabricator.kde.org/p/cullmann/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@cullmann</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>Hi, I still don't like that we do different things in replaceText depending on the selection of the potential activeView, that makes this function harder to use correctly, as it might magically do something the user might not expect.</p></div>
</blockquote>

<p>If the code is moved to say insertText it'd be the same problem, just further down the line.</p>

<p>I could overload replaceText, with the overload taking a view parameter with the responsibility of deciding which replaceText to use falling on the caller; (which is something KDevelop wants to avoid to ease their integration). The thing is, this piece of code needs a home somewhere.</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">replaceText shouldn't use typeChars because the latter is interactive by design</li>
<li class="remarkup-list-item">replaceText shouldn't be over-clever and do something different/unexpected depending on the selection in the activeView()</li>
</ul>

<p>The work of inserting the completion falls on executeCompletionItem, so it should be handled there; also note that executeCompletionItem will become more complicated anyway, because of the remove-tail functionality...</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18793">https://phabricator.kde.org/D18793</a></div></div><br /><div><strong>To: </strong>ahmadsamir, KTextEditor, cullmann, dhaumann, mwolff<br /><strong>Cc: </strong>loh.tar, kde-frameworks-devel, kwrite-devel, KTextEditor, gennad, domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann<br /></div>