D25006: Make sure -dpi value is valid

[Gabriel Craciunescu]

gcraciunescu marked 2 inline comments as done.
gcraciunescu added a comment.


  In D25006#605578 , @filipf wrote:
  
  > David's suggestion to just turn `dpiValue` into an int sounds sensible to 
me. @gcraciunescu do you want to finish this patch?
  
  
  I think you guys need to agree on something first. I don' mind if someone 
else commits a fix for that.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: gcraciunescu, filipf, #plasma, ngraham, davidedmundson
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

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

D25006: Make sure -dpi value is valid

[Gabriel Craciunescu]

gcraciunescu updated this revision to Diff 68898.
gcraciunescu retitled this revision from "Checking dpi value being empty is not 
enough" to "Make sure -dpi value is valid".
gcraciunescu edited the summary of this revision.
gcraciunescu added a comment.


  Addressed changes requested by ngraham.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25006?vs=68882=68898

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

AFFECTED FILES
  src/advancedconfig.cpp

To: gcraciunescu, filipf, #plasma, ngraham
Cc: 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

D25004: Fix force font DPI UI logic

[Gabriel Craciunescu]

gcraciunescu added a comment.


  You can use cr...@frugalware.org.

REPOSITORY
  R119 Plasma Desktop

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

To: gcraciunescu, davidedmundson, #plasma
Cc: 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

D25006: Checking dpi value being empty is not enought

[Gabriel Craciunescu]

gcraciunescu created this revision.
gcraciunescu added a reviewer: filipf.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
gcraciunescu requested review of this revision.

REVISION SUMMARY
  We need to make sure the -dpi value is something valid.
  Right now we can pass everything as value to -dpi, including 0,1 or any 
string.
  Check for the lowest possible value set from force font settings.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

AFFECTED FILES
  src/advancedconfig.cpp

To: gcraciunescu, filipf
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25004: Fix force font DPI UI logic

[Gabriel Craciunescu]

gcraciunescu created this revision.
gcraciunescu added a reviewer: davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
gcraciunescu requested review of this revision.

REVISION SUMMARY
  The kcm UI has right now a step size of 24 with a max size of 1000 and a 
minimum of 1.
  
  Max size need to divide to step size and the lowest possible value should be 
the step size
  because  0 means disabled.

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: gcraciunescu, davidedmundson
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart