hubot-stackstorm icon indicating copy to clipboard operation
hubot-stackstorm copied to clipboard

Catch all response sc.15.4.16

Open SimonAropama opened this issue 8 years ago • 5 comments

At present there is no default message when a command is not recognised so it fails silently. It would be more user friendly to send a simple message to inform the user that the command was not recognised so that they can check and try again.

A CatchAll can be added via an additional Coffee script but I found that once hubot-stackstorm was installed it only worked with general messages and not those directed to Hubot.

SimonAropama avatar Apr 15 '16 10:04 SimonAropama

Hi! Thank you for the submission.

I can potentially see a problem with it: since hubot-stackstorm uses a catch-all, the "not found" message will probably fire on any valid command that's not part of stackstorm but a separate Hubot plugin. Since we have a number of users who integrate hubot-stackstorm into their own Hubot setup, or use additional modules with st2chatops, I can't merge the PR.

A better way to design it would be this: instead of making hubot-stackstorm fire on catchall, you can get a list of regexes that's built when StackStorm commands are loaded, and only register them as commands. That would involve some work, but this way an external catch-all script will work for sure.

emedvedev avatar Apr 15 '16 11:04 emedvedev

Ok, presumably during our testing it didn't fire on other commands (e.g. !help etc) as the hubot-stackstorm plugin is loaded last.

Is the suggestion regarding regexes and only registering them something to be done in this script?

SimonAropama avatar Apr 15 '16 13:04 SimonAropama

Yes, you could try to replace this catch-all (that makes hubot-stackstorm fire on every command addressed to Hubot): https://github.com/StackStorm/hubot-stackstorm/blob/master/scripts/stackstorm.js#L297 With a list of regexes. The commands are fetched here: https://github.com/StackStorm/hubot-stackstorm/blob/master/scripts/stackstorm.js#L183 And here's where you get a regex for each format string: https://github.com/StackStorm/hubot-stackstorm/blob/master/lib/command_factory.js#L80 You'll also have to deal with edits/removals of aliases on reload, but it's not too tricky. Of course, it might be a bit of an overkill for your case, but it would be a very welcome contribution.

emedvedev avatar Apr 15 '16 22:04 emedvedev

bump? 👍

mbrannigan avatar Aug 21 '17 19:08 mbrannigan

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

:x: upnica
:x: SimonAropama
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar May 11 '22 10:05 CLAassistant