Fix KConfig
ClosedPublic

Authored by tcanabrava on Jul 4 2018, 8:17 PM.

Details

Summary

This fixes quite a few bugs and open space for a massive cleanup.
Because the symbols where not exported we recompiled the same object
three times, one for the private library, one for the desktop
application and another for the KPart, all of those having a static
object inside of the same compilation unit - but linked in three
different objects.

This added a static data in three different objects, the object that
was supposed to be a singleton (!) - The result: KonsoleSettings::self()
in mainwindow.cpp had one pointer, in TerminalDisplay had another,
making the singleton useless.

Having just one singleton fixes a major misdesign in Konsole:
There's a call-chain starting from main.cpp that goes thru all objects
changing every setting needed, and most of the calls are just forwarding
calls to another object.

With this fixed we can remove this call-chain, and do a
connect(KonsoleSettings::self(), &KonsoleSettings::settingsChanged(),

...) inside of the object that needs to react to the changed setting

making the code much smaller and easier to figure out what's going
on.

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tcanabrava created this revision.Jul 4 2018, 8:17 PM
Restricted Application added a project: Konsole. · View Herald TranscriptJul 4 2018, 8:17 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
tcanabrava requested review of this revision.Jul 4 2018, 8:17 PM

I would like to request a high priority on this one as it's quite simple and it's holding back quite a few patches that cleans a lot of code.

ngraham added a subscriber: ngraham.

Adding a few others because I think this is something that if changed, should be changed in quite a few other places, so let's make sure relevant folks are aware and approve.

dfaure accepted this revision.Jul 5 2018, 1:00 PM

Makes sense. Q_DECL_EXPORT wouldn't be enough on Windows, but fortunately konsole doesn't support Windows. Just be careful if you ever do this elsewhere.

This revision is now accepted and ready to land.Jul 5 2018, 1:00 PM
dfaure added a comment.Jul 5 2018, 1:00 PM

Just one thing: please improve the first line of the commit log. I initially thought there was a bug in KConfig...

Just one thing: please improve the first line of the commit log. I initially thought there was a bug in KConfig...

In a sense it

Just one thing: please improve the first line of the commit log. I initially thought there was a bug in KConfig...

David, What should I use for something that will work in Windows? if there's a one size fits all, I'd rather use it.

This revision was automatically updated to reflect the committed changes.

As David mentioned, try to get the first line of the commit log to be more informative