litaio / lita

ChatOps for Ruby.
https://www.lita.io
MIT License
1.68k stars 179 forks source link

Add support for passing a Proc to routes #174

Closed glittershark closed 3 years ago

glittershark commented 8 years ago

Add support for passing a Proc to routes instead of a regular expression for matching - this proc will then be called with the message in the context of the handler object and should return a boolean indicating whether the route matches or not.

This allows for dynamically defined routing, plus support for configuring things like command names and prefixes, which is useful as plugin configuration is only available on a handler instance, not the singleton.

Probably addresses most of what #100 is asking for

jsjohnst commented 8 years ago

as an example of the use case:

https://github.com/jsjohnst/lita-pagerduty/blob/master/lib/lita/handlers/pagerduty_utility.rb#L20

jimmycuadra commented 8 years ago

Thank you for the PR! This is a great idea I had never considered. I have only two hesitations compared to what I originally had in mind with regards to #100:

  1. It still requires the plugin author to define the route in a special way in order to allow the user to override it. Ideally users would be able to arbitrarily override route conditions no matter how the route is defined.
  2. Potential overhead of evaluating many procs at runtime when a message is received. If the proc is only being used as a workaround for regular expressions that can't be constructed until some user-supplied configuration value is known (as in @jsjohnst's example), then it's redoing the same work every time a new message is received. It'd be much more efficient to have a phase during the robot's start up where the procs get called once and converted into the regular expressions that will actually be used to do the matching. If there are other use cases for this where a single-pass evaluation of the proc isn't sufficient, that might not be possible, but I'd like to hear about those cases.