I like indentedStream.

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.

• Fix code style

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 converted char-by-char (from 8 bits to 16 bits), so isn't it faster to concatenate QStringLiterals?

I have never seen numbers, but any time I asked someone who might have seen numbers, they told me: QStringLiteral only for final strings not further modified or given to operations. Sadly cannot find references now, but I asked often the recent years.
Looking at the code, the qt_from_latin1 helper method might be as efficient on most processors as the memcpy is perhaps, and making up for any other overhead the QStringLiteral-generated code has, perhaps atomic stuff around the generated QString instance is expensive compared to the copy logic?

But ready to hear from real Qt experts once more, perhaps with numbers this time :)

update / rebased.

• Use qAsConst

it's a +1 for me. @dfaure and @ervin ?

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:

• Simplify Architecture

In D26986#602373, @davidedmundson wrote:

This is definitely an area for improvement.

This code was initially written before QGuiApplication::screenRemoved existed, which is why it's like this.
I think this solution is overly complex, you have the pointer to what changed in screenRemoved, we then discard that information and then go through loops to effectively retrieve that information we originally had.
With a refactor that could be cleaner.

• Fix Variable

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 it's a *variable reference*.
so, if I change my setting via:

• use std::any_of

• Fix position of reference on parameter

• Rebase

@ervin since they are just nitpicks I'll fix in code and land the patch.

(ah, also, konsole has been migrated to gitlab in invent.kde.org - you migth get better reviews there)

In D26202#597044, @davidre wrote:

I checked https://cgit.kde.org/kconfig.git/tree/src/kconfig_compiler/kcfg.xsd and it says

<xsd:element name="group" maxOccurs="unbounded" >
                   <xsd:complexType>
                       <xsd:sequence>
                           <xsd:element name="entry" maxOccurs="unbounded">

According to google default of minOccurs is 1 so empty files are indeed not allowed

In D26761#596825, @sandsmark wrote:

It works if not launched from another terminal I think, but Konsole explicitly launches a new instance in that case:

But before that it checks if --new-tab is passed, and then forces re-using the existing, so I'm not sure why that doesn't work. But the correct thing to do is to fix that at least:

Ideally we should pass along the environment to the existing konsole so it can set it in the new tab, but that involves reimplementing kdbusservice (like okular does), and is a bit less trivial.

edit; I tested, and --new-tab works as expected with my revert.

I think we are misunderstanding eachother here.
If I'm developing the software and running cmake all the time, having a
warning that I can't fix (because it depends in another platform) is noise,
but still being a developer I don't want to run with -Wno-dev.
I do work in applications that targets more than one system (and they are
mac / windows / ios / android) I tend to use buildhosts, and those
buildhosts will tell me the warning - if any. But only if I'm targeting
them.

@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?

That’s not a developer issue, it’s a packaging issue.

But why would I get the warning if I build on Linux? The warning should
target the platform, not the entire build system.

My patch fixed one bug (and probably created the others as you found out),
this patch reverts mine, but does —new-tab works if konsole is set to
multiple instances?

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.

• Revert "Revert "WIP: Refactor KConfigXT""
• Add Reference files for Broken KDevelop Configuration
• Fix generating of empty configuration files

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

Reopening for review :)

In D26202#595820, @tcanabrava wrote:

We can revert, and I’ll fix the full build. Doing that now.

We can revert, and I’ll fix the full build. Doing that now.

• Rebase

• Add missing copyright holders

Took the time to nuke the SignalArguments and use Param instead, easier than I initially tougth.

• Simplify List creation
• Fail with the diff file name
• Simplify Code
• Fix Typo
• Remove SignalArgument for Param
• Nitpicks and Include fixes

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.

• Fix bug ediging param variable that's supposed to be const
• Const Correctness:

• Rename KConfigCodeGenerator to KconfigCodeGeneratorBase
• Simplify diff file saving
• Add License Headers
• Documentation, Privatization, Unused method / variable
• Const, Documentation
• Newlines at the end of files
• Spacing
• Remove Globals
• Remove unused code and Debugs
• Constify ParseResult
• English Typos
• qAsConst
• Public / Private issues
• Simplify Construction
• Revert "Constify ParseResult"

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 plan to fix tomorrow, as it's almost 9:45 and it's saturday. I believe that this code is getting near review quality.
I'v also rebased against master as some other people also started to change this code - I already got a few conflicts, so I really need to rush this before it gets unmergeable.

• Fix whitespace on the reference code / genreator

• 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

• Fix many small whitespace issues

• Rebase on master

With OverlaySheet

• Merge branch 'master' into arcpatch-D25449
• Use OverlaySheet

• Move typeRegistration to the Plugin instanciation

• ListElement does not handle i18n calls, use raw array
• Dont complain about null background - Failing on Qt 5.14
• Add Key entries to the kconfigxt generator
• ArtsBell* -> CustomBell*
• kModifiers* -> KeyboardModifiers*
• Rename mouse settings
• Remove deprecated call
• Fix nitpicks
• Fix Qml Style
• Use QFileDialog to look for audio files
• Fix Binary checks
• Mark as Q_INVOKABLE, not Q_SCRIPTABLE
• Use QtQuick Dialogs for the audio file
• Fix orca calls
• Use Label instead of placeholder text
• Fix icon & text for search audio files
• Fix Triggers
• Trigger orcaInstalled() only when entering ScreenReader
• Add explanation for magic number
• Add transient window for the KNofityConfigWidget dialog
• Fix KConfig Misuse
• Explain property
• Audible Bell Layout
• Add string context

• Fix esc and click outside to close the overlay