-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105319/#review15052
-----------------------------------------------------------
I think you've overcomplicated this:
Everything after currentContainment could be replaced with simply:
---
currentContainment->setWallpaper(name, mode);
if (!path.isEmpty()) {
currentContainment->wallpaper()->setUrls(KUrl(path));
}
---
if the name and mode haven't changed setWallpaper will do nothing.
if the plugin doesn't support setUrls it will do nothing, if it does - your API
will work with it \o/.
Containment::setWallpaper appears to save when changing, same for setUrls on
the plugin.
I tested the above and it saves and restores fine :)
plasma/desktop/shell/plasmaapp.cpp
<http://git.reviewboard.kde.org/r/105319/#comment11838>
This makes it only work with local files.
Image::addUrl (which is what you call from Plasma::Wallpaper::setUrls()
will accept any remote file and copy it. So this seems a pointless restriction.
plasma/desktop/shell/plasmaapp.cpp
<http://git.reviewboard.kde.org/r/105319/#comment11841>
could you just do currentContainment->config() ?
plasma/desktop/shell/plasmaapp.cpp
<http://git.reviewboard.kde.org/r/105319/#comment11837>
You set the currentContainment wallpaper to single image twice. Here and in
a few lines.
plasma/desktop/shell/plasmaapp.cpp
<http://git.reviewboard.kde.org/r/105319/#comment11840>
currentContainment->wallpaper()=>save(grp);
might be better than meddling with someone elses config.
(obviously do this after you've set the image)
plasma/desktop/shell/plasmaapp.cpp
<http://git.reviewboard.kde.org/r/105319/#comment11839>
second setting of currentContainment->setWallpaper
- David Edmundson
On June 22, 2012, 2:36 p.m., Varun Herale wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105319/
> -----------------------------------------------------------
>
> (Updated June 22, 2012, 2:36 p.m.)
>
>
> Review request for Plasma.
>
>
> 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.
>
>
> Diffs
> -----
>
> plasma/desktop/shell/dbus/org.kde.plasma.App.xml eefce32
> plasma/desktop/shell/plasmaapp.h 6ae0c89
> plasma/desktop/shell/plasmaapp.cpp 7abd8fc
>
> Diff: http://git.reviewboard.kde.org/r/105319/diff/
>
>
> 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 ?
>
>
> Thanks,
>
> Varun Herale
>
>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
