> On 2009-10-06 19:05:00, Aaron Seigo wrote: > > shouldn't the wallpaper export its own dbus interface, and the path to that > > dbus object be dependent on the containment id? e.g. something like > > Containments/1/Wallpaper? then instead of a plugin-specific hack, we could > > have per-plugin controls on the bus? > > Ivan Cukic wrote: > The main reason for this is that it provides a quick, minimal change that > provides the desired functionality (no changes in the libplasma, wallpaper > plugins, ... just PlasmaApp). > > I agree that the d-bus objects for each containment/wallpaper would be a > much cleaner solution*, but it would need a lot of work that nobody > (including myself) doesn't seem to have the time to do now (as it seems). > > So, we have a lot ppl asking for this while we could remove the function > once we get the more detailed one (if there is a need for that at all) since > we don't need to probide d-bus API compatibility. > > * Although a bit overblown IMO. > > Aaron Seigo wrote: > if nobody is willing to do the work, then the feature doesn't make it in. > the people asking for the feature can come up with the resources to make it > happen, as far as i'm concerned. if this patch is allowed in then people will > start relying on it being there and we will not be able to remove it, even if > a proper solution comes along. moreover, by giving people a half-way-done > solution that takes away just enough of the annoyance/pain, there is even > less motivation to do it properly. so we'll end up with a poor solution to a > problem that actually has a clear solution but which people even less likely > to implement than they are now. > > Ivan Cukic wrote: > Idea: We have the arguments for creating wallpapers (like for all other > plugins) - what about making wallpaper plugins to use those arguments (if > they exist) and making the dbus function that uses those for setting the > wallpaper parameters? > > benefits: > - still a very simple dbus interface (no need for registering multiple > objects*) > - plugin independent > > drawbacks: > - the caller of the method would need to know what the plugin accepts as > parameters > > * why I'm against the per-containment dbus objects - it is all powerful > approach, but since we decided to go for the javascript we don't need the all > powerful approach using dbus. > > Aaron Seigo wrote: > that would imply deleting and creating a new wallpaper object, and as you > note it wouldn't be perfect. > > it's really not a big deal to simply have the Image wallpaper plugin > export a small dbus interface and register it under the "right" path on the > bus. nothing in Containment is needed to do that and your patch already adds > the "list containments of type Foo by ID" so this information would all be > available. > > iow, you don't need to create a Containment DBUS object for this, just > create the Image wallpaper plugin one and put it in the right path in DBUS > (/Containments/$ID/Wallpaper) > > Ivan Cukic wrote: > OK. But there still are some questions :) > > - How to handle switching the wallpaper plugins? (If there is no > Containment object in D-Bus) > - Wallpaper can not access its containment's ID to set the path > > > >
"- How to handle switching the wallpaper plugins? (If there is no Containment object in D-Bus)" your current patch doesn't address that either, so not an issue for this feature. "- Wallpaper can not access its containment's ID to set the path" a solution would be to have Containment set the D-Bus path for the Wallpaper. in fact, Containment could export the Wallpaper object using QDBusConnection::registerObject on it; it could be limited to properties or perhaps signals/slots marked as Q_SCRIPTABLE using QDBusConnection::RegisterOptions. that would allow Wallpaper plugins to simply provide a set of properties and/or scriptable methods and voila they get a DBus interface based on that for free. - Aaron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1798/#review2568 ----------------------------------------------------------- On 2009-10-06 11:28:00, Ivan Cukic wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1798/ > ----------------------------------------------------------- > > (Updated 2009-10-06 11:28:00) > > > Review request for Plasma. > > > Summary > ------- > > There are many users who want the way to set the wallpaper via d-bus. > > This enables them to do so, but only for the image wallpaper. > > Since there is no mechanism in Plasma::Wallpaper (as far as I know) to set > wallpaper options from outside of the wallpaper plugin, this patch relies on > the structure of the Image wallpaper plugin and its configuration file format. > > > Diffs > ----- > > > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/dbus/org.kde.plasma.App.xml > 1031712 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.h 1031712 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/plasmaapp.cpp 1031712 > > Diff: http://reviewboard.kde.org/r/1798/diff > > > Testing > ------- > > Testing done - changing the wallpaper from one image to another, from another > plugin to image plugin. > > > Thanks, > > Ivan > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
