<table><tr><td style="">cullmann 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/D19446">View Revision</a></tr></table><br /><div><div><p>Hi,</p>

<p>I think this is an improvement.</p>

<p>Could you update the patch to current master?</p>

<p>For your questions:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Feature or bug? Select in block mode from right->left on a single line after last char a block. The brackets are exchanged )( and placed at the end of the block</li>
</ul>

<p>Hmm, I think this counts as a bug.</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Code use MovingRange, old(?) similar code fumble around with backup cursors, I hope not for some optimizing but only because its older than MovingRange</li>
</ul>

<p>I think this was just ancient code.<br />
I think for this action, there is no need to try to micro-optimized, the movingrange solution is more safe.</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">No use of toVirtualColumn or similar. It looks so far good anyway. Only when you mix tabs and spaces the result may sometimes odd. But I think there is no solution in using toVirtualColumn in the cases I had played around</li>
</ul>

<p>I would have thought that toVirtualColumn solves the issues if you have mixed tabs/spaces. Thought I am not that concerned if that is not perfect, given block selection + mixing tabs/spaces is always a mess.</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">No call of view->slotTextInserted (will only emit signal) can't see any effect and isn't such signal not already emit by the doc itself?</li>
</ul>

<p>Calls to that should be added. People might rely in the apps using the part that we emit that for "user-inserted" text, as the docs state.<br />
Should be no big issue, or? Just call that after each insertText operation, like before.</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Autotests looks OK, I may add some for the new feature when patch gets accepted so far</li>
</ul>

<p>That would be great!<br />
You can count this as accepted after the above pointed out things are altered.</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/D19446">https://phabricator.kde.org/D19446</a></div></div><br /><div><strong>To: </strong>loh.tar, KTextEditor<br /><strong>Cc: </strong>cullmann, kwrite-devel, kde-frameworks-devel, KTextEditor, gennad, domson, michaelh, ngraham, bruns, demsking, sars, dhaumann<br /></div>