D25006: Make sure -dpi value is valid

[Gabriel Craciunescu]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191126/32da8fbc/attachment-0001.html>