mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed.
we override execution in our own completion models, so this patch will only change the behavior for the builtin word and keyword completion models in ktexteditor I believe that said, I think it makes sense to insert the word everywhere in block selection, it shouldn't be different from typing text. so +1 for the idea, but -1 on the actual implementation: - we need to have a unit test for this new behavior - we should introduce new helper API to make it easier to opt-in to this new behavior and reduce the if/else depth. This would also make it easier for us in KDevelop to change our behavior accordingly. I believe the code completion execution code should basically be agnostic to the block selection mode. I.e. instead of the proposed if (completeBlockSelection) { removeText typeChars } else { replaceText } it should always just call "replaceText" with the the block selection range and then internal API should duplicate the text, if the selection is a block selection as-is, this patch adds too much code duplication REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D18793 To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop, mwolff Cc: mwolff, kde-frameworks-devel, kwrite-devel, #ktexteditor, gennad, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann