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

Reply via email to