D22700: Sync scaling DPI from Plasma to SDDM

[David Edmundson]

davidedmundson added a comment.


  One potentially major issue that if I'm right needs fixing ASAP

INLINE COMMENTS

> advanceconfig.cpp:156
> +    KConfigGroup dpiConfigGroup(&dpiConfig, "General");
> +    QString dpiValue = QStringLiteral("-dpi ") + dpiConfigGroup.readEntry("forceFontDPI");
> +

What are you going to write if this entry is missing?

I suspect you'll write just

"-dpi "  into ServerArguments

and then X11 will run with "X11 -dpi"   expect another argument and simply fail to load?

> advanceconfig.cpp:157
> +    QString dpiValue = QStringLiteral("-dpi ") + dpiConfigGroup.readEntry("forceFontDPI");
> +
>      KConfig numLockConfig(QStringLiteral("kcminputrc"));

As a coding note it's useful to make sure the data in the variables matches their name.

In this case

"dpiValue" isn't just the DPI value, it's a string formatted as X server arguments "-dpi" here.

So I would suggest either renaming or (probably easier with the other fix) adding the "-dpi "prefix when you insert it into the map.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: filipf, #plasma, davidedmundson, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190724/910ca5a9/attachment.html>