Skip to content

Conversation

SimonAropama
Copy link

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.

@emedvedev
Copy link
Contributor

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.

@SimonAropama
Copy link
Author

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?

@emedvedev
Copy link
Contributor

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.

@mbrannigan
Copy link

bump? 👍

@CLAassistant
Copy link

CLAassistant commented May 11, 2022

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.

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

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.

5 participants