After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 571685 - default vfunc handler for signals; and specifying SignalFlags.
default vfunc handler for signals; and specifying SignalFlags.
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Code Generator
0.5.x
Other All
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
: 590200 (view as bug list)
Depends on: 574403 575475
Blocks:
 
 
Reported: 2009-02-13 21:13 UTC by rainwoodman
Modified: 2010-03-24 08:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for improved handling of signal handlers. (2.64 KB, patch)
2009-02-13 21:14 UTC, rainwoodman
reviewed Details | Review
Allow function body. (7.78 KB, patch)
2009-03-22 21:49 UTC, rainwoodman
none Details | Review
check the default handler method instead of the method body. (1004 bytes, patch)
2009-03-23 00:44 UTC, rainwoodman
none Details | Review
ref and out for signals (1017 bytes, patch)
2009-03-23 01:55 UTC, rainwoodman
committed Details | Review
member access of overriden signal should refer to the base signal. (3.07 KB, patch)
2009-03-23 01:56 UTC, rainwoodman
none Details | Review
signal body for virtual signal as default handler (7.78 KB, patch)
2009-03-23 01:57 UTC, rainwoodman
none Details | Review
check the default handler (1004 bytes, patch)
2009-03-23 01:58 UTC, rainwoodman
none Details | Review
[Signal (has_emitter, stage)] (3.67 KB, patch)
2009-03-23 01:59 UTC, rainwoodman
none Details | Review
remove vfunc prototypes for default signal handlers (1.72 KB, patch)
2009-03-23 02:00 UTC, rainwoodman
none Details | Review
remove the vfunc bodies for default signal handlers (1.81 KB, patch)
2009-03-23 02:01 UTC, rainwoodman
none Details | Review
the emitter when has_emitter=true (1.91 KB, patch)
2009-03-23 02:02 UTC, rainwoodman
none Details | Review
combine all patches. (14.00 KB, patch)
2009-03-28 19:44 UTC, rainwoodman
none Details | Review
define the default handler in signal body. (14.03 KB, patch)
2009-03-29 17:51 UTC, rainwoodman
none Details | Review
Rebased against master (12.72 KB, patch)
2009-07-28 14:50 UTC, Jürg Billeter
reviewed Details | Review
Pending part of the patch (7.03 KB, patch)
2009-07-28 15:57 UTC, Jürg Billeter
none Details | Review

Description rainwoodman 2009-02-13 21:13:07 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.
Comment 1 rainwoodman 2009-02-13 21:14:27 UTC
Created attachment 128689 [details] [review]
patch for improved handling of signal handlers.
Comment 2 rainwoodman 2009-02-13 21:17:39 UTC
An alternative solution is to allow function body for signals.
Comment 3 Jürg Billeter 2009-03-09 22:36:22 UTC
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'.
Comment 4 rainwoodman 2009-03-10 06:14:01 UTC
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.
Comment 5 rainwoodman 2009-03-22 21:49:34 UTC
Created attachment 131143 [details] [review]
Allow function body.

Depends on the patch for bug 575475
Comment 6 rainwoodman 2009-03-23 00:44:20 UTC
Created attachment 131148 [details] [review]
check the default handler method instead of the method body.

Follows patch 131143, fixing errors regarding the formal parameters.
Comment 7 rainwoodman 2009-03-23 01:55:12 UTC
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.
Comment 8 rainwoodman 2009-03-23 01:56:05 UTC
Created attachment 131152 [details] [review]
member access of overriden signal should refer to the base signal.
Comment 9 rainwoodman 2009-03-23 01:57:09 UTC
Created attachment 131153 [details] [review]
signal body for virtual signal as default handler
Comment 10 rainwoodman 2009-03-23 01:58:20 UTC
Created attachment 131154 [details] [review]
check the default handler

Fixing an bug in patch 131153.
Comment 11 rainwoodman 2009-03-23 01:59:16 UTC
Created attachment 131155 [details] [review]
[Signal (has_emitter, stage)]

has_emitter = true / false
stage = "FIRST" / "LAST" / "CLEANUP"
Comment 12 rainwoodman 2009-03-23 02:00:16 UTC
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.
Comment 13 rainwoodman 2009-03-23 02:01:03 UTC
Created attachment 131157 [details] [review]
remove the vfunc bodies for default signal handlers

follow-up of patch 131156
Comment 14 rainwoodman 2009-03-23 02:02:14 UTC
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.
Comment 15 rainwoodman 2009-03-26 15:26:48 UTC
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?
Comment 16 rainwoodman 2009-03-28 19:44:57 UTC
Created attachment 131590 [details] [review]
combine all patches.

A combined patch might be easier for the reviewer.
Comment 17 rainwoodman 2009-03-29 17:51:57 UTC
Created attachment 131637 [details] [review]
define the default handler in signal body.

Rebased against origin.
Comment 18 rainwoodman 2009-05-19 22:58:44 UTC
No longer applies to the master branch.
Problem:
the anonymous methods are not visited by get_methods().
Comment 19 Jürg Billeter 2009-07-28 14:50:32 UTC
Created attachment 139386 [details] [review]
Rebased against master
Comment 20 Jürg Billeter 2009-07-28 15:53:43 UTC
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.
Comment 21 Jürg Billeter 2009-07-28 15:57:07 UTC
Created attachment 139394 [details] [review]
Pending part of the patch
Comment 22 rainwoodman 2009-07-29 09:42:38 UTC
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.

Comment 23 Jürg Billeter 2009-08-01 15:37:04 UTC
*** Bug 590200 has been marked as a duplicate of this bug. ***
Comment 24 rainwoodman 2009-11-06 16:34:38 UTC
The default handler sometimes behave weirdly. 

Just to keep a note here. No conclusive result was found yet.
Comment 25 Jürg Billeter 2010-03-24 08:58:11 UTC
The stage support has been added in the meantime. I don't intend to support HasEmitter for more than bindings for now.