> On Feb. 15, 2014, 1:38 p.m., David Edmundson wrote: > >
If the kded is used to enable / disable nowplaying using the contactlist to additionally enable / disable nowplaying or enabling nowplaying while set on a custom presence the contactlist selector completely borks. The kded module in master now defaults status + message plugins highest priority followed by user-specified custom presences and then message plugin only messages. This was the better approach eg fallback benefits of user over automatic, automatic over empty status messages, force the user to disable plugins for empty status message availability, custom status over message plugin for user-take-control, automatic message over empty automatic presence + message etc. Also the addition of new status message plugins will require rethinking nowplaying's prominence and enable / disable mechanisms. This approach had the additional benefit of solving a number of status-sticking bugs in the contact list selector, but exposed bugs in the nowplaying activation handling when changing from a custom user presence by starting nowplaying. Nowplaying activation now needs a consistency between the contact list and the kded module and the kconfig to properly function. There are 2 approaches: 1) Modify the kded code to pick up the old behavior i.e. all state-sticky bugs and functioning nowplaying interface in the contact list. problems - sometimes returns from busy-away status after track change to the wrong status message, depending on in what state now playing was activated from. Sometimes sticks on custom presences when clearing to available status. 2) Change the now playing engaging behavior in the contactlist, to also lessen the visual priority like what kded module now does internally. problems - contact list won't activate nowplaying from a custom user presence, to get an empty available status message, all custom status message plugins must be disabled. > On Feb. 15, 2014, 1:38 p.m., David Edmundson wrote: > > global-presence-chooser.cpp, line 372 > > <https://git.reviewboard.kde.org/r/115425/diff/3/?file=244160#file244160line372> > > > > So the summary of this patch is that you're removing the > > activateNowPlaying signals and instead relying on changing the config and > > sending a configChanged method. > > > > The slots still exist in the kded so this is very half finished. > > > > It seems like all of these changes are just going round in circles > > changing things rather than coming up with a understanding of how things > > are meant to work and coming up with a solid design. > > > > Martin: now playing is your area, can you please comment. There have > > been several people randomly hacking on this now and things are still > > broken. Can you please comment and fix this mess. > > What is the purpose of the config and the nowPlaying signals? How is it > > /meant/ to work? As long as the kded dbus interface isn't used elsewhere it's probably redundant code originally there to trigger the first status update after enabling the nowplaying and essentially provide control over a semi-autonomous process thereafter. The kconfig method (functionally) duplicates this transparently enough where nowplaying will enable when the config entry is updated and self-terminate when respectively disabled. The slots in the kded don't set the nowplaying config either, so its a fairly overlapping implementation. Can we remove the dbus logic or at least properly set the kconfig from it? We can have kconfig write the current enable status from activatenowplaying / deactivatenowplaying but that still requires kconfig also to a) check whether the plugin is enabled, and b) also to write out enabled status to kconfig, while still using dbus to activate / deactivate the plugin. > On Feb. 15, 2014, 1:38 p.m., David Edmundson wrote: > > global-presence-chooser.cpp, line 345 > > <https://git.reviewboard.kde.org/r/115425/diff/3/?file=244160#file244160line345> > > > > Are you on our mailing list? > > > > We're in a string freeze so this can't go in 0.8. > > > > Why do we want want to show a prompt when you chose something else? The nowplaying selector was partially broken by changes to the kded module. The prompt toggles activation / deactivation. The plugin requires deactivation of kded module plugin to reset to an empty status message. I'd rather a short generic prompt for enabling, so multiple plugins can reuse the same string. The status message plugin stack has a priority-based approach identical to the status plugin stack and enabling a plugin doesn't guarantee that it's status message will be at the top of the stack. The string freeze might require reverting #114569 to ship a functional contact list. If one doesn't mind changing status to a preset with a blank presence message in order to turn on now playing, it should function fine until more work can be done on the contact list. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115425/#review49821 ----------------------------------------------------------- On Feb. 15, 2014, 1:25 p.m., James Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115425/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2014, 1:25 p.m.) > > > Review request for Telepathy and Martin Klapetek. > > > Repository: ktp-contact-list > > > Description > ------- > > Enables / disables Now Playing in systemsettings every time it is enabled / > disabled in the contact list. > > Fixes systemsettings kcm showing nowplaying enabled while the contact list > has disabled its functionality. > > > Diffs > ----- > > global-presence-chooser.cpp 2047473 > > Diff: https://git.reviewboard.kde.org/r/115425/diff/ > > > Testing > ------- > > Compile, run. > > > Thanks, > > James Smith > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
