KConfigSkeletonItem : allow to set a KconfigGroup to read and write items in nested groups
ClosedPublic

Authored by crossi on Jan 31 2020, 10:51 AM.

Details

Summary

Currently KConfgiSkeleton cannot manage item entry in subgroup, like

[General][Colors]
MyItem=foo

Example of use

// Generated Class with KConfig compiler, inherits KConfigSkeleton

KConfigGroup generalGroup( &config, "General" );
KConfigGroup colorsGroup = config.group( "Colors" )
myItem->setGroup(cg); // Now can take a KConfigGroup 
addItem(myItem, "MyItem");

Evolution of kconfig compiler will follow to consider this.

Diff Detail

Repository
R237 KConfig
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
crossi created this revision.Jan 31 2020, 10:51 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 31 2020, 10:51 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
crossi requested review of this revision.Jan 31 2020, 10:51 AM
crossi edited the summary of this revision. (Show Details)Jan 31 2020, 10:58 AM
crossi edited the summary of this revision. (Show Details)Jan 31 2020, 11:02 AM
ervin added inline comments.Jan 31 2020, 11:05 AM
src/core/kcoreconfigskeleton.h
86

s/bu/but/

97

Space before * not after.

crossi updated this revision to Diff 74756.Jan 31 2020, 11:15 AM

code style

ervin requested changes to this revision.Jan 31 2020, 1:45 PM

Sorry, couple of further style fixes I missed on the previous run

src/core/kcoreconfigskeleton.cpp
1634

Space before & not after

1639

Space before * not after

src/core/kcoreconfigskeleton.h
464

Space before * not after

This revision now requires changes to proceed.Jan 31 2020, 1:45 PM
crossi updated this revision to Diff 74799.Jan 31 2020, 3:19 PM

code style

crossi edited the summary of this revision. (Show Details)Feb 6 2020, 10:22 AM
ervin added a comment.Feb 12 2020, 1:06 PM

LGTM, giving a couple more days for @dfaure to react to it though.

dfaure requested changes to this revision.Feb 16 2020, 9:53 AM

Well, I'm not the KConfig maintainer, mdawson is :-)

src/core/kcoreconfigskeleton.h
89

Missing @since 5.68

96

Missing @since 5.68

455–456

So this KF6 TODO is to make those methods virtual? It's a bit unclear to me...

I'm asking because: if the solution is something else then maybe it can be done already for those new methods.

This revision now requires changes to proceed.Feb 16 2020, 9:53 AM
ervin added inline comments.Feb 16 2020, 1:38 PM
src/core/kcoreconfigskeleton.h
455–456

That's my understanding to have them virtuals in KF6 indeed, that was introduced by David Edmundson a little while ago. This is necessary because of the way KConfigSkeletonItem is done and because KConfigCompilerSignallingItem is basically a decorator.

crossi updated this revision to Diff 75829.Feb 17 2020, 1:59 PM

Comments

crossi marked 9 inline comments as done.Feb 17 2020, 2:02 PM
crossi added inline comments.
src/core/kcoreconfigskeleton.h
455–456

Added David's comment from D25791 to make things clearer on what to do.

crossi marked an inline comment as done.Feb 17 2020, 2:07 PM
dfaure accepted this revision.Feb 17 2020, 9:33 PM
dfaure added inline comments.
src/core/kcoreconfigskeleton.h
459

the end of the sentence is missing. ("should be fine")

Well, I'm not the KConfig maintainer, mdawson is :-)

Well, my understanding is that mdawson is MIA, so I rely on the old "dfaure as default maintainer" model. ;-)

ervin accepted this revision.Feb 24 2020, 11:14 AM

LGTM, please don't forget to address dfaure's comment before pushing though.

This revision is now accepted and ready to land.Feb 24 2020, 11:14 AM
crossi updated this revision to Diff 76281.Feb 24 2020, 12:23 PM

fix comment

crossi marked an inline comment as done.Feb 24 2020, 12:23 PM
ervin accepted this revision.Feb 24 2020, 12:43 PM
This revision was automatically updated to reflect the committed changes.