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