>
> Thanks!
>
> Looks there are no real objections against the proposal but I am rather concerned about general lack of activity during review. As a compromise, what about putting it in "candidate" category (http://code.dlang.org/?sort=updated&category=library.std_aspirant) and making 1-2 month pause before proceeding to actual voting? That will also fix minor issue of me being a bit busy and not paying proper attention :)

Well it was already on the registry for a few months, I don't really see what another two months would gain. Considering the current state of std.signals and how nothing has happened to it in years, it does not seem that there is a huge amount of people interested in signals for D, I also don't see this to change within the next two months.

I personally would like to continue going on to voting and get it over with, so I can clear this item from my todo list ;-)

That being said, I of course understand if you are busy right now, a problem well known, so in the end it is your call. Thanks again for doing this in the first place!

Best regards,

Robert

On Friday, 22 November 2013 at 16:07:56 UTC, robert wrote:
> Well it was already on the registry for a few months, I don't really see what another two months would gain.

With a confusing name and not in expected category ;) I think renaming it to be visible by "signal" search query in package list + making D.announcement with "please please try to use it!" should make things a bit better. I personally can
get back to this stuff only by the middle-end of December so there is no much else  that can be done right now :(

Robert, please add `RestrictedSignal(Args...).strongConnect(void function(Args) dg)` method or at least add link to the `std.functional.toDelegate` function.
In current situation your example doesn't work (easy to fix via toDelegate):

Error: function phobosx.signal.RestrictedSignal!(string, int).RestrictedSignal.strongConnect (void delegate(string, int) dg) is not callable using argument types (void function(string, int))

On Friday, 22 November 2013 at 18:20:37 UTC, ilya-stromberg wrote:
> Robert, please add `RestrictedSignal(Args...).strongConnect(void function(Args) dg)` method or at least add link to the `std.functional.toDelegate` function.
> In current situation your example doesn't work (easy to fix via toDelegate):
>
> Error: function phobosx.signal.RestrictedSignal!(string, int).RestrictedSignal.strongConnect (void delegate(string, int) dg) is not callable using argument types (void function(string, int))

That's weird. It works in the unit test, I'll look into it. Thanks!

On Friday, 22 November 2013 at 18:33:31 UTC, Robert wrote:
> That's weird. It works in the unit test, I'll look into it. Thanks!

No, it isn't.
The function `void watch(string msg, int i)` from unit tests is declarated into the unittest block. So, it's delegate because it contains pointer to the unittest scope.
Try to change function to the `static void watch(string msg, int i)` and you'll see the same error.

Sorry, I completely missed this message. Thanks for the hint, I fixed the documentation.

Best regards,

Robert

>
> No, it isn't.
> The function `void watch(string msg, int i)` from unit tests is declarated into the unittest block. So, it's delegate because it contains pointer to the unittest scope.
> Try to change function to the `static void watch(string msg, int i)` and you'll see the same error.

On Friday, 15 November 2013 at 11:25:30 UTC, ilya-stromberg wrote:
> On Thursday, 7 November 2013 at 13:45:57 UTC, Dicebot wrote:
>> Our current victim is std.signal by Robert Klotzner which is supposed to deprecate and supersede existing (and broken) std.signals
>
> Robert, it will be great to see some more documentation for `std.signal`.
> For example, you can:
>  - add more usage examples for each public function

I think this is overkill for this simple API. I would basically have to repeat the example given at the top for every single function, not really useful.

>  - add tread-safe example for multi-tread application

std.signal is in no way special regarding multi threading, it is just the same as with every other module in phobos.

>  - add better signal description and add more external links (for examle, to the wikipedia - http://en.wikipedia.org/wiki/Signals_and_slots). Look at the old std.signals - http://dlang.org/phobos/std_signals.html

Done :-)

On Saturday, 4 January 2014 at 18:01:17 UTC, Robert wrote:
> On Friday, 15 November 2013 at 11:25:30 UTC, ilya-stromberg wrote:
>> On Thursday, 7 November 2013 at 13:45:57 UTC, Dicebot wrote:
>>> Our current victim is std.signal by Robert Klotzner which is supposed to deprecate and supersede existing (and broken) std.signals
>>
>> Robert, it will be great to see some more documentation for `std.signal`.
>> For example, you can:
>> - add more usage examples for each public function
>
> I think this is overkill for this simple API. I would basically have to repeat the example given at the top for every single function, not really useful.

OK.

>> - add tread-safe example for multi-tread application
>
> std.signal is in no way special regarding multi threading, it is just the same as with every other module in phobos.

Not exactly. Almost all Phobos functions can be used for thread-local variables. Theoretically, `std.signal` can be used for multi-thread applications. So, it will be great to know what should I do for it.
For example, must I use mutex to protect `Signal` method calls? So, please add example or at least add documentation how to do it.

>> - add better signal description and add more external links (for examle, to the wikipedia - http://en.wikipedia.org/wiki/Signals_and_slots). Look at the old std.signals - http://dlang.org/phobos/std_signals.html
>
> Done :-)

Thanks.

On Thursday, 7 November 2013 at 13:45:57 UTC, Dicebot wrote:
> Time to move forward with some more reviews :)

Hi,

Apologies, I missed this review thread. So, some questions / observations:

IMO, the effort to disallow third parties to "emit" signals on your object by far exceeds its worth.

For this purpose, you have needed:
- string mixins
  - ...which declare, by convention, a field with a name not explicitly given by the user
  - ...with a workaround for when they don't work
- an enumeration type
- two different signal struct types
- a boatload of documentation
- a nontrivial cognitive effort on the user to grok all of the above

This little feature steals too much focus, and IMO is not worth it.

As a compromise, have you considered template mixins?

mixin template M()
{
	public void pub() {}
	private void pri() {}
}

struct S
{
	mixin M m;
}

Trying to access S.m.pri() from another module fails as expected.

The downside is that the ".m" is not required, so the mixin members will "spill over" the parent object's field namespace.

> This implementation supersedes the former std.signals

I don't think comparisons of older versions of a module belong in the module's documentation.

> a.valueChanged.connect!"watch"(o);        // o.watch is the slot

This is really ugly, but I don't see any way to improve it either. Maybe if the type of &classInstance.method was actually a special "pointer-to-class-method" type that implicitly down-casted to a delegate, it would be possible to safely use &o.watch instead...

> dg must not be null. (Its context may.)

I don't think the context will ever be null (unless you take the address of a null class/struct pointer's method). (toDelegate uses the context pointer to store the function pointer, and the function pointer to store a template instance according to the function's arguments.)

> It implements the emit function for all other functionality it has this aliased to RestrictedSignal.

Missing some punctuation, I think.

>  The idea is to instantiate a Signal privately and provide a public accessor method for accessing the contained RestrictedSignal.

I had a hard time understanding this line the first time. Please reword without using "The idea is", because it's not clear what "the idea" is for - usage? Rationalization of implementation method?

> Use this overload if you either really, really want strong ref semantics for some reason

Was the emphasis really necessary? It makes complete sense to build an asynchronous-event-model application using 100% only "strong connections" (delegates or delegate arrays). I don't remember ever NEEDING weak-referenced callbacks.

On Monday, 6 January 2014 at 10:13:44 UTC, Vladimir Panteleev wrote:
> Apologies, I missed this review thread.

=/ Any good idea to make those more visible? Maybe we should setup a mailing list for those who want to be notified about ongoing reviews?