D27463: KconfigXT: Add a value attribute to Enum field choices

This revision was automatically updated to reflect the committed changes. Closed by commit R237:ec207330d5bd: KconfigXT: Add a value attribute to Enum field choices (authored by meven). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=77225=77226

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 77225. meven added a comment. Rebase on master REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=77077=77225 BRANCH arcpatch-D27463_2 REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin accepted this revision. REPOSITORY R237 KConfig BRANCH arcpatch-D27463_2 REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 77077. meven added a comment. Fix typo REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=77064=77077 BRANCH arcpatch-D27463_2 REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin accepted this revision. ervin added a comment. This revision is now accepted and ready to land. LGTM, please fix the typo in the docs before pushing though INLINE COMMENTS > README.dox:372 > > +Since 5.68, if present the value attribute will be used used as the > choice value

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 77064. meven marked 2 inline comments as done. meven added a comment. Fix comment and formatting REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=77006=77064 BRANCH arcpatch-D27463_2 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcoreconfigskeleton.cpp:581 > +// HACK for BC concerns > +// TODO KF6: remove KCoreConfigSkeletonPrivate::mValues and add a value > field to

D27463: KconfigXT: Add a value attribute to Enum field choices

meven marked 4 inline comments as done. meven added inline comments. INLINE COMMENTS > ervin wrote in kcoreconfigskeleton.cpp:580 > No I meant the comment needs to be adjusted due to the changes (fields > changing place and such) sorry if I was unclear. I updated the comment since your first

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 77006. meven added a comment. Fix typo REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=76999=77006 BRANCH arcpatch-D27463_2 REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin added inline comments. INLINE COMMENTS > meven wrote in kcoreconfigskeleton.cpp:580 > You mean I should prepare this and put it in KF6 waiting for merge queue ? No I meant the comment needs to be adjusted due to the changes (fields changing place and such) sorry if I was unclear. >

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added inline comments. INLINE COMMENTS > ervin wrote in kcoreconfigskeleton.cpp:580 > Will need an update You mean I should prepare this and put it in KF6 waiting for merge queue ? > ervin wrote in kcoreconfigskeleton.h:788 > const QString > > Probably worth adding a KF6 comment

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76999. meven marked 9 inline comments as done. meven added a comment. Remove enumValues, const ref args, move mValues to KCoreConfigSkeletonItemPrivate, improve comments REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcoreconfigskeleton.cpp:580 > +{ > +// TODO KF6 move value to ItemEnum::Choice and remove > KCoreConfigSkeleton::ItemEnum::mValues > +const auto inHash

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added a comment. I fixed an issue when testing this with D27477 and it is fixed and the issue it revealed is tested. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc:

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76866. meven added a comment. Improve naming, fix case of normal entries, adding a test for this case REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=76856=76866 BRANCH arcpatch-D27463_1 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76856. meven added a comment. Fix no value set case REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=76849=76856 BRANCH arcpatch-D27463_1 REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76849. meven added a comment. Move values attribute for ItemEnum::Choice to KCoreConfigSkeleton::ItemEnum::ItemEnum::mValues to preserve Binary compatiblity REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D27463: KconfigXT: Add a value attribute to Enum field choices

meven marked an inline comment as done. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76446. meven added a comment. Move construct of QRegularExpression out of loop REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=76405=76446 BRANCH arcpatch-D27463 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ervin wrote in kcoreconfigskeleton.h:764 > Oh right, stupid me, this obviously breaks binary compatibility, we need to > find another way to store this. Most

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin added inline comments. INLINE COMMENTS > meven wrote in kcoreconfigskeleton.h:764 > Something I have noticed while testing this. > Since it changes the memory of a very common data struct in a very common > lib, it creates a lot of crashes if apps are not compiled with the installed >

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added inline comments. INLINE COMMENTS > kcoreconfigskeleton.h:764 > QString whatsThis; > +QString val; > + Something I have noticed while testing this. Since it changes the memory of a very common data struct in a very common lib, it creates a lot of crashes if

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76405. meven marked 6 inline comments as done. meven added a comment. Move KCoreConfigSkeleton::ItemEnum::Choice::value to cpp file, use a regex to filter valid choice name REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D27463: KconfigXT: Add a value attribute to Enum field choices

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcoreconfigskeleton.h:766 > + > +public: > +QString value() const { It's a struct you can drop the public: here. >

D27463: KconfigXT: Add a value attribute to Enum field choices

crossi added inline comments. INLINE COMMENTS > meven wrote in KConfigXmlParser.cpp:203 > If you don't mind I am putting aside the second part as it is not directly > related to this PR, i.e name attribute validation. (FYI we had > KConfigXmlParser::validateNameAndKey) I guess there are more

D27463: KconfigXT: Add a value attribute to Enum field choices

meven edited the summary of this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc: ngraham, davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 76040. meven marked an inline comment as done. meven added a comment. Warn user about / . : being forbidden in choice's attribute name REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75969=76040 BRANCH

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added inline comments. INLINE COMMENTS > bport wrote in KConfigXmlParser.cpp:203 > Can we do a test on more than ' ' value, proably solve other case. > And we probably want to limit what is a correct value, any string seems a bit > too large, some character can be problematic when we

D27463: KconfigXT: Add a value attribute to Enum field choices

bport added inline comments. INLINE COMMENTS > KConfigXmlParser.cpp:203 > } > +else if (choice.name.contains(QLatin1Char(' '))) { > +std::cerr << "Tag attribute 'name' cannot contain a > space. You can use attribute 'value' to pass any string as the choice value."

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75969. meven added a comment. Add a since to dox REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75968=75969 BRANCH arcpatch-D27463 REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75968. meven added a comment. Add documentation about value in dox REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75967=75968 BRANCH arcpatch-D27463 REVISION DETAIL https://phabricator.kde.org/D27463

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added a comment. In D27463#613961 , @davidre wrote: > Could you also add documentation to whatever https://api.kde.org/frameworks/kconfig/html/kconfig_compiler.html is generated from? Thanks for pointing it out. I meant to

D27463: KconfigXT: Add a value attribute to Enum field choices

davidre added a comment. Could you also add documentation to whatever https://api.kde.org/frameworks/kconfig/html/kconfig_compiler.html is generated from? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc:

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75967. meven added a comment. Ignore empty value attribute for choice REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75966=75967 BRANCH arcpatch-D27463 REVISION DETAIL https://phabricator.kde.org/D27463

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75966. meven added a comment. Default value must refere to the choice name REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75964=75966 BRANCH arcpatch-D27463 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75964. meven marked 2 inline comments as done. meven added a comment. Clean garbage files REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75917=75964 BRANCH arcpatch-D27463 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

ngraham added inline comments. INLINE COMMENTS > CTestCostData.txt:1 > +--- Seems like this file was added in error > LastTest.log:3 > +-- > +End testing: Jan 29 14:08 CET Seems like this file was added in error REPOSITORY R237

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75917. meven added a comment. Default value must refere to the choice name REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75852=75917 BRANCH arcpatch-D27463 REVISION DETAIL

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added a dependent revision: D27477: KCM/Kwinoptions: Port title bar and window actions tabs UI and conf to KConfigXT. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc: davidre, kde-frameworks-devel, LeGast00n,

D27463: KconfigXT: Add a value attribute to Enum field choices

meven added a comment. In D27463#613114 , @davidre wrote: > If you touch the xsd do you neeed to increase the version number? That would be a good practice but how to publish a 1.1 version ? And what is the point ? I mean this xsd and

D27463: KconfigXT: Add a value attribute to Enum field choices

davidre added a comment. If you touch the xsd do you neeed to increase the version number? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27463 To: meven, ervin, bport, crossi, #frameworks Cc: davidre, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh,

D27463: KconfigXT: Add a value attribute to Enum field choices

meven updated this revision to Diff 75852. meven added a comment. update xsd REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27463?vs=75837=75852 BRANCH master REVISION DETAIL https://phabricator.kde.org/D27463 AFFECTED FILES

D27463: KconfigXT: Add a value attribute to Enum field choices

meven created this revision. meven added reviewers: ervin, bport, crossi, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. meven requested review of this revision. REVISION SUMMARY Allow to write choices such as : TEST