broulik added a comment.

  Interesting approach!
  However, this doesn't seem to apply anything on first login?
  Maybe you want to extend the "rdb" thing instead:
  `runRdb(KRdbExportQtColors | KRdbExportGtkTheme | (m_applyToAlien ? 
KRdbExportColors : 0));` and add a `KRdbExportGtkCssColors` which then does all 
of that, given there's already a "Gtk Theme" in there.
  
  Overall I must say I'm not a fan of adding yet another place where we mess 
with GTK config files and would prefer someone write that centralized gtk 
settings daemon I proposed numerous times :) but I don't want to block this on 
a hypothetical idea that will probably never emerge...

INLINE COMMENTS

> colors.cpp:323
>  
> +QString gtkColorsHelper(QString name, QString color)
> +{

Pass by const-ref `const QString &name, ..`

> colors.cpp:325
> +{
> +    return ("@define-color " + name + " " + color) + ";\n";
> +}

`return QStringLiteral("@define-color %1 %2").arg(name, color);\n"`;

> colors.cpp:454
>  
> +    QFile gtkCss(QDir::homePath() + "/.config/gtk-3.0/gtk.css");
> +    QFile colorsCss(QDir::homePath() + "/.config/gtk-3.0/colors.css");

You probably want to be using 
`QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation)` or 
something along the lines instead of hardcoding `~/.config`

> colors.cpp:458
> +    if (gtkCss.open(QIODevice::ReadWrite))
> +    {
> +        QTextStream gtkStream(&gtkCss);

Coding style, brace on same line as `if`

> colors.cpp:471
> +    }
> +    if (colorsCss.open(QIODevice::ReadWrite | QIODevice::Truncate)) {
> +        QTextStream colorsStream(&colorsCss);

Not just `WriteOnly`?

REPOSITORY
  R119 Plasma Desktop

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

To: cblack, #plasma
Cc: broulik, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

Reply via email to