Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-08 Thread Ivan Cukic


 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
 
 
 

 
 Aaron Seigo wrote:
 - 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.

your current patch doesn't address that either, so not an issue for this 
feature.

It does, it switches automatically from any plugin to the image plugin.

For the rest, ok


- Ivan


---
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.
 

Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-07 Thread Ivan Cukic


 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.

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.


- Ivan


---
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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-07 Thread Aaron Seigo


 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.

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)


- 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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-07 Thread Ivan Cukic


 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)

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


- Ivan


---
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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-07 Thread Aaron Seigo


 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.
 

Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-06 Thread Ivan Cukic

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

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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-06 Thread Aaron Seigo

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


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?

- Aaron


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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-06 Thread Ivan Cukic


 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?

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.


- Ivan


---
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


Re: Review Request: Plasma D-Bus Interface: Setting wallpaper image

2009-10-06 Thread Aaron Seigo


 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.

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.


- 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