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

Use slash commands instead of message listeners for stored replies #241

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vcarl
Copy link
Member

@vcarl vcarl commented Jul 28, 2022

Alternative approach to #148, will close #131

This puts every !command under a single top-level /r command, with an autocompleted list of replies as the first argument. It includes the idea introduced by #240 (thanks @ScriptyChris!) of telling the bot who to ping — #240 had the problem of double-pinging, but as a slash command argument it can be done without that drawback.

This code removes some commands for simplicity, it's a functional proof of concept more than final code. Remove commands:

  • !mdn
  • !lock
  • !unlock

Another change: !commands has been changed to /r help, which uses an ephemeral reply to avoid shoving everything off scroll back.

This is very much a rough draft, functional but incomplete. Before merging we should add logic to encourage folks to shift from !commands syntax to slash commands and figure out how to handle !mdn

Demo:

Screen Shot 2022-07-27 at 10 18 14 AM

Screen Shot 2022-07-27 at 10 18 31 AM

@ghost
Copy link

ghost commented Jul 28, 2022

👇 Click on the image for a new way to code review
  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

},
},
// {

Choose a reason for hiding this comment

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

Shouldn't this long commented section be removed (i guess it's a remnant from code testing)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this is more of a draft state atm, roughed out. I'll update description to add necessary followup before this can merge

@vcarl vcarl marked this pull request as draft July 28, 2022 18:05
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.

Convert !commands to slash commands
2 participants