Allow preDispatchSignal to cancel dispatching

[andrewkaufman]

We should change preDispatchSignal() to allow slots to return a value to cancel the dispatch.

[johnhaddon]

The OpDialogue.preExecuteSignal() is probably a good model for this - in that a boolean return value of true from a slot cancels the execution, false allows it to continue.

[andrewkaufman]

Since Dispatcher::preDispatchSignal() is in c++, I went digging for an equivalent example there, and found StandardSet::memberAcceptanceSignal(). It looks like that one takes the opposite logic though (slots returning false cancel the action). Should Dispatcher match StandardSet or OpDialogue? Or should all 3 match?

[andrewkaufman]

Also, do you mind if I have postDispatchSignal use the same signal type, even though the return value isn't checked there? Or would you prefer I have different typedefs for each signal?

[andrewkaufman]

Ignore that commit... I pushed it prematurely, and even though I never made a pull request and I deleted the remote branch, it still shows up here...

[johnhaddon]

I'm sure we shouldn't change anything about memberAcceptanceSignal() - it's pretty clear that there true means "yes I accept" and false means "no thanks".

And the return values for preDispatchSignal() and preExecuteSignal() should definitely work the same way as each other since those signals have identical semantics. But it's less clear to me there whether it's true or false that should mean "stop!". I think I chose false to mean "carry on" because then Python slots who have no intention of ever stopping execution (they're just in it to be notified and nothing else) don't even need to return anything (they'll get a default None return value which will evaluate to False). But probably also because that's what WidgetSignal already did and I didn't feel like writing another combiner.

Unless you feel strongly I'd suggest just matching preDispatchSignal() to preExecuteSignal() as-is. I'm not concerned with the differences to memberAcceptanceSignal() because the semantics there are different anyway. If you think it really needs clarifying then I'd suggest returning an enum with descriptive values, but I honestly think just documenting the behaviour is good enough.

[andrewkaufman]

And what about the postDispatchSignal question? I've got it working both ways, so just waiting on your preference to make a pull request. In the version that they provide different signal typedefs, I've called one DispatchSignal and the other CancellableDispatchSignal

[johnhaddon]

You spoil me! I'd go with the simpler one, and just ignore the bool return value from postDispatchSignal().