Order of command requirement matters

[cabello]

Hi,

Thanks for this project, it's been 9 months since I've been using it for my bot.

I am now turning the bot into a Slack app which requires a web server and I chose Rails, Rails has auto loading and eager loading mechanisms and usually load files in alphabetical order.

Looking at the implementation of match, scan and routes, it seems (and this is how the app behave in production) that the order of command registration matters.

Meaning, requiring a file that matches all first, e.g. match(/^(?<answer>.*)$/m) would prevent other files from matching at all. (This is our "unknown" command, it has "did you mean" and implements our conversation logic)

We also want to guarantee help to be available at all times, so that one would need to be the first route that is checked when routes.each_pair.

It would be nice if there was a way to specify where a command should be put in the ordered list of routes. At this moment the I named the command zzz_unknown to get the correct behaviour.

My suggestion is to draw inspiration from Ruby Array insert method, so our commands for example:

# catch all
match(/^(?<answer>.*)$/m, insert: -1) do |client, data, match|
  client.say "I was the last match"
end

command('help', insert: 0) do |client, data, match|
  client.say "No one can ever match help before me"
end

The obvious problem with this solution is that if another command used insert: 0 and it was required after help, then that command would be first.

The other potential idea is to separate the matches/commands into a separate file like Rails routes, I haven't thought much about this approach but it could be a nice looking one.

Thanks for taking the time to read and consider this issue / feature request.
💜

[cabello]

I had some time to think about how to get inspiration from Rails routes and bring them over to Slack Ruby Bot, this is what I come up with:

ExampleBot.commands.draw do  
  # Do blocks are still supported but are suggested only when quick hacking.
  command 'ping' do |client, data, match|
    client.say(text: 'pong', channel: data.channel)
  end

  # This is the preferred method, it would call a `call` static method with
  # `client`, `data` and `match`.
  command 'ping', to: ExampleBot::Commands::Ping
  # Strings would also be allowed.
  command 'ping', to: 'ExampleBot::Commands::Ping'
  # Some convention could be established so `ExampleBot::Commands::` would not
  # be needed.

  # It would still allow for multiple matches
  command 'call', '呼び出し' do |client, data, match|
    client.say(channel: data.channel, text: 'called')
  end

  match /^How is the weather in (?<location>\w*)\?$/, to: 'Weather'
  scan(/([A-Z]{2,5})/), to: 'Market'
  attachment 'Slack API Documentation', to: 'Attachment'
  operator '=', to: 'Calculate'
end

Benefits of this approach:

• the order of "route" definition matters, make that explicit
• it would be friendly to auto loaders (command could be loaded only when needed, help would still list all commands)
• decouple command logic from pattern matching

What are your thoughts?

[dblock]

This is a dup of #75, but let's keep it open since it has a lot of detail.

[dblock]

I am down with an explicit declaration. This could be similar to Rails or to Grape where the order declared is the order processed. I believe that would require deprecating auto-loading of command classes, which is totally OK with me assuming someone does all the work :)

I don't think insert: ... is necessary. What we need is a way where no automatic mounting happens. I like Grape's approach with ordered declarations and an explicit mount command.