D16326: Replace leading typed text when completing function implementation

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  lgtm in general, three small nitpicks and I'll have a look at the broken unit test now too.

INLINE COMMENTS

> context.cpp:109
>  
> +QStringRef trimLeft(const QStringRef string) {
> +    const int position = std::find_if_not(string.begin(), string.end(),

please move this to stringhelpers.h/cpp in kdevplatform/language/duchain, call it `stripLeadingWhitespace` or similar

> context.cpp:112
> +                            [](QChar c){ return c.isSpace(); }) - string.begin();
> +    return string.mid(position);
> +}

this should return `midRef`

> context.cpp:348
>      {
> -        view->document()->replaceText(word, m_replacement);
> +        KTextEditor::Document* const document = view->document();
> +

`const auto* document = `

> context.cpp:350
> +
> +        // try and replace leading typed text that match the proposed implementation
> +        const QStringRef leading = trimLeft(document->line(word.end().line()).leftRef(word.end().column()));

I agree that we should integrate this proposed feature for now, but want to mention that it can easily fail. Most notably, it will fail when

- someone uses the boost coding style with the return type on its own line
- whitespace mismatch, e.g. someone typed `foo*` but the code completion wants to insert `foo *`

but as I said, your new behavior is already better than before, we can improve it later as needed

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D16326

To: amhndu, #kdevelop, mwolff
Cc: mwolff, apol, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20181021/434e395c/attachment-0001.html>