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

Commands #2

Closed
wants to merge 4 commits into from
Closed

Commands #2

wants to merge 4 commits into from

Conversation

bkeepers
Copy link
Contributor

"Commands" are an explicit interface for defining discrete pieces of functionality in a Hubot script. Initially, a command is a subset of the existing robot.respond interface. Over time, commands will start to gain more power and will be exposed through a command line interface or as native "slash commands" on services that support it, like Slack.

robot
  .command('cowsay <words ...>')
  .description('make the cow say words')
  .action(function(words) {
    return require('cowsay').say(words.join(' '));
  });

👀 read the proposal 👀

@gasolin
Copy link

gasolin commented May 25, 2017

In discuss of command (or regex we are using at hubot 2.x) and the new way to write plugin,
IMHO hubot 3 could be evolved to suit for some more general usage without ditch the existing plugins.

Current flow of input message to get processed by plugin is

raw Message -> robot.respond("regex", actionCallback)

A more general flow could introduce the processor layer, and make the internal flow looks like

raw Message ->
processed by regex/NLP -> distill proper `command(intent)` and `data` ->
dispatch to the right command(intent) -> do the ActionCallback

If we treat plugin-name as the command name, current plugin API

robot.respond("regex", actionCallback)

could be re-imeplemted as:

when we got a legacy plugin,
1. register {regex: command} pair to hubot default regex `processor`
2. if pattern matched in regex processor, return `intent/command` as this plugin name, and treat the rest matched item as data.
3. dispatch (redux pattern) the intent to correspondent actionCallback

For future expansion,
In Step 2 we can replace current regex processor to other NLP processor, pre-process to correct the spelling issue, or wire with some online processor, like wit.ai, api.ai, to unlock many possibilities.

In Step 3 we are able to apply redux pattern to dispatch actions & update brain in reducers

Copy link
Member

@technicalpickles technicalpickles left a comment

Choose a reason for hiding this comment

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

This is a good start. I have some questions to help flesh it out:

  • how would you use listener metadata with commands? would you be able to use different metadata on different subcommands?
  • how do commands behave if you don't get the usage just right? consider just calling deploy without anything else, a non-existent subcommand, or otherwise not matching the regex for the command?
  • is there any consideration for testing, in terms of users of this new feature?


## Proposed solution

A new API based on [Commander.js](https://github.com/tj/commander.js), a library with nice APIs for defining command-line interfaces, would be added on `robot.command`.
Copy link
Member

Choose a reason for hiding this comment

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

Nice! One of the first things I thought before digging into the details was: "I wonder if there's prior art around command parsing we can take inspiration from / use implementation of"

@technicalpickles
Copy link
Member

Transcribing a comment from @bkeepers in #contributors:

one of my (undocumented) motivations for the command interface is to abstract how a command is implemented. The initial spike I'm doing it'll still be in JS, but it could easily shell out or be an RPC call to another service


Here are some of the problems with `robot.hear` and `robot.respond` for defining scripts:

- listeners are registered with **regular expressions**, which can be cumbersome and error-prone, and encourage authors to design scripts with unpredictable interfaces.
Copy link

Choose a reason for hiding this comment

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

Huge 👍 to this as a problem. I find myself wishing all the time that I could define commands as something more like optparse.

- https://github.com/github/hubot/issues/249 - a proposal very similar to this one from 5 years ago that even suggests using commander.js. As "slash commands" have become a common feature in chat clients, this feels even more like the right direction.
- https://github.com/github/hubot/issues/873 - an alternative that proposes using classes to define an entire script. While this is interesting, it feels like a very dramatic departure from the current style.

TODO: more on why this feels like the right approach as we look toward supporting non-chat interfaces
Copy link

Choose a reason for hiding this comment

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

What does it mean for hubot to support non-chat interfaces? Does hubot consume them, but still stay connected to chat? Or does hubot become a way to write shell commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a future evolution proposal, we'll look at commands being invoked from the command line or through an API.

robot
.command('cowsay <words ...>')
.description('make the cow say words')
.action(function(words) {

Choose a reason for hiding this comment

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

It looks like the action callback receives only the capture group component of the command? Is that just brevity for this example? Wouldn't you also need at least the response object, to get the whole match, the user object etc, in case you need to interrogate that context to perform controller logic.

Copy link
Member

@technicalpickles technicalpickles left a comment

Choose a reason for hiding this comment

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

How are commands internally represented? Would it be a public API for getting access to them?

Would help for commands show up in the same way that help shows up for regex-based listeners?

Also worth noting, there is robot.commands which is an array that contains all the help usage that has been parsed out of scripts. That will be a little confusing to have that not be the same thing that robot.command registers.

@technicalpickles
Copy link
Member

How are commands internally represented? Would it be a public API for getting access to them?

Related, would there be a way to invoke commands outside of an incoming request? I am thinking of use cases that have come up over the years where people want to use the same code to do something based on an incoming HTTP response as an incoming chat message, ie posting to a room.

Copy link

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I’d like to discuss if the fluent API is really the right approach for this feature before we move with this

.description('make the cow say words')
.action(function(words) {
return require('cowsay').say(words.join(' '));
});
Copy link

@gr2m gr2m Jun 12, 2017

Choose a reason for hiding this comment

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

I would recommend against a fluent API such as this. It is harder to test and for what you try to achieve it’s not well suited. I think a much clearer API would be this

robot.command({
  name: 'cowsay',
  description: 'make the cow say words',
  action: function (text) { // text being the text passed to the command
    return require('cowsay').say(text)
  }
})

It is just a JavaScript method without any magic, it will be simple to test and understand by users.

Another benefit of that approach is that commands will be easier to share. A command definition is a simple JavaScript object which can be put in a single file, there is no need to have access to the robot instance at all.

// could live in a place like hubot/my-command.js
module.exports = {
  name: 'cowsay',
  description: 'make the cow say words',
  action: function (text) { // text being the text passed to the command
    return require('cowsay').say(text)
  }
}

@bkeepers
Copy link
Contributor Author

bkeepers commented Dec 3, 2018

Closing as stale.

@bkeepers bkeepers closed this Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants