D26065: [KCM/Access] Port to use KConfigXT

2019-12-18 Thread Nathaniel Graham 
ngraham added a comment.


  Sorry to invalidate your work. :/ Please feel free to review that patch!

REPOSITORY
  R119 Plasma Desktop

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

To: meven, ervin, #plasma, crossi
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-18 Thread Méven Car 
meven abandoned this revision.
meven added a comment.


  In D26065#579715 , @ngraham wrote:
  
  > FYI this KCM is also being rewritten in QML+KConfigXT somewhere else: 
D25375 
  
  
  Since the other diff seems quite advanced already I am abandonning this one.

INLINE COMMENTS

> broulik wrote in kaccess_settings.kcfg:55
> The default is `false`

Well no : see defaults() "ui.stickyKeysBeep->setChecked(true);" or load() 
"keyboardGroup.readEntry("StickyKeysBeep", true)"

> broulik wrote in kaccess_settings.kcfg:76
> The default is `false`

No see default() " ui.slowKeysPressBeep->setChecked(true);" or load()
The label is contextual.
But slowKeys is by default false making this setting disabled by default still.

> broulik wrote in kaccess_settings.kcfg:80
> The default is `false`

No ui.slowKeysAcceptBeep->setChecked(true);

> broulik wrote in kaccess_settings.kcfg:84
> The default is `false`

Same.

> broulik wrote in kaccess_settings.kcfg:97
> BounceKeysRejectBeep

What do you mean ?
This is the current key of this entry.

> broulik wrote in kaccess_settings.kcfg:121
> There is only `kNotifyAccessX` (note the X)

What do you mean ?

> broulik wrote in kaccess_settings.kcfg:142
> This is actually being calculated referencing "interal" in kaccess code:
> 
>   mouseGroup.readEntry("MKTimeToMax", (5000 + interval / 2) / interval);

Yes and it the default value taking into account the default settings : 
int((5000 + 5/2) / 5) = 1000

> broulik wrote in kaccess_settings.kcfg:144
> The key is `MKTimeToMax`

No, there are two similar keys MK-TimeToMax and MKTimeToMax, hence your 
confusion.
I had to replace the "-" with another character to allow to generate function 
with the name.

REPOSITORY
  R119 Plasma Desktop

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

To: meven, ervin, #plasma, crossi
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-18 Thread Nathaniel Graham 
ngraham added a comment.


  FYI this KCM is also being rewritten in QML+KConfigXT somewhere else: D25375 


REPOSITORY
  R119 Plasma Desktop

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

To: meven, ervin, #plasma, crossi
Cc: ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-18 Thread Méven Car 
meven updated this revision to Diff 71757.
meven added a comment.


  Remove unconvenient white space, add a else to a if block

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26065?vs=71756=71757

BRANCH
  kaccess-kconfigxt

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

AFFECTED FILES
  kcms/access/CMakeLists.txt
  kcms/access/kaccess_settings.kcfg
  kcms/access/kaccess_settings.kcfgc
  kcms/access/kcmaccess.cpp

To: meven, ervin, #plasma, crossi
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-18 Thread Méven Car 
meven updated this revision to Diff 71756.
meven added a comment.


  Remove unconvenient white space, add a else to a if block

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26065?vs=71734=71756

BRANCH
  kaccess-kconfigxt

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

AFFECTED FILES
  kcms/access/CMakeLists.txt
  kcms/access/kaccess_settings.kcfg
  kcms/access/kaccess_settings.kcfgc
  kcms/access/kcmaccess.cpp

To: meven, ervin, #plasma, crossi
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-17 Thread Méven Car 
meven updated this revision to Diff 71734.
meven marked 12 inline comments as done.
meven added a comment.


  Address review

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26065?vs=71719=71734

BRANCH
  kaccess-kconfigxt

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

AFFECTED FILES
  kcms/access/CMakeLists.txt
  kcms/access/kaccess_settings.kcfg
  kcms/access/kaccess_settings.kcfgc
  kcms/access/kcmaccess.cpp

To: meven, ervin, #plasma, crossi
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-17 Thread Kai Uwe Broulik 
broulik added a comment.


  I read kaccess.cpp and commented on where the defaults diverge, there could 
be more. There's also some keys that have typos and/or don't match.
  This shows quite impressively why having this stuff in a central place is 
useful :)
  Given kaccess.cpp is what does the handling, ultimately its defaults will be 
what is being used, not the ones in the KCM.

INLINE COMMENTS

> kaccess_settings.kcfg:13
> +
> +
> +  Whether or not to use a custom bell

lol

Can you give this a better name, like "customBell"

(Arts was the KDE 3 sound server)

> kaccess_settings.kcfg:17
> +
> +
> +  The custom bell sound file

Likewise, customBellFile

> kaccess_settings.kcfg:46
> +
> +  Lock sticked keys
> +  true

"stickied" is not a word but I can see that "stuck" implies something else

> kaccess_settings.kcfg:55
> +  Send a system bell notification when sticky keys are enabled or 
> disabled
> +  true
> +

The default is `false`

> kaccess_settings.kcfg:61
> +
> +
> +  Send a plasma notification when a lock key or sticky keys state 
> changes

"key" or whatever that k stands for?

> kaccess_settings.kcfg:76
> +  Use system bell when a key is pressed
> +  true
> +

The default is `false`

> kaccess_settings.kcfg:80
> +  Use system bell when a key is accepted
> +  true
> +

The default is `false`

> kaccess_settings.kcfg:84
> +  Use system bell when a key is rejected
> +  true
> +

The default is `false`

> kaccess_settings.kcfg:97
> +  Use system bell when a bounced key is rejected
> +  true
> +

BounceKeysRejectBeep

> kaccess_settings.kcfg:121
> +
> +
> +  Send a plasma notification when a keyboard accessbility feature 
> is enabled or disabled

There is only `kNotifyAccessX` (note the X)

> kaccess_settings.kcfg:142
> +
> +  1000
> +

This is actually being calculated referencing "interal" in kaccess code:

  mouseGroup.readEntry("MKTimeToMax", (5000 + interval / 2) / interval);

> kaccess_settings.kcfg:144
> +
> +
> +  5000

The key is `MKTimeToMax`

> kaccess_settings.kcfg:148
> +
> +  1000
> +

Defaults to `interval`

> kaccess_settings.kcfg:155
> +
> +  Acceleration profil
> +  0

profil*e*

REPOSITORY
  R119 Plasma Desktop

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

To: meven, ervin, #plasma, crossi
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26065: [KCM/Access] Port to use KConfigXT

2019-12-17 Thread Méven Car 
meven updated this revision to Diff 71719.
meven added a comment.


  Fix some default values

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26065?vs=71716=71719

BRANCH
  kaccess-kconfigxt

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

AFFECTED FILES
  kcms/access/CMakeLists.txt
  kcms/access/kaccess_settings.kcfg
  kcms/access/kaccess_settings.kcfgc
  kcms/access/kcmaccess.cpp

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

D26065: [KCM/Access] Port to use KConfigXT

2019-12-17 Thread Méven Car 
meven created this revision.
meven added reviewers: ervin, Plasma, crossi.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
meven requested review of this revision.

TEST PLAN
  Tested changing settings

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kaccess-kconfigxt

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

AFFECTED FILES
  kcms/access/CMakeLists.txt
  kcms/access/kaccess_settings.kcfg
  kcms/access/kaccess_settings.kcfgc
  kcms/access/kcmaccess.cpp

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