GNOME Bugzilla – Bug 669272
Emit a signal to notify signal activation
Last modified: 2012-02-17 21:24:26 UTC
Created attachment 206659 [details] [review] Emit signal on signal activation Anjuta-glade integration can be greatly improved if glade notifies anjuta whenever a signal is activated in the signal editor. Note that this patch mimics closely the inspector behavior. Please review it.
Please reject this patch for now as it contains some errors and is not good enough. I'll come up with a more appropriate one ASAP.
Created attachment 207667 [details] [review] Send signal activated signal. The attached makes glade signal editor emit a signal when a signal is double-clicked (activated). This will be used for a future feature for glade-anjuta integration. Note that the behavior mimics the one already present in the glade inspector (and used by anjuta[1]) while the emitted data mimics the one used for the signal drop feature of the glade-anjuta integration. Please review it. [1] http://git.gnome.org/browse/anjuta/commit?id=5abe8fd3a97d802c50ce7ac036252aba24feba6c
Review of attachment 207667 [details] [review]: In general it seems reasonable to me to have a "signal-activated"-signal whatever we do with it but it should have the following signature: void on_signal_activated (GladeSignalEditor *editor, GladeSignal *signal, gpointer user_data). ::: gladeui/glade-signal-editor.c @@ +822,3 @@ + 0, signal_data, project); + g_object_unref (signal); + g_free (signal_data); Using the same data as for the dragging isn't a good idea. As this is a signal anyway, you can directly pass the GladeSignal pointer to the callback - no need to put all this in a string. The string is just a work-around because with drag&drop it is tricky to pass arbitary data around.
Review of attachment 207667 [details] [review]: We might need to add a new function to GladeSignalEditor glade_signal_editor_get_widget() to give access to the widget, this way we do not even need to pass the GladeProject object. BTW wont we need a signal-updated signal? ::: gladeui/glade-signal-editor.c @@ +818,3 @@ + gchar *signal_data = g_strdup_printf ("%s:%s:%s:%s:%s:%s", + widget_type_name, signal_name, signal_handler, signal_object, + signal_swap?"1":"0", signal_after?"1":"0"); Please define variables in the start of the block, we try to keep it ansi C. @@ +1047,3 @@ + G_SIGNAL_RUN_LAST, + 0, + NULL, NULL, NULL, G_TYPE_NONE, 2, You can use g_cclosure_marshal_VOID__OBJECT here and use the GladeSignal object directly as suggested by Johannes, also update the type accordingly to GLADE_TYPE_SIGNAL, GladeProject is not needed if we add a new function glade_signal_editor_load_widget()
Created attachment 207759 [details] [review] Send signal activated signal. The attached patch implements changes requested in previous comments. Please review it.
Created attachment 207776 [details] [review] Emit signal on signal activation Attached patch is the same as the previous one but adds a runtime check. Please commit it.
Review of attachment 207776 [details] [review]: Looks good
Patch looks fine to me too, as I discussed with Marco, perhaps he's going to want to listen to signal-added/signal-removed signals on GladeWidget for the purpose of actually creating and removing user code to handle it... the action of actually activating the signal row in the editor can always also be useful to frontends like Anjuta... so this added signal is a good thing in any case. Juan can you get this into master branch since you seem to be doing some releases ?
Created attachment 207812 [details] [review] Send signal activated signal. The attached patch fixes a check in the previous one. Please commit it.
Commited in master