gcraciunescu marked 2 inline comments as done.
gcraciunescu added a comment.
> Though I'm not sure I fully understand why we have this sort of wrong value
in the config anyway?
The problem coming from force font DPI code so far I see.
Now the min/max & steps to scale down/up are fixed in the UI but any reset to
default
enable/disable a DPI value there will write a forceFontDPI=0 in the config
file which seems to mean disabled.
I am not sure why we need to write a 0 on reset/disable since default has
nothing also.
However, even if we fix there we may still want to be sure the value is a
number and >= 24, at least in my opinion.
People do crazy stuff even writing manually or with scripts to configuration
files.
> Also do we want
>
> } else {
> args[QStringLiteral("kde_settings.conf/X11/ServerArguments")] =
QString();
> }
>
>
> so that a broken DPI wipes the old config entry out? Not sure.
Not sure about that either but maybe a good idea.
INLINE COMMENTS
> ngraham wrote in advancedconfig.cpp:193
> You can combine both of these conditions into the same if statement.
I thought I should follow the coding style in the file which does
all the comparing in at least 2 lines.
> ngraham wrote in advancedconfig.cpp:195
> `>=` has a different meaning when comparing strings vs comparing integers.
> You need to first parse or cast `dpiValue` to an `int`, then compare it to
> the number 24.
Right, I keep forgetting that =) Not doing much in Qt these days.
Also, QStringLiteral(X) needs to be .toInt() so I guess I drop that and use
plain number.
REPOSITORY
R123 SDDM Configuration Panel (KCM)
REVISION DETAIL
https://phabricator.kde.org/D25006
To: gcraciunescu, filipf, #plasma, ngraham, davidedmundson
Cc: davidedmundson, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev,
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
