D26202: WIP: Refactor KConfigXT

dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Please remove the "WIP" from the commit log (and from phabricator, which only gets updated if you do `arc diff --verbatim`) REPOSITORY R237 KConfig REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. Aparently the kmymoney issue was the same: empty kconfig file. I just successfully compiled kdevelop and kmymoney. I'll let the computer to compile the whole kde applications from scratch tonigth to see if it will fail somewhere. REPOSITORY R237 KConfig

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73846. tcanabrava added a comment. - Revert "Revert "WIP: Refactor KConfigXT"" - Add Reference files for Broken KDevelop Configuration - Fix generating of empty configuration files REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. who knew? This actually was not a false positive: the kdevelop build failure was a bug in kdevelop. I already opened a ticket: https://invent.kde.org/kde/kdevelop/merge_requests/90 but at the same time I added code to handle the case of broken / empty

D26202: WIP: Refactor KConfigXT

kossebau added a comment. In D26202#595817 , @kossebau wrote: > where "full" would also need to mean "clean fresh build dirs" :) so that the codegenerator is triggered to be run, as the current cmake code seems to not inject a build dep between

D26202: WIP: Refactor KConfigXT

cgiboudeaux added a comment. In D26202#595846 , @tcanabrava wrote: > I Just reverted this and I'm working on a full build of kde using kdesrc-build --refresh-build, I'll reopen this patch when *all* projects build sucessfully, with a unittest

D26202: WIP: Refactor KConfigXT

tcanabrava reopened this revision. tcanabrava added a comment. This revision is now accepted and ready to land. Reopening for review :) REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: kossebau, bport,

D26202: WIP: Refactor KConfigXT

tcanabrava requested review of this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: kossebau, bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. In D26202#595820 , @tcanabrava wrote: > We can revert, and I’ll fix the full build. Doing that now. I Just reverted this and I'm working on a full build of kde using kdesrc-build --refresh-build, I'll reopen

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. We can revert, and I’ll fix the full build. Doing that now. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: kossebau, bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2,

Re: D26202: WIP: Refactor KConfigXT

We can revert, and I’ll fix the full build. Doing that now. On Fri, 17 Jan 2020 at 09:03 Friedrich W. H. Kossebau < nore...@phabricator.kde.org> wrote: > kossebau added a comment. View Revision > > > where "full" would also need to mean "clean fresh build

D26202: WIP: Refactor KConfigXT

kossebau added a comment. where "full" would also need to mean "clean fresh build dirs" :) so that the codegenerator is triggered to be run, as the current cmake code seems to not inject a build dep between generator instance and generated files, so a new version does not trigger

D26202: WIP: Refactor KConfigXT

dfaure added a comment. Argh, I wanted to say "please do a full kdesrc-build of all modules" before approving, and forgot to do that. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: kossebau, bport,

D26202: WIP: Refactor KConfigXT

kossebau added a comment. This morning it was found e.g. on opensuse build server (but reproduced by me locally) that this change broke KMyMoney & KDevelop, who now start to fail e.g. with (using `VERBOSE=1 make`) [ 38%] Generating kcfg_custombuildsystemconfig.h,

D26202: WIP: Refactor KConfigXT

This revision was automatically updated to reflect the committed changes. Closed by commit R237:98c32e29f504: WIP: Refactor KConfigXT (authored by tcanabrava). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73608=73688 REVISION DETAIL

D26202: WIP: Refactor KConfigXT

dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R237 KConfig BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2,

D26202: WIP: Refactor KConfigXT

tcanabrava added inline comments. INLINE COMMENTS > dfaure wrote in kconfigcompiler_test.cpp:129 > OK, this needs a hint for the person debugging regressions then. Something > like > > QVERIFY2(content == contentRef, "Failure, see foo.diff"); This is done now within the appendFileDiff

D26202: WIP: Refactor KConfigXT

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

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73608. tcanabrava added a comment. - Rebase REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73557=73608 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTED FILES

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73557. tcanabrava added a comment. - Add missing copyright holders REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73510=73557 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. Took the time to nuke the SignalArguments and use Param instead, easier than I initially tougth. INLINE COMMENTS > dfaure wrote in kconfigcompiler_test.cpp:180 > I meant QVERIFY2(diffFile.open(...), ...). > No need to make a separate call to isOpen(). ups :)

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73510. tcanabrava marked 10 inline comments as done. tcanabrava added a comment. - Simplify List creation - Fail with the diff file name - Simplify Code - Fix Typo - Remove `SignalArgument` for `Param` - Nitpicks and Include fixes REPOSITORY

D26202: WIP: Refactor KConfigXT

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > tcanabrava wrote in kconfigcompiler_test.cpp:129 > the text was really not friendly, it was a big blob of diff content pasted on > screen. Considering that

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. @dfaure , @ervin Right now the code passes all the tests and I tried to be extra careful not to break away with the general architecture. I think it's the first "safe" version to review. REPOSITORY R237 KConfig REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73345. tcanabrava added a comment. - Fix bug ediging param variable that's supposed to be const - Const Correctness: REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73336=73345 BRANCH arcpatch-D26202

D26202: WIP: Refactor KConfigXT

tcanabrava marked 8 inline comments as done. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26202: WIP: Refactor KConfigXT

tcanabrava added inline comments. INLINE COMMENTS > tcanabrava wrote in kconfig_compiler.cpp:753 > that was a bit harder than I want, but done. Inside of the code generation > there was code that manipulated the ParseResult. I think this is one of the > good spots that show that this rewrite

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73336. tcanabrava marked 4 inline comments as done. tcanabrava added a comment. - Rename KConfigCodeGenerator to KconfigCodeGeneratorBase - Simplify diff file saving - Add License Headers - Documentation, Privatization, Unused method / variable

D26202: WIP: Refactor KConfigXT

tcanabrava marked 22 inline comments as done. tcanabrava added inline comments. INLINE COMMENTS > dfaure wrote in kconfigcompiler_test.cpp:129 > Why did you remove this? It's just a more user-friendly version of the > QVERIFY on the next line, so it can't possibly have failed while the next >

D26202: WIP: Refactor KConfigXT

dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kconfigcompiler_test.cpp:129 > -// use split('\n') to avoid > -// the whole output shown inline > -QCOMPARE(content.split('\n'),

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. Looking at the review you it's a bis strange to see that I'v touched some test reference generated code, I did this only on *whitespace only changes* where the new whitespace made more sense than the old one. there is *one* bug I havent fixed on this code that I

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73310. tcanabrava added a comment. - Fix whitespace on the reference code / genreator REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73301=73310 BRANCH arcpatch-D26202 REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73301. tcanabrava added a comment. - More whiteespaces fixed - Fix *space only* issues with the test reference headers - More whitespace fixes, only six files to go - Way more tests passing - space fixes REPOSITORY R237 KConfig CHANGES SINCE

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73292. tcanabrava added a comment. - Fix many small whitespace issues REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=73290=73292 BRANCH arcpatch-D26202 REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 73290. tcanabrava added a comment. - Rebase on master REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=72715=73290 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTED

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72715. tcanabrava edited the test plan for this revision. tcanabrava added a comment. - Add Hack for Enums - Fix Whitespace diff - Move some raw scopes to start/end scope() REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D26202: WIP: Refactor KConfigXT

tcanabrava edited the summary of this revision. tcanabrava edited the test plan for this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2,

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72711. tcanabrava added a comment. - Add missing .h - Fixes placement of code - Change how tests save tests - Change how the Indentation is done - Fix filename in the preamble - Separate logic to make function readable - Fix more newlines

D26202: WIP: Refactor KConfigXT

dfaure added a reviewer: dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: bport, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26202: WIP: Refactor KConfigXT

tcanabrava added a subscriber: bport. tcanabrava added a comment. Not really as all the calls are the same, just split into classes and logical bits. I haven’t write a single line of logic. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava,

Re: D26202: WIP: Refactor KConfigXT

Not really as all the calls are the same, just split into classes and logical bits. I haven’t write a single line of logic. On Mon, 30 Dec 2019 at 19:00 Nathaniel Graham wrote: > ngraham added a comment. View Revision > > > At this point, it seems more like

D26202: WIP: Refactor KConfigXT

ngraham added a comment. At this point, it seems more like a total rewrite than a refactor... REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72377. tcanabrava added a comment. - Reorganize code - Fix filename - Fix Static Methods - Fix name generation - Call forgotten function in the right place - Fix signal generation - remove stray bit of code REPOSITORY R237 KConfig CHANGES

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72371. tcanabrava added a comment. - Regression: Remove space from the autogen preamble - Whitespace regression - Fix hasNamespace: logic was inverted - Space issues and scope - Use whitespace() function to manage indentation level - Space

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72349. tcanabrava added a comment. - Fix some obvious bugs. REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=72338=72349 BRANCH megaRefactor REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTED

D26202: WIP: Refactor KConfigXT

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

D26202: WIP: Refactor KConfigXT

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

D26202: WIP: Refactor KConfigXT

ngraham added reviewers: Frameworks, ervin, bport. ngraham added a comment. That explanation really belongs in the Description section of this patch, not in a comment. :) REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin,

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72338. tcanabrava added a comment. - Move XmlParser to another file. - Move Code around again - Fix some issues - still broken - Move Parser to own file - Move a lot of code to the Parser Class - Fix a few of the many many warnings.

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72326. tcanabrava added a comment. - Add new Class, KConfigSourceGenerator, fix usage of addHeader - Move CreateSingletonImplementation - Create Cpp Preamble - Move Constructor implementation - move Getter / Setter / Destructor - Remove unused

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72171. tcanabrava added a comment. - Finish header / Move basename to the ParameterConfiguration REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26202?vs=72166=72171 BRANCH megaRefactor REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava updated this revision to Diff 72166. tcanabrava added a comment. - Port more header functions - Move CreateItemAcessors and CreateSignals - Remove indentation level - Move NonDPointerMembers and DPointerMembers REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D26202: WIP: Refactor KConfigXT

tcanabrava added a comment. In D26202#582541 , @ngraham wrote: > > This is really a wip, completely broken. It's just so people can point me to the right direction. > > Maybe could you clarify which direction you want to be pointed in?

D26202: WIP: Refactor KConfigXT

ngraham added a comment. > This is really a wip, completely broken. It's just so people can point me to the right direction. Maybe could you clarify which direction you want to be pointed in? What's the goal here? What do you need help with? REPOSITORY R237 KConfig REVISION DETAIL

D26202: WIP: Refactor KConfigXT

tcanabrava created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. tcanabrava requested review of this revision. REVISION SUMMARY This is really a wip, completely broken. It's just so people can point me to the right direction. Rework