Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Listener options and filtering hooks #724

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
"optparse": "1.0.4",
"scoped-http-client": "0.9.8",
"log": "1.4.0",
"express": "3.3.4"
"express": "3.3.4",
"async": "~0.9.0"
},

"engines": {
Expand Down
56 changes: 44 additions & 12 deletions src/listener.coffee
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{inspect} = require 'util'
async = require 'async'

{TextMessage} = require './message'

Expand All @@ -9,25 +10,51 @@ class Listener
# robot - A Robot instance.
# matcher - A Function that determines if this listener should trigger the
# callback.
# callback - A Function that is triggered if the incoming message matches.
constructor: (@robot, @matcher, @callback) ->
# options - An Object of additional parameters keyed on extension name
# (optional).
# handler - A Function that is triggered if the incoming message matches.
constructor: (@robot, @matcher, @options, @handler) ->
if not @handler?
@handler = @options
@options = {}

# Public: Determines if the listener likes the content of the message. If
# so, a Response built from the given Message is passed to the Listener
# callback.
#
# message - A Message instance.
#
# Returns a boolean of whether the matcher matched.
call: (message) ->
# @callback - Call with a boolean of whether the matcher matched.
call: (message, cb) ->
if match = @matcher message
@robot.logger.debug \
"Message '#{message}' matched regex /#{inspect @regex}/" if @regex

@callback new @robot.Response(@robot, message, match)
true
if @regex
@robot.logger.debug \
"Message '#{message}' matched regex /#{inspect @regex}/"
response = new @robot.Response(@robot, message, match)
@testAllHooks response, (result) =>
if result
@robot.logger.debug "Message '#{message}' passed all hooks"
@handler response
cb true
else
@robot.logger.debug "Message '#{message}' failed some/all hooks"
cb false
else
false
cb false

testAllHooks: (msg, callback) ->
wrappedHooks = []
# Wrap all hooks for execution
Listener.hooks.forEach (hook) =>
wrappedHooks.push (cb) =>
hook(@robot, this, @options, msg, cb)

# Execute all hooks
async.parallel wrappedHooks, (err, results) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why parallel? Listeners are generally invoked in series, so it might not be obvious that hooks work different.

Also, shouldn't we be doing something with this err?

allHooksPassed = true
results.forEach (result) ->
allHooksPassed = allHooksPassed && result
callback(allHooksPassed)

class TextListener extends Listener
# TextListeners receive every message from the chat source and decide if they
Expand All @@ -36,11 +63,16 @@ class TextListener extends Listener
# robot - A Robot instance.
# regex - A Regex that determines if this listener should trigger the
# callback.
# callback - A Function that is triggered if the incoming message matches.
constructor: (@robot, @regex, @callback) ->
# options - An Object of additional parameters keyed on extension name
# (optional).
# handler - A Function that is triggered if the incoming message matches.
constructor: (@robot, @regex, @options, @handler) ->
@matcher = (message) =>
if message instanceof TextMessage
message.match @regex
super @robot, @matcher, @options, @handler

Listener.hooks = []

module.exports = {
Listener
Expand Down
39 changes: 25 additions & 14 deletions src/robot.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Log = require 'log'
Path = require 'path'
HttpClient = require 'scoped-http-client'
{EventEmitter} = require 'events'
async = require 'async'

User = require './user'
Brain = require './brain'
Expand Down Expand Up @@ -74,8 +75,8 @@ class Robot
# callback - A Function that is called with a Response object.
#
# Returns nothing.
hear: (regex, callback) ->
@listeners.push new TextListener(@, regex, callback)
hear: (regex, options, callback) ->
@listeners.push new TextListener(@, regex, options, callback)

# Public: Adds a Listener that attempts to match incoming messages directed
# at the robot based on a Regex. All regexes treat patterns like they begin
Expand All @@ -85,7 +86,7 @@ class Robot
# callback - A Function that is called with a Response object.
#
# Returns nothing.
respond: (regex, callback) ->
respond: (regex, options, callback) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break all existing scripts. Maybe check the type of these arguments, and assign options to callback if it's a function?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like that is actually being handled in the TextListener constructor.

re = regex.toString().split('/')
re.shift()
modifiers = re.pop()
Expand All @@ -110,7 +111,7 @@ class Robot
modifiers
)

@listeners.push new TextListener(@, newRegex, callback)
@listeners.push new TextListener(@, newRegex, options, callback)

# Public: Adds a Listener that triggers when anyone enters the room.
#
Expand Down Expand Up @@ -190,17 +191,27 @@ class Robot
#
# Returns nothing.
receive: (message) ->
results = []
for listener in @listeners
try
results.push listener.call(message)
break if message.done
catch error
@emit('error', error, new @Response(@, message, []))
# Try executing all registered Listeners in order of registration
# and return after message is done being processed
anyListenersExecuted = false
async.detectSeries(
@listeners,
(listener, cb) =>
try
listener.call message, (listenerExecuted) ->
anyListenersExecuted = anyListenersExecuted || listenerExecuted
cb(message.done)
catch err
# TODO provide useful info after catching an error (like a stacktrace!)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment stale? If you get an err, you it should have the stacktrace at this point.

@emit('error', err, new @Response(@, message, []))
,
(result) =>
# If no registered Listener matched the message
if message not instanceof CatchAllMessage and not anyListenersExecuted
@logger.debug 'No listeners executed; falling back to catch-all'
@receive new CatchAllMessage(message)
)

false
if message not instanceof CatchAllMessage and results.indexOf(true) is -1
@receive new CatchAllMessage(message)

# Public: Loads a file in path.
#
Expand Down