Community
Participate
Working Groups
Using the build service messages as example we have two types of events and two corresponding sinks. Normally we would want the build service events to show up in the builds view and the build plan events in the notification pop-ups. Because of ongoing improvements to the notification API it is possible for the user to specify which events to display where. By default all events are displayed everywhere as we currently have no means to do otherwise. Hence it should be possible to specify which event types to display by default when declaring a sink. Allowing a wild-card would also be useful. If we for instance want all builds notifications in the pop-up we could declare it something like this: <extension point="org.eclipse.mylyn.commons.notifications.notifications"> <sink class="org.eclipse.mylyn.internal.commons.ui.notifications.popup.PopupNotificationSink" id="org.eclipse.mylyn.commons.notifications.sink.Popup" label="Desktop Popup"> <include> <event match="org.eclipse.mylyn.builds.ui.events.*"/> </include> </sink> </extension> Or if we just want the build plan events: <include> <event match="org.eclipse.mylyn.builds.ui.events.PlanStatusChanged"/> </include> It should be possible to use regular expressions in the "match" clause.
Considering that one would typically have only a few notification sinks and maybe just reuse what is already defined; it makes more sense that one on an event declaration would be able to specify which default sinks to use. After all it is more likely that new events are added than new sinks. Changed subject accordingly.
Created attachment 185692 [details] Patch to fix the issue Default sinks to use for a particular event can now be specified in the event type declaration. For instance: <extension point="org.eclipse.mylyn.commons.notifications.notifications"> <event categoryId="org.eclipse.mylyn.builds.ui.category.Builds" id="org.eclipse.mylyn.builds.ui.events.PlanStatusChanged" label="Plan Status Changed"> <description/> <defaultHandler sink="org.eclipse.mylyn.commons.notifications.sink.Popup"> </defaultHandler> </event>
Created attachment 185693 [details] mylyn/context/zip
(In reply to comment #2) > Default sinks to use for a particular event can now be specified in the event > type declaration. For instance: Note that if no default sinks are specified, all will be used. So this will not break existing code.
Sounds like a good idea to me and a regular expression should work well. To keep it simple I would suggest that we add an optional filter tag under the sink extensions: <filter match="..."/>. In theory we could use the core expression extension schema to allow complex expressions but as a start I think the proposed approach would work well.
Ok... So for each "sink" you want to be able to specify which "event" it will per default handle? That was my initial idea. But I later on (when I actually started implementing this) I found that it may not work so good. It would mean that we would probably have to change the declaration of the current notification popup (org.eclipse.mylyn.commons.notifications.sink.Popup) for each new event added that will use it. For API consumers that want to use notifications that could be a problem. Which why I ended up with the implementation described in comment #1 and comment #2. Now we could implement the initial proposal with the changes in comment #5 but I don't see the need for it unless there is a reason why the patch won't work.
Good point. We could solve this through an extension point that maps event types to sinks, e.g. <eventMapping sinkId="org.eclipse.mylyn.commons.notifications.sink.Popup" eventIds="org.eclipse.mylyn.builds.ui.events.*" </eventMapping> If no mapping is specified for a particular event it would be handled by all sinks.
Ok, I can add this. I think we should keep the mapping/handler mechanism described in comment #2 though. Except adjust it a bit so the attribute names are the same.
Created attachment 185907 [details] adds support for "eventMapping" extension In addition to the optional "defaultHandler" element in "event" it is now possible to add an "eventMapping" extension as described in comment #7. The extension point document explains use and gives an example.
Thanks! I have applied the patch.
Reopening to fix assignment.
Closing.
