[Differential] [Requested Changes To] D4359: Add ClangClassHelper, to restore features of CppClassHelper

[Milian Wolff]

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


  some nitpicks, otherwise lgtm

INLINE COMMENTS

> clangclasshelper.cpp:72
> +                                                                  TopDUContext::AllDeclarationsAndContexts));
> +    DUChainReadLocker lock;
> +

scope this lock, i.e. free it before removing the file

> clangclasshelper.cpp:100
> +
> +    QMap<QString, VariableDescriptionList> variableDescriptions;
> +    QMap<QString, FunctionDescriptionList> functionDescriptions;

hashes, or do you really need this to be sorted?

> clangclasshelper.cpp:109
> +            signalDescriptions << function;
> +        }
> +        else if (function.isSlot) {

wrap

> clangclasshelper.cpp:112
> +            slotDescriptions[access] << function;
> +        }
> +        else {

wrap

> clangclasshelper.cpp:123
> +
> +    QMap<QString, VariableDescriptionList>::const_iterator vit, vend;
> +    vit = variableDescriptions.constBegin();

remove line

> clangclasshelper.cpp:124
> +    QMap<QString, VariableDescriptionList>::const_iterator vit, vend;
> +    vit = variableDescriptions.constBegin();
> +    vend = variableDescriptions.constEnd();

instead use auto vit = ...

> clangclasshelper.cpp:125
> +    vit = variableDescriptions.constBegin();
> +    vend = variableDescriptions.constEnd();
> +    for (; vit != vend; ++vit) {

and auto vend = ...

> clangclasshelper.cpp:130
> +
> +    QMap<QString, FunctionDescriptionList>::const_iterator fit, fend;
> +    fit = functionDescriptions.constBegin();

dito

> clangclasshelper.cpp:137
> +
> +    fit = slotDescriptions.constBegin();
> +    fend = slotDescriptions.constEnd();

dito

also, these three blocks can share code, please implement that

> clangclasshelper.cpp:191
> +    // strip access specifier
> +    QStringList splitBase = base.split(QLatin1Char(' '), QString::SkipEmptyParts);
> +

super inefficient. check if base starts with public/private/protected + space, if not, append "public "

> clangclasshelper.h:33
> +{
> +
> +public:

remove empty line

> clangclasshelper.h:39
> +    KDevelop::TemplateClassGenerator* createGenerator(const QUrl& baseUrl) override;
> +    QList< KDevelop::DeclarationPointer > defaultMethods(const QString& name) const override;
> +};

remove spaces in <...>

> clangclasshelper.h:74
> +     */
> +    QVariantHash extraVariables();
> +

const

> kdevclangsupport.json:57
>      ], 
> -    "X-KDevelop-Language": "C", 
> +    "X-KDevelop-Language": ["C", "C++"], 
>      "X-KDevelop-LoadMode": "AlwaysOn", 

separate commit, do so directly

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/7ee8b068/attachment-0001.html>