[Differential] [Commented On] D1005: Added "move into source" helper

[mwolff (Milian Wolff)]

mwolff added a comment.


  overall really good work, thanks a lot Sergey! I was actually thinking of working on that the other day but got distracted. So excellent timing on your side :)
  
  I have a bunch of nitpicks, but overall this is fabulous work already. I guess most of my nitpicks actually are valid in the old code paths as well, so you are not to blame here. I just want to get this cleaned up before we get it in for clang.

INLINE COMMENTS
  languages/clang/codegen/simplerefactoring.cpp:133 afaik we have a "isHeader" check, no? If not, introduce it please
  languages/clang/codegen/simplerefactoring.cpp:134 the below should be moved into a helper function, "documentFinderHelpers::sourceForHeader(headerPath)"
  languages/clang/codegen/simplerefactoring.cpp:204     auto localDeclarations = funcCtx->localDeclarations();
      signature.reserve(localDeclarations.size());
  
  or even better, use `std::transform` with a `back_inserter` and a lambda for the conversion.
  languages/clang/codegen/simplerefactoring.cpp:264 shouldn't be required anymore afaik
  languages/clang/codegen/simplerefactoring.h:38 This name was (and still is) really ill-chosen. can you come up with something better? I'd go for ClangRefactoring.
  languages/clang/codegen/sourcemanipulation.cpp:77 don't we have such a function elsewhere already, e.g. for the "update signature" assistant? could we share code?
  languages/clang/codegen/sourcemanipulation.cpp:232 uhm can't we use the source formatter to do this work for us?
  languages/clang/codegen/sourcemanipulation.cpp:268 align with line above again
  languages/clang/codegen/sourcemanipulation.cpp:306 early return if == continue; to decrease indent level
  languages/clang/codegen/sourcemanipulation.h:48 align first const with line above
  languages/clang/codegen/sourcemanipulation.h:72 DUContextPointer?
  languages/clang/codegen/sourcemanipulation.h:74 ReferencedTopDUContext?
  languages/clang/duchain/builder.cpp:563 in upstream clang?

REPOSITORY
  rKDEVELOP KDevelop

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: skalinichev
Cc: mwolff, kfunk, kdevelop-devel, KDevelop, arrowdodger