D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

tcanabrava added a comment. +1. @cfeck do you think this can land now? REPOSITORY R236 KWidgetsAddons BRANCH named_color_support REVISION DETAIL https://phabricator.kde.org/D29502 To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham Cc: cblack, broulik, cfeck,

D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

tcanabrava added inline comments. INLINE COMMENTS > kcolorcombo.cpp:244 > +{ > +QList namedColors; > +for (int colorIndex = 0; colorIndex < colors.count(); colorIndex++) { namedColors.reserve(colors.size()); > kcolorcombo.cpp:245 > +QList namedColors; > +for (int colorIndex =

D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

tcanabrava added a comment. In D29502#665582 , @cfeck wrote: > Does the delegate ensure the text is rendered in a color visible over the colored background? not yet, I talked to gustavo and he's working in an updated version of the

Re: D26877: Simplify calls to whitespace() and use it in more places.

I like indentedStream. On Sun, 2 Feb 2020 at 11:36 David Faure wrote: > dfaure requested changes to this revision. > dfaure added a comment. > This revision now requires changes to proceed. View Revision > > > I don't like it either. It doesn't "read" well.

D26877: Simplify calls to whitespace() and use it in more places.

tcanabrava added a subscriber: ervin. tcanabrava added a comment. I like indentedStream. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26877 To: tcanabrava, dfaure, ervin Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26133: Enable Auto Save

tcanabrava added a comment. > That's why personally I think it's fine to assume people need to opt-in for GenerateProperties if they want the feature, it's closely related after all. I disagree here. my application can be a simple QWidget based and have a KConfigXT enabled

D26868: Move newItem to private method in KConfigSourceGenerator

tcanabrava updated this revision to Diff 74600. tcanabrava added a comment. - Fix code style REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26868?vs=74234=74600 BRANCH simplify_newEntry REVISION DETAIL https://phabricator.kde.org/D26868 AFFECTED

D26868: Move newItem to private method in KConfigSourceGenerator

tcanabrava added a comment. In D26868#602162 , @kossebau wrote: > In D26868#602150 , @dfaure wrote: > > > There is indeed a QString overload for concatenating QLatin1String, but it will have to be

D26127: Simplify cppType method: Return Early, Use a Map and Assert.

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26127 To: tcanabrava, ervin Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26128: Simplify defaultValue method: Return Early, Use Default Initialization, and Assert.

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26128 To: tcanabrava, ervin Cc: ervin, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26126: Simplify param method: Return Early, Use a Map and Assert.

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26126 To: tcanabrava, ervin, dfaure Cc: dfaure, ervin, GB_2, apol, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D26130: Simplify return logic

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26130 To: tcanabrava, patrickelectric, ervin Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26129: Remove Iterator based loops for range based loops

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26129 To: tcanabrava, ervin Cc: ervin, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26876: Remove the Enum hack: finish lists with a comma, it's valid c++

tcanabrava added a comment. update / rebased. INLINE COMMENTS > ervin wrote in KConfigHeaderGenerator.cpp:252 > I lost track of constness here so I might be wrong, shouldn't this use > qAsConst in this context? yeah. REPOSITORY R237 KConfig BRANCH remove_enum_hack REVISION DETAIL

D26876: Remove the Enum hack: finish lists with a comma, it's valid c++

tcanabrava updated this revision to Diff 74596. tcanabrava marked an inline comment as done. tcanabrava added a comment. - Use qAsConst REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26876?vs=74253=74596 BRANCH remove_enum_hack REVISION DETAIL

D26368: Add an isImmutable to know if a property is immutable

tcanabrava added a comment. it's a +1 for me. @dfaure and @ervin ? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26368 To: meven, ervin, #frameworks Cc: dfaure, tcanabrava, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26368: Add an isImmutable to know if a property is immutable

tcanabrava added a comment. I like it, but considering that this adds a new method, I'd like to see it exposed to Qml too o the generated code if GenerateProperties is set to true, currently we write this kind of code in Qml: enabled:

D26573: Add missing Import Env Variable

This revision was automatically updated to reflect the committed changes. Closed by commit R240:ee0531749057: Add missing Import Env Variable (authored by tcanabrava). REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26573?vs=74512=74513 REVISION

D26573: Add missing Import Env Variable

tcanabrava updated this revision to Diff 74512. tcanabrava added a comment. - Fix Variable REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26573?vs=73237=74512 BRANCH add_missing_env_var REVISION DETAIL https://phabricator.kde.org/D26573

D26961: Be more helpfully verbose when we can't start an action

tcanabrava accepted this revision. This revision is now accepted and ready to land. REPOSITORY R283 KAuth BRANCH master REVISION DETAIL https://phabricator.kde.org/D26961 To: davidedmundson, tcanabrava Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26133: Enable Auto Save

tcanabrava added a comment. I agree with the comments, but I'm a bit lost on how to implement that in KCoreConfigSkeleton: the isSaveNeeded reads the value of the variable and return if it's different from the reference variable. (that I tougth it was a reference *value*, to find out that

D26131: Braces around for, break early.

tcanabrava added reviewers: ervin, dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26131 To: tcanabrava, ervin, dfaure Cc: ervin, patrickelectric, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26867: New class: KConfigTypeInformation

tcanabrava added reviewers: ervin, dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26867 To: tcanabrava, ervin, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26876: Remove the Enum hack: finish lists with a comma, it's valid c++

tcanabrava added reviewers: ervin, dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26876 To: tcanabrava, ervin, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26868: Move newItem to private method in KConfigSourceGenerator

tcanabrava added reviewers: ervin, dfaure. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26868 To: tcanabrava, ervin, dfaure Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26877: Simplify calls to whitespace() and use it in more places.

tcanabrava added reviewers: dfaure, ervin. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26877 To: tcanabrava, dfaure, ervin Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26131: Braces around for, break early.

tcanabrava updated this revision to Diff 74258. tcanabrava added a comment. - use std::any_of REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26131?vs=71913=74258 BRANCH arcpatch-D26131 REVISION DETAIL https://phabricator.kde.org/D26131 AFFECTED

D26877: Simplify calls to whitespace() and use it in more places.

tcanabrava created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. tcanabrava requested review of this revision. REVISION SUMMARY whitespace now returns a textStream so we don't need to stream() << whitespace() Use whitespace where we had

D26876: Remove the Enum hack: finish lists with a comma, it's valid c++

tcanabrava created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. tcanabrava requested review of this revision. TEST PLAN Run tests REPOSITORY R237 KConfig BRANCH remove_enum_hack REVISION DETAIL https://phabricator.kde.org/D26876

D26868: Move newItem to private method in KConfigSourceGenerator

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 only used here, should only be exposed here. TEST PLAN Run unittests REPOSITORY R237 KConfig

D26867: New class: KConfigTypeInformation

tcanabrava updated this revision to Diff 74231. tcanabrava added a comment. - Fix position of reference on parameter REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26867?vs=74224=74231 BRANCH rewrite_maps REVISION DETAIL

D26867: New class: KConfigTypeInformation

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 class handles transformations and queries for xml types to cpp types. Use KConfigTypeInformation to

D26202: Refactor KConfigXT

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

D26202: Refactor KConfigXT

tcanabrava updated this revision to Diff 74118. tcanabrava marked an inline comment as done. tcanabrava retitled this revision from "Refactor KConfigXT " to "Refactor KConfigXT". tcanabrava added a comment. - Rebase REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE

D26768: Revert "Revert "WIP: Refactor KConfigXT""

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26768 To: tcanabrava Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26202: Refactor KConfigXT

tcanabrava marked 26 inline comments as done. tcanabrava added inline comments. INLINE COMMENTS > ervin wrote in KConfigCodeGeneratorBase.cpp:80 > This is an odd indentation logic, is it to stay close to the original? (which > I'd actually support, just trying to understand where that's coming

D26202: Refactor KConfigXT

tcanabrava added a comment. @ervin since they are just nitpicks I'll fix in code and land the patch. REPOSITORY R237 KConfig BRANCH rework_kconfig_compiler REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: davidre, bcooksley,

D26202: Refactor KConfigXT

tcanabrava added a comment. In D26202#597044 , @davidre wrote: > I checked https://cgit.kde.org/kconfig.git/tree/src/kconfig_compiler/kcfg.xsd and it says > > > > >

Re: D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

do see a warning on a linux system about windows or mac run time issues (and missing icons is a run time issue). On Sun, 19 Jan 2020 at 12:03 Tomaz Canabrava wrote: > That’s not a developer issue, it’s a packaging issue. > > On Sun, 19 Jan 2020 at 12:02 Christophe Giboudeaux

D26202: Refactor KConfigXT

tcanabrava added a comment. @dfaure I tried to remove the `WIP` from the history but I'm worried that will force git push --force as I merged this with the wip before. any hints? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks,

D26202: Refactor KConfigXT

tcanabrava retitled this revision from "WIP: Refactor KConfigXT" to "Refactor KConfigXT ". REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26202 To: tcanabrava, #frameworks, ervin, bport, dfaure Cc: cgiboudeaux, kossebau, bport, ngraham, kde-frameworks-devel,

D26768: Revert "Revert "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 reverts commit 5f8c2ce63499d05dfb4753eb1acc21dccf21d434

D26766: Revert "Revert "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 Simplify If-Else chain inside of defaultValue function Use a type collection to verify which value we should

D26766: Revert "Revert "WIP: Refactor KConfigXT""

tcanabrava abandoned this revision. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26766 To: tcanabrava Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

tcanabrava added a comment. That’s not a developer issue, it’s a packaging issue. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D26752 To: patrickelectric, apol, tcanabrava, cgiboudeaux Cc: apol, cgiboudeaux, kde-frameworks-devel, kde-buildsystem,

Re: D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

That’s not a developer issue, it’s a packaging issue. On Sun, 19 Jan 2020 at 12:02 Christophe Giboudeaux < nore...@phabricator.kde.org> wrote: > cgiboudeaux added a comment. View Revision > > > You may use Linux to develop software that's intended to be used

D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

tcanabrava added a subscriber: apol. tcanabrava added a comment. But why would I get the warning if I build on Linux? The warning should target the platform, not the entire build system. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D26752 To:

Re: D26752: ECMAddAppIcon: Do not warn about mac and window icons if isnt a OS specific build

But why would I get the warning if I build on Linux? The warning should target the platform, not the entire build system. On Sun, 19 Jan 2020 at 10:00 Christophe Giboudeaux < nore...@phabricator.kde.org> wrote: > cgiboudeaux requested changes to this revision. > cgiboudeaux added a comment. >

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

D26752: ECMAddAppIcon: Do not warn about mac icons if isnt a mac build

tcanabrava accepted this revision. This revision is now accepted and ready to land. REPOSITORY R240 Extra CMake Modules BRANCH warning_icons REVISION DETAIL https://phabricator.kde.org/D26752 To: patrickelectric, apol, tcanabrava Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n,

D26751: ECMAddAppIcon: Add sc in regex to extract extension from valid names

tcanabrava accepted this revision. This revision is now accepted and ready to land. REPOSITORY R240 Extra CMake Modules BRANCH sc_appicon REVISION DETAIL https://phabricator.kde.org/D26751 To: patrickelectric, tcanabrava Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2,

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

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

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

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

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

D26573: Add missing Import Env Variable

tcanabrava created this revision. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. tcanabrava requested review of this revision. REVISION SUMMARY Without this, in Qt 5.14 I get an android-like QQC2 theme This used to work on Qt

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

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

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

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

Re: D26126: Simplify param method: Return Early, Use a Map and Assert.

David, Thanks for the detailed explanation, I'm currently reworking most of the patches here. About the optimizations - I know the linear search is a bad optimization, but it adds legibility, that's what I tried to do. I tried to emulate the python `if value in vector` that is really easy to

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?

Re: D26127: Simplify cppType method: Return Early, Use a Map and Assert.

doing that as we speak. On Mon, Dec 23, 2019 at 9:52 PM Kevin Ottens wrote: > ervin added a comment. View Revision > > In D26127#582398 , @tcanabrava > wrote: > > In

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

D26128: Simplify defaultValue method: Return Early, Use Default Initialization, and Assert.

tcanabrava added inline comments. INLINE COMMENTS > apol wrote in kconfig_compiler.cpp:1122 > If instead of creating a QVector you used an initializer list directly, you > could use std::find_if without having to initialize it entirely. Also you'd > be able to use QLatin1String which would

D26127: Simplify cppType method: Return Early, Use a Map and Assert.

tcanabrava added a comment. In D26127#582296 , @ervin wrote: > Those data structure look really similar to the ones you introduced in param() for D26126 . It looks like we'll end up with a Q_GLOBAL_STATIC or

D26128: Simplify defaultValue method: Return Early, Use Default Initialization, and Assert.

tcanabrava added a comment. In D26128#582304 , @ervin wrote: > This also has similarities with D26126 , has the same defects and missed opportunities for sharing. > > Beside I'm not sure what we're trying

D26129: Remove Iterator based loops for range based loops

tcanabrava added inline comments. INLINE COMMENTS > ervin wrote in kconfig_compiler.cpp:2119 > This logic is very flawed... instead of comparing positions in the list now > you're comparing actual values. If two different positions in the list would > end up having the same values (for some

D26129: Remove Iterator based loops for range based loops

tcanabrava added a comment. In D26129#582336 , @ervin wrote: > The patch being large I didn't check each one separately but I suspect this patch misses boat loads of qAsConst. Yes, it indeed misses, I'll add the qAsConst. ( I would

  1   2   >