[Differential] [Updated] D4359: Add ClangClassHelper, to restore features of CppClassHelper

[Friedrich W. H. Kossebau]

kossebau marked 13 inline comments as done.
kossebau added inline comments.

INLINE COMMENTS

> arrowdodger wrote in clangclasshelper.cpp:45
> `const`?

Cannot do, see `ICreateClassHelper::createGenerator(QUrl)`.

> arrowdodger wrote in clangclasshelper.cpp:52
> While `QTemporaryFile` guarantees that the file name will be unique, how about using `name` parameter in it? Something like `QLatin1String("/") + name + QLatin1String("class_XXXXXX.cpp")`. That might ease debugging in some cases.

Sounds like a good idea. That technique might be also deployed to other QTemporaryFile usages in KDevelop.

> arrowdodger wrote in clangclasshelper.cpp:53
> Why do we need it?

Good question, that was in the old CppClassHelper code. Will add a TODO for now only, because perhaps this whole block can be dropped and clang directly provides the wanted info about default methods, and I would first spend time on that :)
Perhaps some porting left-over from KTempFile->QTemporaryFile, like also the remove below.

> mwolff wrote in clangclasshelper.cpp:137
> dito
> 
> also, these three blocks can share code, please implement that

No instant idea how to add code sharing without complicating things for the code reader. What did you think of? Be aware that first of 3 loops is over another type of container, which would ask for template code?

The unrolled code seems more easy to grasp fpr me compared to what I could envision, but eager to learn better code :)

> mwolff wrote in clangclasshelper.cpp:191
> super inefficient. check if base starts with public/private/protected + space, if not, append "public "

Looking more closely it seems this whole method is not even called from any place, so removing instead.

> arrowdodger wrote in clangclasshelper.h:5
> Proper copyrights.

I only did s/Cpp/Clang/g in this file, so nothing worth a copyright, no? :)

> mwolff wrote in kdevclangsupport.json:57
> separate commit, do so directly

Okay, will do.

REPOSITORY
  R32 KDevelop

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

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

To: kossebau, #kdevelop, mwolff
Cc: mwolff, arrowdodger, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170131/0a17b59d/attachment.html>