That looks like an interesting solution. I don't think that we should support add/remove as that's not common in GObject libraries and I also don't see a need for it.

Something else we should support is to have a vfunc as default signal handler and a nice syntax to override that default handler in derived classes.

We probably also need to support at least the FIRST/LAST/DETAILED signal flags and the AFTER connect flag.

(In reply to comment #1)
> That looks like an interesting solution. I don't think that we should support
> add/remove as that's not common in GObject libraries and I also don't see a
> need for it.

Yes agreed. Just added them for completeness. We should not add this, unless someone comes up with an use-case.

> Something else we should support is to have a vfunc as default signal handler
> and a nice syntax to override that default handler in derived classes.

Thought it would be easy:

public class Object {
    public signal void notify(ParamSpec! property) {
        virtual void emit(string! property_name) {
        }
    }
}

for declaration and:

public class Maman.Bar : Object {
    public signal void notify(ParamSpec! property) {
        override void emit(string! property_name) {
        }
    }
}

for override, when I remembered the initial purpose of this ticket: Conflicting signatures. So what about this:

public class Object {
    public virtual signal void notify(ParamSpec! property) {
        default { // NOTICE: use "default" to avoid new keyword
        }
    }
}

public class Maman.Bar : Object {
    public override signal void notify(ParamSpec! property) {
        default { // NOTICE: use "default" to avoid new keyword
        }
    }
}

Or:

public class Object {
    public signal void notify(ParamSpec! property) {
        virtual default { // NOTICE: use "default" to avoid new keyword
        }
    }
}

public class Maman.Bar : Object {
    public signal void notify(ParamSpec! property) {
        override default { // NOTICE: use "default" to avoid new keyword
        }
    }
}

Difference is in position of the virtual/override modifier. 
Do we even need that modifier - expect for consistence?

> We probably also need to support at least the FIRST/LAST/DETAILED signal flags

Without new keywords:

    [SignalFlags.RUN_FIRST | SignalFlags.DETAILED]
    public signal void notify(ParamSpec! property);

or more redundant, but consistent with current attribute usage:

    [SignalFlags(SignalFlags.RUN_FIRST | SignalFlags.DETAILED)]
    public signal void notify(ParamSpec! property);

or with absolutly smart parsing, which changes identifier scope, when detecting the attribute actually being a flag:

    [SignalFlags(RUN_FIRST, DETAILED)]
    public signal void notify(ParamSpec! property);

Alternativly signals would be avoided by something like this:

    public signal void notify(ParamSpec! property) {
        flags = SignalFlags.RUN_FIRST | SignalFlags.DETAILED;
    }

> and the AFTER connect flag.

I still thinks this should be implemented:

    signal_name["detail_name"].flag += ....

Alternativly:

    signal_name["detail_name", flags] += ....


Guess I even prefer this second variant, which should looks more pretty to me and should be easy to implement. Specially when allowing to drop the flag's typename:

    notify["bar", AFTER | SWAPPED] += ....

or (would make the code generator even more trivial):

    notify["bar", AFTER, SWAPPED] += ....
    notify[AFTER "bar", SWAPPED,] += ....
    notify[AFTER, SWAPPED, "bar"] += ....

Code would be like this:

    foreach (Expression index in expr.indices) {
        if (index is StringLiteral) {
            if (null == detail) {
                detail = ((StringLiteral)index).value;
            }
                Report.error (...);
            }
        } else if (index.static_type.data_type.is_subtypeof (connect_flags_type)) {
            ....
            cflags = new CCodeBinaryExpression (CCodeBinaryOperator.OR, cflags, index.ccodenode);
            ....
        }
    }

Using the default keyword might make sense. As the default handler is the only thing you can override, I wonder whether we should just use

    public override signal void notify(ParamSpec! property) {
        ...
    }

in derived classes. Don't know whether that syntax combination would make sense, though.

I don't think we should ever expose SWAPPED as we don't expose user_data. Something like

    notify["bar"].after += ...

makes more sense, IMO.

(after being a member of all signals, not a keyword)

(In reply to comment #3)
> Using the default keyword might make sense. As the default handler is the only
> thing you can override, I wonder whether we should just use
> 
>     public override signal void notify(ParamSpec! property) {
>         ...
>     }
> 
> in derived classes. Don't know whether that syntax combination would make
> sense, though.

Looks convenient, as it requires less code, but:

 - violates the principle of minimal surprise, as this syntax differs from the syntax for base classes
 - closes the door for overriding other signal related methods, like emit or add/remove (when ever added)

> I don't think we should ever expose SWAPPED as we don't expose user_data.

Exposing swapped makes sense for attaching static methods and for directly attaching the current instances hide methode to some button's click signal:

   button.click.swapped += hide;

Without SWAPPED the button would disappear on click.

(In reply to comment #4)
>  - violates the principle of minimal surprise, as this syntax differs from the
> syntax for base classes

Yes, that is also my concern. I'd like to get overriding a default signal handler easy as it's probably more common than using a custom emitter, for example when deriving a Widget.

>  - closes the door for overriding other signal related methods, like emit or
> add/remove (when ever added)

I haven't seen any virtual emit function yet, we probably don't need to support that. I'm not really conerned about add/remove.

> > I don't think we should ever expose SWAPPED as we don't expose user_data.
> 
> Exposing swapped makes sense for attaching static methods and for directly
> attaching the current instances hide methode to some button's click signal:
> 
>    button.click.swapped += hide;
> 
> Without SWAPPED the button would disappear on click.

That should be handled internally by Vala, in my opinion. I consider the position of the self/this, sender, and user_data parameters as implementation details. Your SWAPPED example only works for the special case of parameter-less methods/signals.

(In reply to comment #5)
> > Exposing swapped makes sense for attaching static methods and for directly
> > attaching the current instances hide methode to some button's click signal:
> > 
> >    button.click.swapped += hide;
> > 
> > Without SWAPPED the button would disappear on click.
> 
> That should be handled internally by Vala, in my opinion. I consider the
> position of the self/this, sender, and user_data parameters as implementation
> details. Your SWAPPED example only works for the special case of parameter-less
> methods/signals.

I was going to saying that without a crystal ball you cannot know that. Well but considering the semantics of

    button.click += this.button_click_cb;

it is absolutly clear, that
 
    button.click += this.hide;

shall call the hide method of the current widget. To connect to the button's hide method you'd write:

    button.click += button.hide;

*** Bug 509736 has been marked as a duplicate of this bug. ***

I'm currently tending to the following proposal.

Default handlers:

public class Object {
    public virtual signal void notify(ParamSpec! property) {
        default {
        }
    }
}

public class Maman.Bar : Object {
    public override signal void notify(ParamSpec! property) {
        default {
        }
    }
}

Emitters:

public class Object {
    public signal void notify(ParamSpec! property) { emit; }
}

Signals declared without "emit" may only be emitted from methods in the same class.

notify signal emitter gets a string as far as I know.
Isn't better to have something like:
public class Object {
    public signal void notify(ParamSpec! property) { emit(String! property); }
}

2008-05-29  Jürg Billeter  <j@bitron.ch>

	* vala/valamethod.vala:
	* vala/valaparser.vala:
	* vala/valasignal.vala:

	Add support for overriding default method handlers of signals
	that have been declared with the `virtual' modifier

Bug 571685 is related to this one.

Notice that in vala 0.5.7, even if a signal is declared as virtual, no corresponding vfunc is ever created, and no default handler is set in g_signal_new.

Handling of virtual signals has been fixed in the meantime. I'm not planning to add explicit emitter in the foreseeable future.