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

On June 28th, 2012, 4:48 p.m., Aaron J. Seigo wrote:

plasma/desktop/shell/plasmaapp.cpp (Diff revision 5)
void PlasmaApp::cleanup()
397
void PlasmaApp::setWallpaper(const QString &name, const QString &mode, const QString &path)
i would prefer it if the plugin name and mode were not exposed. the use case is "setting a wallpaper image" so let's implement that. 

the main problem with being able to set the name and mode is that not only are most of the plugins optional (just asking for fun breakage) but also require configuration.

so i would recommend a rather simpler setWallpaperImage(const QString &url).

it should probably do similar to what the drag and drop support does and if the url is not local then try to fetch it using KIO.
Doesn't the Image plugin support fetching url if it is not local ? So it is not needed for this function right ?

Also what about exposing the mode also - to be able to change between "SingleImage" and "Slideshow" mode ? Or should we make this just "SingleImage" ?

- Varun


On June 24th, 2012, 3:47 p.m., Varun Herale wrote:

Review request for Plasma.
By Varun Herale.

Updated June 24, 2012, 3:47 p.m.

Description

This patch is for hosting a dbus-interface that can be used to load any installed wallpaper plugin onto current desktop containment. In case of default "image" plugin, the path to the image can also be sent which will change the wallpaper.  

Testing

Tested on different activities and made sure it works for per-virtual desktop containment.

Haven't tested on a system with multiple screens though, as I don't have access to one. Could someone please test for that ?

Diffs

  • plasma/desktop/shell/dbus/org.kde.plasma.App.xml (eefce32)
  • plasma/desktop/shell/plasmaapp.h (6ae0c89)
  • plasma/desktop/shell/plasmaapp.cpp (7abd8fc)

View Diff