Named signal handlers

[chuckremes2]

Support for named signal handlers, e.g. on_signal('SOME_NAME') { }. Always call the "catch-all" signal handler even when a named handler exists.

[antstorm]

See inline comments

[antstorm]

I feel like a cleaner solution would be to add the handler_name to event_name and dispatch the event twice:

• "signaled:signalName"
• "signaled"

I think this would keep the dispatcher simpler for now and does allow us to achieve everything that is needed

[antstorm]

I wonder if we should let the called to this instead of handling it in the Dispatcher ^. Basically I'm thinking that we can leave the Dispatcher's API unchanged and just allow the caller to construct the event name the way they want.

Another approach might be to actually use the target to represent a signal itself, then what you can do is:

signal = Signal.new(signal_name)
dispatcher.register_handler(signal, 'signaled') { |input| ... }

and for a catch-all:

dispatcher.register_handler(Dispatcher::TARGET_WILDCARD, 'signaled') { |signal, input| ... }

I'm a bit hesitant to extend the Dispatcher's interface because I feel like we can achieve what we need with the current version and can avoid overcomplicating it

[chuckremes2]

As you see, I tried to move the naming logic into Dispatcher. I agree that it changes the Dispatcher interface but now the complexity will move elsewhere.

I like the second suggestion about making a Signal object. I'll try that first since it preserves some encapsulation without leaking it to multiple classes.

[chuckremes2]

Ended up going with the first suggestion. I couldn't figure out how to do it the second way with a Signal class. In StateManager#apply_event it never popped a Signal target so no events ever matched it. I'm sure I missed something simple but I ran out of time to dig into it. More documentation here about how elements get onto the History::EventTarget queues might be useful (unless this concept is going away in the rewrite).

[antstorm]

Something is missing here. This will basically call the handler on every single event dispatched on the History::EventTarget.workflow

[chuckremes2]

I was worried about that but unsure. You confirmed it was wrong. Just pushed a change more in line with your earlier guidance using Dispatcher::TARGET_WILDCARD, 'signaled' instead. Hopefully that won't get executed for all workflows whenever a signal arrives.

[antstorm]

🎉 🎉 🎉