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, kde-framewor

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 = 0;

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 patch

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

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.

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 config

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&id=74600 BRANCH simplify_newEntry REVISION DETAIL https://phabricator.kde.org/D26868 AFFECTED F

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 co

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 h

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&id=74596 BRANCH remove_enum_hack REVISION DETAIL http

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: !kcm.balooSettings.isImmutable("indexingE

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&id=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&id=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&id=74258 BRANCH arcpatch-D26131 REVISION DETAIL https://phabricator.kde.org/D26131 AFFECTED FI

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 st

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 AF

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 BRANCH

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&id=74231 BRANCH rewrite_maps REVISION DETAIL https://phabricator.kde.org

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 ge

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&id=74121 REVISION DETAIL https://phabrica

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 https://phabricator.kd

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 f

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, cgib

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

'm not *deploying*. nor I 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

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, er

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, LeGast00n,

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, LeGast

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 a

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: patricke

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. > Th

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 REVIS

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 https://phabr

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 kconfigx

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, GB_2,

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, bencreas

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, ngraha

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 t

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, michae

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 dirs

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&id=73688 REVISION DETAIL https://ph

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 functi

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&id=73608 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTED FILES aut

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&id=73557 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D2620

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 :) RE

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 https://phabricator.kde.org

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&id=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 is

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 > l

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&id=73310 BRANCH arcpatch-D26202 REVISION DETAIL https://phabri

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 L

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&id=73292 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D2

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&id=73290 BRANCH arcpatch-D26202 REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTED F

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 5

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 https://phabricator.k

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, michaelh,

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, #fram

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 a

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 S

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 fixe

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&id=72349 BRANCH megaRefactor REVISION DETAIL https://phabricator.kde.org/D26202 AFFECTE

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. REPOSITORY

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&id=72171 BRANCH megaRefactor REVISION DETAIL https:

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 https://pha

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 read,

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? What's

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 D26127

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 Cf

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 save

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 s

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 rea

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 actual

  1   2   >