D22748: whitelist the "turn off screen" shortcut on the lock screen

[Martin Blumenstingl]

This revision was automatically updated to reflect the committed changes.
Closed by commit R133:5ed0dc17f3f9: Allow the turn off screen to be 
used from the lockscreen (authored by mblumenstingl).

REPOSITORY
  R133 KScreenLocker

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22748?vs=62570=62637

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

AFFECTED FILES
  globalaccel.cpp

To: mblumenstingl, broulik, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22748: whitelist the "turn off screen" shortcut on the lock screen

[Martin Blumenstingl]

mblumenstingl added a comment.


  In D22748#502337 , @mblumenstingl 
wrote:
  
  > I did some more digging and I now believe that I'm facing a problem that's 
related to me running wayland.
  >
  > 
https://github.com/KDE/kwindowsystem/blob/05f5379c24a59e1ded2f12a6fd09946617abd0ac/src/kkeyserver.h#L29
  >  KKeyServer has a compile-time switch for it's backend.
  >  I guess this needs to be converted to a runtime switch similar to what was 
done here 
https://github.com/KDE/kguiaddons/commit/0fe2990dbad992a4925a7b7bee09b1cdfbe5a7a7#diff-928c366533ef47aba95b92e8767bc8ff
  
  
  there's even a task on phabricator for this: T4423 


REPOSITORY
  R133 KScreenLocker

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

To: mblumenstingl, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22748: whitelist the "turn off screen" shortcut on the lock screen

[Martin Blumenstingl]

mblumenstingl added a comment.


  I did some more digging and I now believe that I'm facing a problem that's 
related to me running wayland.
  
  
https://github.com/KDE/kwindowsystem/blob/05f5379c24a59e1ded2f12a6fd09946617abd0ac/src/kkeyserver.h#L29
  KKeyServer has a compile-time switch for it's backend.
  I guess this needs to be converted to a runtime switch similar to what was 
done here 
https://github.com/KDE/kguiaddons/commit/0fe2990dbad992a4925a7b7bee09b1cdfbe5a7a7#diff-928c366533ef47aba95b92e8767bc8ff

REPOSITORY
  R133 KScreenLocker

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

To: mblumenstingl, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

mblumenstingl added a comment.


  In D22261#502044 , @broulik wrote:
  
  > Just checking, are you still up for making this kscreenlocker change? Given 
the heatwave right now I would have loved to be able to turn off my screens on 
the lock screen explicitly :D
  
  
  I just created D22748 
  unfortunately the patch is not working for me, probably because there's some 
other issue on my system with kscreenlocker. see the description for details

REPOSITORY
  R122 Powerdevil

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

To: mblumenstingl, #plasma, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22748: whitelist the "turn off screen" shortcut on the lock screen

[Martin Blumenstingl]

mblumenstingl created this revision.
mblumenstingl added a reviewer: broulik.
mblumenstingl added a project: Plasma.
mblumenstingl requested review of this revision.

REVISION SUMMARY
  This is a follow-up to D22261 
  
  unfortunately the test plan is not working for me:
  when I press the global shortcut on the lock screen then the display does not 
turn off.
  however, this is not related to the new global action because "volume up", 
"volume down", "mute" and "play pause" are also not working for me on the lock 
screen

TEST PLAN
  start a plasma wayland session
  open systemsettings5 -> Shortcuts -> Global Shortcuts -> Power Management
  assign a shortcut to "Turn Off Screen"
  press this shortcut
  (screen must now turn off -> this confirms that the shortcut is working)
  move the mouse or press a key to turn the screen back on
  lock the screen
  press the global shortcut again
  (screen must now turn off)

REPOSITORY
  R133 KScreenLocker

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

AFFECTED FILES
  globalaccel.cpp

To: mblumenstingl, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

mblumenstingl added a comment.


  In D22261#490680 <https://phabricator.kde.org/D22261#490680>, @broulik wrote:
  
  > Alright, very nice!
  >  I suppose you don't have commit access, so I need an email address from 
you to add you as commit author. Thanks!
  
  
  I once had commit access but I'm not sure if I still have.
  either way, I would be more comfortable with you committing this with the 
following author information: Martin Blumenstingl 

  
  >> I tried adding |Turn Off Screen to the powerdevil QRegularExpression but I 
couldn't turn off the display from the lock screen
  > 
  > You probably need to log out and back in for the new screenlocker binary to 
be loaded (or run the kscreenlocker_test binary in the build/bin directory)
  
  alright, I'll give this another go and open another review

REPOSITORY
  R122 Powerdevil

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

To: mblumenstingl, #plasma, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

mblumenstingl added a comment.


  In D22261#490660 , @broulik wrote:
  
  > Bonus points if you also make a patch for kscreenlocker globalaccel.cpp to 
whitelist this new action so that it also works on the lock screen.
  
  
  I tried adding `|Turn Off Screen` to the powerdevil QRegularExpression but I 
couldn't turn off the display from the lock screen
  my test plan for this is:
  
  - original test plan
  - move mouse so the screen turns on again
  - application launcher -> Leave -> Lock -> confirm
  - press my shortcut
  - (nothing happens)

REPOSITORY
  R122 Powerdevil

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

To: mblumenstingl, #plasma, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

mblumenstingl added inline comments.

INLINE COMMENTS

> broulik wrote in powerdevildpmsaction.cpp:83
> I suppose there's no `Qt::Key_` enum for that, given laptops typically bypass 
> the operating system here anyway.

I couldn't find any appropriate `Qt::Key_` so I'm leaving it up to the user to 
choose one

> broulik wrote in powerdevildpmsaction.cpp:84
> Use new style connect, you can probably just use a lambda
> 
>   connect(globalAction, ::triggered, this, [this] {
>   if (m_helper) {
>   m_helper->trigger(QStringLiteral("TurnOff"));
>   }
>   });

that if (m_helper) is probably the overloaded != nullptr from 
https://doc.qt.io/qt-5/qscopedpointer.html#operator-not-eq-1
thanks for this hint, it's been a long time since I last did some c++/Qt

> broulik wrote in powerdevildpmsaction.h:57
> For `SLOT(...)` syntax to work this must be in a section `private Q_SLOTS`, 
> but note my comment below about using a lambda instead

sorry for that, I missed that when I moved from a public slot to a private one
your lambda expression is working fine so I'm going with that instead

REPOSITORY
  R122 Powerdevil

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

To: mblumenstingl, #plasma, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

mblumenstingl updated this revision to Diff 61124.
mblumenstingl marked 5 inline comments as done.
mblumenstingl added a comment.


  updated patch to address the comments from @broulik

REPOSITORY
  R122 Powerdevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22261?vs=61118=61124

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

AFFECTED FILES
  daemon/actions/dpms/CMakeLists.txt
  daemon/actions/dpms/powerdevildpmsaction.cpp

To: mblumenstingl, #plasma, broulik
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22261: Add a global shortcut action to turn off the screen

[Martin Blumenstingl]

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

REVISION SUMMARY
  PowerDevilDPMSAction already provides implementations for X11 and Wayland 
which allow turning off the screen.
  For X11 there was a way to do this on the command line (with "xset dpms force 
off"), but there's no such replacement for Wayland yet.
  
  Add an action - to which users can assign a shortcut - to turn off the screen.

TEST PLAN
  start a plasma wayland session
  open systemsettings5 -> Shortcuts -> Global Shortcuts -> Power Management
  assign a shortcut to "Turn Off Screen"
  press this shortcut
  (screen must now turn off)

REPOSITORY
  R122 Powerdevil

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

AFFECTED FILES
  daemon/actions/dpms/CMakeLists.txt
  daemon/actions/dpms/powerdevildpmsaction.cpp
  daemon/actions/dpms/powerdevildpmsaction.h

To: mblumenstingl, #plasma
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

Review Request: kdelibs: default-values in config files should be able to handle code-generated default values

[Martin Blumenstingl]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5525/
---

Review request for Plasma.


Summary
---

For some projects it's necessary to have code-generated default values for some 
of the settings.
Just an example:
say a plasmoid prints text somewhere. The font (including the color) is 
configurable.
Now there's one problem: the developer might run into trouble when setting a 
hardcoded default color in the config file.

Here's my solution:
KConfigXT already support code-generated default values, see: 
http://techbase.kde.org/Development/Tutorials/Using_KConfig_XT (take a look at 
the Font example).
With my patch it's possible to add code to the default tags.
The ConfigLoader will parse it (with the current script's script engine) if the 
code attribute is set to true.

This patch provides the kdelibs part.
The patch for the JavaScript engine (and probably other engines) will be part 
of a separate review:
http://svn.reviewboard.kde.org/r/5526/


Please note that my patch is not finished yet.
I still need to write unit tests (but before that I need to think about how to 
write them..).

There is one issue which I'd like to fix soon:
The ConfigLoader does not handle QVariants yet.
This means: if the user specifies type=color then the code-generated value 
must return a QString (which is then passed to the ctor of QColor()).
But a developer would expect the following: if the code-generated value returns 
a QString then that QString will be passed to the ctor of QColor(). If the 
code-generated value returns a QColor() then that one is directly used.

PS: The reason why I opened a review request for an unfinished patch is quite 
simple:
Communication is the key to success.
Thus I'd like to ask you about comments before I finish it (otherwise someone 
might tell me after I'm done with it: the architecture of your patch sucks. 
Your patch can go to hell. kthxbye ;)).


Diffs
-

  /trunk/KDE/kdelibs/plasma/applet.cpp 1182146 
  /trunk/KDE/kdelibs/plasma/configloader.h 1182146 
  /trunk/KDE/kdelibs/plasma/configloader.cpp 1182146 
  /trunk/KDE/kdelibs/plasma/private/configloader_p.h 1182146 
  /trunk/KDE/kdelibs/plasma/scripting/scriptengine.h 1182146 
  /trunk/KDE/kdelibs/plasma/scripting/scriptengine.cpp 1182146 

Diff: http://svn.reviewboard.kde.org/r/5525/diff


Testing
---

The ConfigLoader unit-test did not fail :)
I also wrote a test-plasmoid: http://filebin.ca/abcwza/codedefault.tar.bz2


Thanks,

Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: kdebase: add support for ScriptEngine::evaluate() in the JavaScript Engine (depends on 5525)

[Martin Blumenstingl]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5526/
---

Review request for Plasma.


Summary
---

This adds support for evaluating code statements in the JavaScript Engine.
Please note that this depends on http://svn.reviewboard.kde.org/r/5525/

If this one gets approved I'll start working on the other engines.
I hope porting this to other engines is possible.


Diffs
-

  /trunk/KDE/kdebase/runtime/plasma/scriptengines/javascript/common/scriptenv.h 
1182146 
  
/trunk/KDE/kdebase/runtime/plasma/scriptengines/javascript/common/scriptenv.cpp 
1182146 
  
/trunk/KDE/kdebase/runtime/plasma/scriptengines/javascript/plasmoid/simplejavascriptapplet.h
 1182146 
  
/trunk/KDE/kdebase/runtime/plasma/scriptengines/javascript/plasmoid/simplejavascriptapplet.cpp
 1182146 

Diff: http://svn.reviewboard.kde.org/r/5526/diff


Testing
---

I wrote a little test-plasmoid.
You can get it here (I'll probably commit a polished version of that to 
kdeexamples at some point):
http://filebin.ca/abcwza/codedefault.tar.bz2


Thanks,

Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: kdelibs: default-values in config files should be able to handle code-generated default values

[Martin Blumenstingl]

 On 2010-10-03 19:01:08, Marco Martin wrote:
  I like the approach, as you said, supporting the extra types is needed,
  so a similar binding like the js engine itself for the color ctor is needed 
  as well here.
  are stringlist supported atm?

StringLists are supported (in a very ugly way) by the ConfigLoader.
Currently a string list is a comma-separated list of strings. Escaping commas 
does not work right now.
(But that is a part of the code which I didn't touch).


- Martin


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5525/#review7951
---


On 2010-10-03 18:10:47, Martin Blumenstingl wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/5525/
 ---
 
 (Updated 2010-10-03 18:10:47)
 
 
 Review request for Plasma.
 
 
 Summary
 ---
 
 For some projects it's necessary to have code-generated default values for 
 some of the settings.
 Just an example:
 say a plasmoid prints text somewhere. The font (including the color) is 
 configurable.
 Now there's one problem: the developer might run into trouble when setting a 
 hardcoded default color in the config file.
 
 Here's my solution:
 KConfigXT already support code-generated default values, see: 
 http://techbase.kde.org/Development/Tutorials/Using_KConfig_XT (take a look 
 at the Font example).
 With my patch it's possible to add code to the default tags.
 The ConfigLoader will parse it (with the current script's script engine) if 
 the code attribute is set to true.
 
 This patch provides the kdelibs part.
 The patch for the JavaScript engine (and probably other engines) will be part 
 of a separate review:
 http://svn.reviewboard.kde.org/r/5526/
 
 
 Please note that my patch is not finished yet.
 I still need to write unit tests (but before that I need to think about how 
 to write them..).
 
 There is one issue which I'd like to fix soon:
 The ConfigLoader does not handle QVariants yet.
 This means: if the user specifies type=color then the code-generated value 
 must return a QString (which is then passed to the ctor of QColor()).
 But a developer would expect the following: if the code-generated value 
 returns a QString then that QString will be passed to the ctor of QColor(). 
 If the code-generated value returns a QColor() then that one is directly used.
 
 PS: The reason why I opened a review request for an unfinished patch is quite 
 simple:
 Communication is the key to success.
 Thus I'd like to ask you about comments before I finish it (otherwise someone 
 might tell me after I'm done with it: the architecture of your patch sucks. 
 Your patch can go to hell. kthxbye ;)).
 
 
 Diffs
 -
 
   /trunk/KDE/kdelibs/plasma/applet.cpp 1182146 
   /trunk/KDE/kdelibs/plasma/configloader.h 1182146 
   /trunk/KDE/kdelibs/plasma/configloader.cpp 1182146 
   /trunk/KDE/kdelibs/plasma/private/configloader_p.h 1182146 
   /trunk/KDE/kdelibs/plasma/scripting/scriptengine.h 1182146 
   /trunk/KDE/kdelibs/plasma/scripting/scriptengine.cpp 1182146 
 
 Diff: http://svn.reviewboard.kde.org/r/5525/diff
 
 
 Testing
 ---
 
 The ConfigLoader unit-test did not fail :)
 I also wrote a test-plasmoid: http://filebin.ca/abcwza/codedefault.tar.bz2
 
 
 Thanks,
 
 Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add an unit test for Plasma::ConfigLoader

[Martin Blumenstingl]

 On 2010-09-12 21:47:38, Martin Blumenstingl wrote:
  /trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp, line 172
  http://svn.reviewboard.kde.org/r/5329/diff/1/?file=35739#file35739line172
 
  I am not sure why, but actual is always empty.
  I can't figure out the reason, because building the list is OK in 
  ConfigLoader.
  
  I can access the list when I use it in a javascript plasmoid - so there 
  may be something wrong with the code in my test.

after a discussion with Aaron: using QListqint32 is wrong here.
ItemIntList is a QListint - that's why casting fails.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5329/#review7566
---


On 2010-09-12 21:45:48, Martin Blumenstingl wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/5329/
 ---
 
 (Updated 2010-09-12 21:45:48)
 
 
 Review request for Plasma.
 
 
 Summary
 ---
 
 Currently I'm changing some of the code in Plasma::ConfigLoader.
 I want to make sure that I don't break that much ;)
 
 Thus I wrote a unit test for the config loader.
 Currently all it does it testing if the config loader can parse the 
 default-values correctly.
 One test for each (data-)type which ConfigLoader can handle.
 
 Unfortunately ConfigLoaderTest::intListDefaultValue does not work yet 
 (everything else works)
 
 
 Diffs
 -
 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.xml PRE-CREATION 
   /trunk/KDE/kdelibs/plasma/tests/CMakeLists.txt 1174512 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.h PRE-CREATION 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp PRE-CREATION 
 
 Diff: http://svn.reviewboard.kde.org/r/5329/diff
 
 
 Testing
 ---
 
 I ran the tests on my box.
 20/21 were successful.
 
 The only one failing is: ConfigLoaderTest::intListDefaultValue
 (and I need some help there)
 
 
 Thanks,
 
 Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add an unit test for Plasma::ConfigLoader

[Martin Blumenstingl]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5329/
---

(Updated 2010-09-13 17:51:03.041714)


Review request for Plasma.


Changes
---

fixed the failing test :)


Summary (updated)
---

Currently I'm changing some of the code in Plasma::ConfigLoader.
I want to make sure that I don't break that much ;)

Thus I wrote a unit test for the config loader.
Currently all it does it testing if the config loader can parse the 
default-values correctly.
One test for each (data-)type which ConfigLoader can handle.


Diffs (updated)
-

  /trunk/KDE/kdelibs/plasma/tests/configloadertest.xml PRE-CREATION 
  /trunk/KDE/kdelibs/plasma/tests/CMakeLists.txt 1174512 
  /trunk/KDE/kdelibs/plasma/tests/configloadertest.h PRE-CREATION 
  /trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp PRE-CREATION 

Diff: http://svn.reviewboard.kde.org/r/5329/diff


Testing (updated)
---

I ran the tests on my box.
All of them were successful.


Thanks,

Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request: Add an unit test for Plasma::ConfigLoader

[Martin Blumenstingl]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5329/
---

Review request for Plasma.


Summary
---

Currently I'm changing some of the code in Plasma::ConfigLoader.
I want to make sure that I don't break that much ;)

Thus I wrote a unit test for the config loader.
Currently all it does it testing if the config loader can parse the 
default-values correctly.
One test for each (data-)type which ConfigLoader can handle.

Unfortunately ConfigLoaderTest::intListDefaultValue does not work yet 
(everything else works)


Diffs
-

  /trunk/KDE/kdelibs/plasma/tests/configloadertest.xml PRE-CREATION 
  /trunk/KDE/kdelibs/plasma/tests/CMakeLists.txt 1174512 
  /trunk/KDE/kdelibs/plasma/tests/configloadertest.h PRE-CREATION 
  /trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp PRE-CREATION 

Diff: http://svn.reviewboard.kde.org/r/5329/diff


Testing
---

I ran the tests on my box.
20/21 were successful.

The only one failing is: ConfigLoaderTest::intListDefaultValue
(and I need some help there)


Thanks,

Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request: Add an unit test for Plasma::ConfigLoader

[Martin Blumenstingl]

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5329/#review7566
---



/trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp
http://svn.reviewboard.kde.org/r/5329/#comment7726

I am not sure why, but actual is always empty.
I can't figure out the reason, because building the list is OK in 
ConfigLoader.

I can access the list when I use it in a javascript plasmoid - so there may 
be something wrong with the code in my test.


- Martin


On 2010-09-12 21:45:48, Martin Blumenstingl wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/5329/
 ---
 
 (Updated 2010-09-12 21:45:48)
 
 
 Review request for Plasma.
 
 
 Summary
 ---
 
 Currently I'm changing some of the code in Plasma::ConfigLoader.
 I want to make sure that I don't break that much ;)
 
 Thus I wrote a unit test for the config loader.
 Currently all it does it testing if the config loader can parse the 
 default-values correctly.
 One test for each (data-)type which ConfigLoader can handle.
 
 Unfortunately ConfigLoaderTest::intListDefaultValue does not work yet 
 (everything else works)
 
 
 Diffs
 -
 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.xml PRE-CREATION 
   /trunk/KDE/kdelibs/plasma/tests/CMakeLists.txt 1174512 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.h PRE-CREATION 
   /trunk/KDE/kdelibs/plasma/tests/configloadertest.cpp PRE-CREATION 
 
 Diff: http://svn.reviewboard.kde.org/r/5329/diff
 
 
 Testing
 ---
 
 I ran the tests on my box.
 20/21 were successful.
 
 The only one failing is: ConfigLoaderTest::intListDefaultValue
 (and I need some help there)
 
 
 Thanks,
 
 Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel