ervin added a comment.

  A few smallish issues only, otherwise LGTM.

INLINE COMMENTS

> KConfigSourceGenerator.cpp:316
>  {
> -   stream() << "  " << itemPath(entry, cfg()) << " = "
> +    QString innerItemVarStr(innerItemVar(entry, cfg()));
> +    if (!entry->signalList.isEmpty()) {

I'd const it and also use the = style of initialization which I find more 
readable.

> KConfigSourceGenerator.cpp:354
> +        QString argBracket = QStringLiteral("[%1]").arg(i);
> +        QString innerItemVarStr = innerItemVar(entry, cfg()) + argBracket;
>  

const those please

> KConfigSourceGenerator.cpp:365
> +
> +        QString itemVarStr(itemPath(entry, cfg()) + argBracket);
> +

ditto

> kconfig_compiler.cpp:397
>  {
> -    if (cfg.itemAccessors) {
> -        return QString();
> +    QString type = cfg.inherits + "::Item" + itemType(e->type);
> +

const

> kconfig_compiler.cpp:401
> +    fCap[0] = fCap[0].toUpper();
> +    QString argSuffix = (!e->param.isEmpty()) ? 
> (QStringLiteral("[%1]").arg(e->paramMax + 1)) : QString();
> +    QString result;

const

> kconfig_compiler.cpp:462
>  
> -QString newItem(const CfgEntry* entry, const QString &key, const QString& 
> defaultValue,
> +QString newItemInner(const CfgEntry *entry, const QString &key, const 
> QString &defaultValue,
>                  const KConfigParameters &cfg, const QString &param) {

Should be named newInnerItem

REPOSITORY
  R237 KConfig

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

To: hchain, meven, crossi, ervin, bport, tcanabrava
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

Reply via email to