Review Request 123482: ktp-common-internals status handler integration and global / multiple account presence fixes

[Martin Klapetek]

> On Feb. 29, 2016, 8:45 p.m., Martin Klapetek wrote:
> > I'm not really sure I want to have this incredibly complex feature-set for the little gain it provides.
> > 
> > I don't know what your overall goal with this set of changes actually is as there is no explanation
> > anywhere, just a bunch of patches. But I assume it has to do with the kded plugins and per-account
> > presences. I would actually like to just kill the kded plugins and leave just auto-away. And the plugins
> > presence handling actually becomes very simple - it's either set by a plugin or not. I don't see a need
> > for a whole dbus infrastructure to mess with presences.
> > 
> > As for your StatusHandlerController, that seems kinda useless. You either controll all accounts by
> > using the GlobalPresence, or you're using separate account presences, at which point the GlobalPresence
> > should simply have two states. You don't need no special class nor dbus interfaces to control per-account
> > presences, you can do that directly.
> > 
> > So, to summarize:
> > 
> > * GlobalPresence should have a "global" and "non-global" state
> > * All our presence controls should have the same two modes
> > * Setting the non-global mode from the UI will disable setting all accounts to the same presence
> > * Setting global presence from the UI will again set everything
> > * Auto-away overrides all presences
> 
> James Smith wrote:
>     The KDE global / account presence setting should as far as possible be done compatibly with how TelepathyQt can accept incoming presences from other IM applications. This increases complexity a bit because we have to take into account our own automatic presences, and on top of this we do our own auto connect. This patchset throws in control and monitor functions that cover what sticking to the basics doesn't actually allow for, in addition to changing some existing shortcomings, and addressing other noticeable flaws.. Having a non-global mode to set is probably not a step in the right direction.

> Having a non-global mode to set is probably not a step in the right direction.

So is not adding 3000 tons of complexity for 3 (effectively 2)
presence plugins that the kded has.

I wanted the mpris plugin long gone, it's just not useful anymore,
which leaves auto-away and screensaver-away that can be merged into
one.

I'm all for fixing our current code by simplifying things, but I'm sorry
I will not accept *this* much code for handling what effectively should
be a single presence plugin that overrides everything.

I'd rather we spent our time on making things simpler and focusing on what
actually matters - like Telegram support - and remove old stuff that should
just die already.


- Martin


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


On March 7, 2016, 7:53 a.m., James Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123482/
> -----------------------------------------------------------
> 
> (Updated March 7, 2016, 7:53 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> -------
> 
> New classes:
> -StatusHandlerPresence, an interface for StatusHandler PluginQueue and AccountStatusHelper DBus interfaces, and a number of control functions for the Presence / StatusMessageInserter plugins (per-app save / restore presence replacement).
> 
> New properties:
> -GlobalPresence: bool activeAutoPresence(), bool mixedPresenceMode(), KTp::Presence globalRequestedPresence().
> 
> New methods:
> -GlobalPresence: onAccountDisabled(Tp::AccountPtr)
> 
> New types:
> KTp::PluginQueue, for indicating the state of the plugin queue plugins, e.g. PluginDisabledOnLoad, PluginDisabled, PluginEnabled, PluginActive, etc.
> 
> New features:
> -pluginQueue active / inactive information in GlobalPresence, separate presenceQueueActiveChanged and statusMessageQueueActiveChanged signals.
> -More reliable account property signalling.
> -Reworked account enable / disable.
> -Presence priority improvements.
> 
> Other simple cleanups (Use KTp::Presence where possible, make function arguments conform, commenting clarifications, errant properties, etc.)
> 
> 
> Diffs
> -----
> 
>   KTp/CMakeLists.txt 85d578bc3cbc0ae5769e6ff9f466381da4e701a7 
>   KTp/Declarative/qml-plugins.cpp 4956a1c94fe60a526367f6a74a357d510e74e519 
>   KTp/Models/presence-model.cpp ddc1a7c75f1a452bf3ac2db1aecbd88a5d1ce519 
>   KTp/global-presence.h 46e6cc358b0f3cdaff35d3a7be25949646b400d3 
>   KTp/global-presence.cpp 8f1148b4ec339ab77f9c2fca0c87429ac14c5194 
>   KTp/presence.h 7da07d0120c75ed057f4b2f2f832f5a1f80bd528 
>   KTp/presence.cpp 6030db0ed821a4399603b5453ab124c741912a1f 
>   KTp/status-handler-presence.h PRE-CREATION 
>   KTp/status-handler-presence.cpp PRE-CREATION 
>   KTp/types.h 27eb8975a4149e78c1e545b7273572c7eb18bad3 
> 
> Diff: https://git.reviewboard.kde.org/r/123482/diff/
> 
> 
> Testing
> -------
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

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