Review Request 113443: Attempt to unbreak pre-configuring QML applets via desktop scripting

[Eike Hein]

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

(Updated Oct. 26, 2013, 3:27 p.m.)


Status
------

This change has been discarded.


Review request for Plasma, Aaron J. Seigo and Marco Martin.


Repository: kde-workspace


Description
-------

I've noticed that a panel layout JavaScript template file doesn't succeed in scripting the config of a pure-QML applet. I.e. this doesn't work:

foo = panel.addWidget("bar");
foo.currentConfigGroup = new Array("General");
foo.writeConfig("foo", "bar");

It does work for C++ applets however. Also, the config does ultimately make it to disk, and a subsequent restart of plasma-desktop reads it in just fine. Or even just opening and OK'ing the config dialog, even if the setting isn't exposed in the GUI.

So I've been looking at the code a little, and what it comes down to is that kde-workspace/libs/plasmagenericshell/scripting/applet.cpp holds an Applet::reloadConfig() that calls Plasma::Applet::configChanged(), which, if there's a d->script, calls readConfig() on its d->configLoader. ConfigLoader is a KConfigSkeleton subclass, and what KCoreConfigSkeleton::readConfig() does is explicitly read in the config from disk.

Therefore the attached patch addresses this by calling for a config sync ahead of time.

Now, I'm new to those parts of the Plasma code base, and I'm really not sure this is a kosher way to go about it. It's not clear to me whether we want to flush to disk at that time, or if there might be a prohibitive performance penalty. There may be a way to avoid this. Perhaps Plasma::Applet shouldn't clobber the modified config group by pulling in from disk -- but perhaps it needs to for other reasons. That's why someone with the bigger picture of how the design was supposed to operate needs to review this.

Debate welcome. OTOH, I am a bit pressed for time to ideally get this fixed for a release, so there'd be some value in a "good enough" too :).


Diffs
-----

  libs/plasmagenericshell/scripting/applet.cpp 3c81f77 

Diff: http://git.reviewboard.kde.org/r/113443/diff/


Testing
-------


Thanks,

Eike Hein

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20131026/96a1ac04/attachment.html>