Review Request 120303: Make the contact list use the kded message plugin dbus interface

[James Smith]

> On Sept. 21, 2014, 12:29 p.m., David Edmundson wrote:
> >

It's a bit more code than I'd like it to be, I couldn't find how to add a class for each plugin with the plugin name part in the dbus address, so each plugin had to be in an exported DBus property. Meaning an entire adapter class to interface with the statusHandler. Which meant a few additional features from the status handler and more methods / properties for each connecting application. This can replace the plugin terminator in the kded plugin, whose functionality should be handled in the presence-setting application anyway, as a direct consequence of user action. It also allows easy enabling of status message plugins from the presence applet.


> On Sept. 21, 2014, 12:29 p.m., David Edmundson wrote:
> > global-presence-chooser.cpp, line 376
> > <https://git.reviewboard.kde.org/r/120303/diff/1/?file=314083#file314083line376>
> >
> >     given we only have one status message plugin, this seems massively over-engineered.

I think this should be asyncCall instead?


> On Sept. 21, 2014, 12:29 p.m., David Edmundson wrote:
> > global-presence-chooser.cpp, line 375
> > <https://git.reviewboard.kde.org/r/120303/diff/1/?file=314083#file314083line375>
> >
> >     I'm not sure I get this line, it means if I have a saved status message with the same text as a song title we don't change anything.
> >     
> >     which means when the song changes this'll change.

No, but if the presence is changed to a different presence (i.e. user-set) the plugins that don't have that message end up disabled as a consequence. Enabling the plugins (line 366 below, "interface->call("ToggleStatusMessagePlugin", plugin);") actually just sets the selected plugin active, disabling the plugins higher in priority and leaving any after that still active.


> On Sept. 21, 2014, 12:29 p.m., David Edmundson wrote:
> > global-presence-chooser.cpp, line 373
> > <https://git.reviewboard.kde.org/r/120303/diff/1/?file=314083#file314083line373>
> >
> >     How can something be active but not enabled?

This is a settable boolean that activates or deactivates the whole stack of status message plugins all at once depending on if the plugin is enabled in the config entry. (isConfigEnabled()).


> On Sept. 21, 2014, 12:29 p.m., David Edmundson wrote:
> > global-presence-chooser.cpp, line 315
> > <https://git.reviewboard.kde.org/r/120303/diff/1/?file=314083#file314083line315>
> >
> >     This is internally a blocking DBus call.
> >     We've had issues with that in the past.

you mean asyncCall? line 366? "interface->call("ToggleStatusMessagePlugin", plugin);"?


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120303/#review67129
-----------------------------------------------------------


On Sept. 21, 2014, 9:53 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120303/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2014, 9:53 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-contact-list
> 
> 
> Description
> -------
> 
> Makes the contact list use the dbus interface of the message plugin class to enable and disable status message plugins.
> 
> 
> Diffs
> -----
> 
>   global-presence-chooser.cpp da6a87b618fe367c7377544a7acd800f4103a749 
> 
> Diff: https://git.reviewboard.kde.org/r/120303/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140922/aa6aba17/attachment.html>