GNOME Bugzilla – Bug 571685
default vfunc handler for signals; and specifying SignalFlags.
Last modified: 2010-03-24 08:58:11 UTC
Please describe the problem: vala was unable of doing both. The patch implements these two features with two CCode attributes on signals: vfunc_name : the cname of the vfunc handler; if not specified, no default handler is used in g_signal_new(the old behavior) flags : an ccode constant of the flags (G_SIGNAL_RUN_LAST | ...) if not specified, G_SIGNAL_RUN_LAST is used (the old behavior); Steps to reproduce: The test case for the patch: public class Something { public virtual void first_default_handler(int number) { message("first default_handler"); } [CCode (vfunc_name = "first_default_handler", flags="G_SIGNAL_RUN_FIRST")] public signal void first(int number); public virtual void last_default_handler(int number) { message("last default_handler"); } [CCode (vfunc_name = "last_default_handler")] public signal void last(int number); } public int main(string[] args) { Something obj = new Something(); Signal.connect(obj, "first", (GLib.Callback)callback, "first"); Signal.connect(obj, "last", (GLib.Callback)callback, "last"); obj.first(100); obj.last(101); return 0; } public void callback(Something instance, int number, string foo) { message("%d %s", number, foo); } Actual results: Expected results: Test case output: ** Message: signal.vala:3: first default_handler ** Message: signal.vala:24: 100 first ** Message: signal.vala:24: 101 last ** Message: signal.vala:9: last default_handler Does this happen every time? Other information: the patch follows.
Created attachment 128689 [details] [review] patch for improved handling of signal handlers.
An alternative solution is to allow function body for signals.
Vala already has support for signals with default handlers in bindings with a different syntax, so we should use the same syntax for actual implementations. public virtual signal void last_default_handler(int number) { message("last default_handler"); } The reason I chose that syntax is that the name of the default handler is usually the same as the signal name, and I don't think string attributes are the right means to connect signal declarations with implementations. Using attributes for signal flags as in your example makes sense, although we should probably use a less generic attribute name than `flags'.
Thanks Jurg. Therefore I should not rely on the default signal handler and should not use any workarounds until the feature is implemented. If there is no plan on implementing, I'll try to implement this syntax in a few weeks.
Created attachment 131143 [details] [review] Allow function body. Depends on the patch for bug 575475
Created attachment 131148 [details] [review] check the default handler method instead of the method body. Follows patch 131143, fixing errors regarding the formal parameters.
Created attachment 131151 [details] [review] ref and out for signals It might be better to provide all of my patches regarding signals in a row under this bug, since they are all related. git formats eight of them.
Created attachment 131152 [details] [review] member access of overriden signal should refer to the base signal.
Created attachment 131153 [details] [review] signal body for virtual signal as default handler
Created attachment 131154 [details] [review] check the default handler Fixing an bug in patch 131153.
Created attachment 131155 [details] [review] [Signal (has_emitter, stage)] has_emitter = true / false stage = "FIRST" / "LAST" / "CLEANUP"
Created attachment 131156 [details] [review] remove vfunc prototypes for default signal handlers Only the prototypes are removed. The function bodies are removed in the following patch.
Created attachment 131157 [details] [review] remove the vfunc bodies for default signal handlers follow-up of patch 131156
Created attachment 131158 [details] [review] the emitter when has_emitter=true This is not finished. The emitter body is empty and I don't know how to associate the arguments of g_signal_emit_by_name.
Test case: -------------- using Gtk; public class SignalTester: Gtk.Container { public SignalTester() { Requisition r; size_request(out r); } public override void size_request(out Requisition r) { base.size_request(out r); } [Signal (stage="FIRST")] public virtual signal void first(string p){ message("hello form first, %s", p); } [Signal (stage="LAST")] public virtual signal void last(string p){ message("hello from last, %s", p); } [Signal (stage="CLEANUP")] public virtual signal void cleanup(string p){ message("hello from cleanup, %s", p); } [HasEmitter] public virtual signal void emitter(string p) { message("hello from emitter, %s", p); } } private void handler(SignalTester tester, string p) { message("customized handler on %s", p); } public int main(string[] args) { SignalTester t = new SignalTester(); t.first += handler; t.last += handler; t.cleanup += handler; t.emitter += handler; t.first("first"); t.last("last"); t.cleanup("cleanup"); t.emitter("emitter"); return 0; } -------------- t.emitter("emitter") line fails because of comment 14. Could anyone help me on this?
Created attachment 131590 [details] [review] combine all patches. A combined patch might be easier for the reviewer.
Created attachment 131637 [details] [review] define the default handler in signal body. Rebased against origin.
No longer applies to the master branch. Problem: the anonymous methods are not visited by get_methods().
Created attachment 139386 [details] [review] Rebased against master
commit 9b37307b035e75c858ed648728db83a8bc70a4c8 Author: Jürg Billeter <j@bitron.ch> Date: Tue Jul 28 17:20:37 2009 +0200 Support virtual default handler for signals Based on patch by Yu Feng, fixes part of 571685.
Created attachment 139394 [details] [review] Pending part of the patch
The pending part is about two issues: 1: the HasEmitter attribute. HasEmitter was introduced for bindings. Do we add also an emitter for the code generated by valac? This is necessary when replacing C-written code with Vala-written code while preserving the API. 2: the stage. What is the preferred way to specify the default handler's stage? I suggest using CCode attributes, because it is a specific issue with GObject.
*** Bug 590200 has been marked as a duplicate of this bug. ***
The default handler sometimes behave weirdly. Just to keep a note here. No conclusive result was found yet.
The stage support has been added in the meantime. I don't intend to support HasEmitter for more than bindings for now.