D27133: kconfig_compiler : generate kconfig settings with subgroup

[Kevin Ottens]

ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kconfigcompiler_test.cpp:80
>      "test_signal",
> +    "test_notifiers",
>      "test_translation_kde",

This seems unrelated... So we had this test case available but it was in fact never run?

> test_subgroups.kcfg:10
> +    </kcfgfile>
> +    <group name="$(SubGroup)" parentGroup="$(GeneralGroup)">
> +        <entry name="Foo" type="Bool">

Now that I see it, I think I'd go for "parentGroupName" since this is not referential and really about the name (like the name parameter)

> test_subgroups.kcfg:20
> +    </group>
> +</kcfg>

Probably worth also having cases with:

- the group name being parameterized but not the parentGroup
- both name and parentGroup not be parameterized

Just to have all the possible combinations.

> KConfigSourceGenerator.cpp:354
>  
> +// TODO : Some compiler option won't work or generate bogus settings file.
> +// * Does not manage properly Notifiers=true kcfgc option for parameterized entries :

Unrelated right, this is not due to your patch, or am I confused?

> KConfigXmlParser.cpp:502
>  
> +    QString parentGroup = e.attribute(QStringLiteral("parentGroup"));
> +

I'd const auto it, or at least const.

REPOSITORY
  R237 KConfig

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

To: crossi, ervin, dfaure, #frameworks
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200305/94cb6339/attachment.html>