D24743: Update GTK settings according to Plasma settings

[Kevin Ottens]

ervin added inline comments.

INLINE COMMENTS

> configeditor.cpp:49
> +    QString 
> configLocation(QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation));
> +    QString gtk3ConfigPath(configLocation + "/gtk-3.0/settings.ini");
> +

Better wrap the const char* in a QStringLiteral.

Also, but more of a nitpick this time: style wise I'd favor using = for those 
initialization (and others in the file) than parenthesis. It gets too close the 
most vexing parse territory, I'd rather not potentially expose a future 
developer touching that file to it.

> configeditor.cpp:81
> +{
> +    QString gtkrcPath(QDir::homePath() + "/.gtkrc-2.0");
> +    QFile gtkrc(gtkrcPath);

QStringLiteral please.

> configeditor.cpp:96
> +    } else {
> +        return "";
> +    }

QString() or {} instead of ""

> configeditor.cpp:102
> +{
> +    const QRegularExpression regExp(paramName + "=[^\n]*($|\n)");
> +

QStringLiteral please

There are more of those in the file, I'll stop pointing them out. ;-)

> configeditor.h:45
> +    QString readFileContents(QFile &gtkrc) const;
> +    pid_t getPidOfXSettingsd() const;
> +};

We don't prefix getters with get.

> configvalueprovider.cpp:35
> +
> +    KSharedConfigPtr 
> config(KSharedConfig::openConfig(QStringLiteral("kdeglobals")));
> +    KConfigGroup configGroup(config->group(QStringLiteral("General")));

I will contradict my previous comment about making those functions static or 
turning into a namespace. I think it'd be better to avoid creating and ditching 
those KSharedConfig instances at each call, so why not instantiate it when 
ConfigValueProvider is created?

You might have to load() from the config at right point in times though.

> configvalueprovider.h:37
> +
> +    QString getConfigFontName() const;
> +    QString getConfigIconThemeName() const;

We don't prefix getters with get

Beside none of those have any state AFAICT, why not make them static or turn 
this into a namespace?

REPOSITORY
  R99 KDE Gtk Configuration Tool

REVISION DETAIL
  https://phabricator.kde.org/D24743

To: gikari, #plasma, #vdg
Cc: ervin, ngraham, broulik, nicolasfella, plasma-devel, LeGast00n, 
The-Feren-OS-Dev, cblack, konkinartem, ian, jguidon, hannahk, Ghost6, jraleigh, 
MrPepe, fbampaloukas, squeakypancakes, alexde, IohannesPetros, GB_2, 
trickyricky26, ragreen, mglb, crozbo, ndavis, ZrenBot, firef, alexeymin, 
skadinna, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, aaronhoneycutt, 
abetts, sebas, apol, ahiemstra, mbohlender, mart