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

Provide Hubot the ability to react to message types #349

Closed
wants to merge 6 commits into from
Closed

Provide Hubot the ability to react to message types #349

wants to merge 6 commits into from

Conversation

wingrunr21
Copy link
Contributor

I needed the ability for hubot to react to topic changes in Campfire as well as the ability to query the Campfire room for information (like the current topic).

I made changes that allow hubot to "see" a message. The function will allow a String, Regex, or actual object name to be specified as what to watch for:

module.exports = (robot) ->
  robot.see "TopicChangeMessage", (msg) ->
    # do something fun
  robot.see /TopicChangeMessage/, (msg) ->
    # works for funness as well
  robot.see TopicChangeMessage, (msg) ->
    # just remember to require TopicChangeMessage...

The enter and leave events have been updated to use this common way of reacting to message types.

I also added an info utility function to the Campfire adaptor. It is possible I missed something more obvious for using it, but here is what I am currently using:

robot.respond /topic/, (msg) ->
  user = msg.message.user
  room = user.room

  robot.adapter.bot.Room(room).info (err, data) ->
    topic = data.room.topic

I'm working on an example script for this stuff to commit to hubot-scripts

@technicalpickles
Copy link
Member

I like the idea of being able to do things with other kinds of messages. I'm not sure about the public api, ie calling the method see though. I almost think using respond for this would make sense as well, but that might be overloading what that is used for.

In addition to that, I'd like to see some examples of this in the README.rdoc and/or the method documentation for see.

@wingrunr21
Copy link
Contributor Author

I decided to go with see as opposed to respond since respond seems to be used for messages directed at the robot itself. I needed the ability to react to specific message types (regardless of how they are directed) within the Campfire room, so I felt adding a new method was the proper approach.

Happy to provide additional docs (I will be pushing an example plugin to hubot-scripts once this is merged). Would you prefer a new section of the README be created or would perhaps putting this kind of documentation in a wiki page be better?

@technicalpickles
Copy link
Member

Let's go with an updated section to the README. That way, the docs get updated with the merge.

@wingrunr21
Copy link
Contributor Author

Ok, added some stuff to the README under a new section called Robot Functionality. I went ahead and documented the other robot functions as well. The Campfire adapter additions I feel would be better served being put on the wiki page. I'll add the example for how to change the topic as well as a link to the GH page regarding the room info API once this is merged.

Shadowfiend and others added 2 commits November 20, 2012 09:46
A couple of functions calls had stray commas in there.
@technicalpickles
Copy link
Member

Been thinking about this a bit more. I'm still not feeling the 'see' API as far as language goes, and plus it feels like it's expose too many internals for scripts to use/abuse.

If you are interested in topic stuff specifically, I rather see a topic listener, rather an this see API. There is prior examples of this actually, with enter and leave listeners, in addition to the more commonly used hear and respond.

It should be pretty simple to implement this in robot.coffee. A theoretical implementation:

  # Public: Adds a Listener that triggers when anyone changes the topic
  #
  # callback - A Function that is called with a Response object.
  #
  # Returns nothing.
  topic: (callback) ->
    @listeners.push new Listener(@, ((msg) -> msg instanceof TopicMessage), callback)

This would depend on some of the work already done. Thoughts?

@wingrunr21
Copy link
Contributor Author

I'm ok with removing see and making a topic listener. Should we keep MessageListener as the backer? There's no difference between topic, enter, and leave except the message type. I would much prefer this:

@listeners.push new MessageListener(@, TopicChangeMessage, callback)
...
@listeners.push new MessageListener(@, EnterMessage, callback)
...
@listeners.push new MessageListener(@, LeaveMessage, callback)

over this:

@listeners.push new Listener(@, ((msg) -> msg instanceof TopicMessage), callback)
...
@listeners.push new Listener(@, ((msg) -> msg instanceof EnterMessage), callback)
...
@listeners.push new Listener(@, ((msg) -> msg instanceof LeaveMessage), callback)

I do have one worry: is topic applicable to adapters other than Campfire? I chose see to function the way it does because I figured it could be used by others who are not using Campfire. I'd like to make sure topic has that same utility.

@technicalpickles
Copy link
Member

@wingrunr21 there's been a number of changes in master preparing for 3.0. Mind taking a stab at merging master so this will merge cleanly?

@wingrunr21
Copy link
Contributor Author

Sure, do you have feedback on my last comment?

@technicalpickles
Copy link
Member

IRC and Jabber both have topics for their rooms, so it's applicable there. It'd be up to the adapter maintainer to implement though.

@neersighted
Copy link

👍

@wingrunr21
Copy link
Contributor Author

Sorry this has been left in the wind by me. I have carved out time on Friday to get this updated against the latest master and implement the changes @technicalpickles asked for.

@tombell tombell closed this in 10645fd Mar 11, 2013
@wingrunr21
Copy link
Contributor Author

Sorry I dropped the ball on that again. Thanks @tombell!

@tombell
Copy link
Contributor

tombell commented Mar 11, 2013

I tried merging the PR, but there were too many conflicts so I recreated the branch, based on the changes you had done, with some minor fixes which make it a bit more consistent.

For example:

module.exports = (robot) ->
  robot.topic (msg) ->
    robot.send "Oh dear #{msg.user} changed the topic to #{msg.message}"

(note I forgot what the actual properties on msg are, but you get the idea!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants