Version: (using Devel) Compiler: g++ 4.3.3 OS: Linux Installed from: Compiled sources When calling org.kde.VisualNotifications.Notify the "replaces_id" argument is used to indicate that a visual notification replaces the notification with the id "replaces_id". This is used by KNotification::update -> KNotificationManager::update However using this argument acts as if the argument was set to "0", and always creates a new visual notification. The expected behaviour is for plasma to replace the notification with id "replaces_id" with the new notification. I've verified all of this with gdb.
I should also point out that in KNotification replaces_id and event_id are identical. i.e. the event should replace itself. Since this is the main interface through to the plasma VisualNotification for most kde programmers I guess it's fairly important this is fixed.
Sorry bad timing but I just followed the code: kDebug() << "notice: updating notifications isn't implemented yet"; Are you happy for me to implement this and submit a patch to you?
> Are you happy for me to implement this and submit a patch to you? absolutely; i'd be happy to look over the patch pre-commit as well :)
The patch was simple. There are two cases, the previous case where prev_id = 0, and a new id should be generated, or the case that prev_id is an existing event, in which case prev_id will be equal to some existing event. In the case that prev_id is set, then the call to setData contains the source "notification " + id. If prev_id is equal to an existing id, the call to setData will locate the existing DataEngine::Data through this source identifier and update it, then update the monitors through dbus. I made this patch using svn diff in kdebase.
Created attachment 31796 [details] patch (apply in kdebase, was made from svn diff in kdebase) Thanks
I have written a use case for this patch - see bug 182163
This patch looks so bare because event updating is already supported by plasma/dataengines.h
wow, got to love it when fixing bugs and implementing outstanding TODOs consists of removing code. ;) it looks good except for two minor issues, neither of which are the fault of your patch though: "unknown app" should be "Unknown Application" and please remove the extra space before the closing )s on the insert() lines. may as well fix that while we're in there, right? :) in any case, this should be committed to svn. do you have an svn account or do you need someone to commit it for you? (and thanks for the patch, btw! :)
I'd really like an svn account, as I plan on contributing much more from now on. I fixed this whilst writing improvements to the kopete notification system in the bug you can see, I'm hoping to be able to contribute that also. I've fixed those issues you mentioned, I can upload a new patch or let me know the svn account :) Cheers, James
Oh if you could call the account "tuxjay". Sorry I shouldn't be using bug updates for things like this.
James: visit http://techbase.kde.org/Contribute/Get_a_SVN_Account and fill out the form :) you can put my email in for your supporting KDE contact and i'll approve it when it arrives in my inbox. welcome to kde :)
SVN commit 935717 by jpike: Implementation for udpating visual notifications. FEATURE: 186096 M +17 -26 notificationsengine.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=935717