From 273fcf1418825376b1def78b13a1f9bfb765559a Mon Sep 17 00:00:00 2001 From: diamondburned Date: Thu, 12 Nov 2020 19:02:52 -0800 Subject: [PATCH] Bot: Added subcommand aliases and better setup API This commit adds subcommand aliases as well as additional code in HelpGenerate to cover for both subcommand and command aliases. A breaking change is that {,Must}RegisterSubcommandCustom methods are now replaced with normal {,Must}RegisterSubcommand methods. This is because they now use variadic strings, which could take 0, 1 or more arguments. This commit also allows AddMiddleware and similar methods to be given a method directly: sub.Plumb(cmds.PlumbedHandler) sub.AddMiddleware(cmds.PlumbedHandler, cmds.plumbMiddleware) This change closes issue #146. --- _example/advanced_bot/bot.go | 2 +- _example/advanced_bot/debug.go | 10 +- bot/ctx.go | 90 ++++++++++++------ bot/ctx_call.go | 36 +++---- bot/ctx_test.go | 58 +++++++----- bot/subcommand.go | 167 +++++++++++++++++++++++++-------- 6 files changed, 252 insertions(+), 111 deletions(-) diff --git a/_example/advanced_bot/bot.go b/_example/advanced_bot/bot.go index f68729d..2ab53db 100644 --- a/_example/advanced_bot/bot.go +++ b/_example/advanced_bot/bot.go @@ -22,7 +22,7 @@ type Bot struct { func (bot *Bot) Setup(sub *bot.Subcommand) { // Only allow people in guilds to run guildInfo. - sub.AddMiddleware("GuildInfo", middlewares.GuildOnly(bot.Ctx)) + sub.AddMiddleware(bot.GuildInfo, middlewares.GuildOnly(bot.Ctx)) } // Help prints the default help message. diff --git a/_example/advanced_bot/debug.go b/_example/advanced_bot/debug.go index b0d0d13..27120f4 100644 --- a/_example/advanced_bot/debug.go +++ b/_example/advanced_bot/debug.go @@ -26,12 +26,14 @@ func (d *Debug) Setup(sub *bot.Subcommand) { // Manually set the usage for each function. - sub.ChangeCommandInfo("GOOS", "GOOS", "Prints the current operating system") - sub.ChangeCommandInfo("GC", "GC", "Triggers the garbage collector") + // Those methods can take in a regular Go method reference. + sub.ChangeCommandInfo(d.GOOS, "GOOS", "Prints the current operating system") + sub.ChangeCommandInfo(d.GC, "GC", "Triggers the garbage collector") + // They could also take in the raw name. sub.ChangeCommandInfo("Goroutines", "", "Prints the current number of Goroutines") - sub.Hide("Die") - sub.AddMiddleware("Die", middlewares.AdminOnly(d.Context)) + sub.Hide(d.Die) + sub.AddMiddleware(d.Die, middlewares.AdminOnly(d.Context)) } // ~go goroutines diff --git a/bot/ctx.go b/bot/ctx.go index e60d1eb..1e5d2dc 100644 --- a/bot/ctx.go +++ b/bot/ctx.go @@ -1,6 +1,7 @@ package bot import ( + "fmt" "log" "os" "os/signal" @@ -267,14 +268,16 @@ func (ctx *Context) FindCommand(structName, methodName string) *MethodContext { // MustRegisterSubcommand tries to register a subcommand, and will panic if it // fails. This is recommended, as subcommands won't change after initializing // once in runtime, thus fairly harmless after development. -func (ctx *Context) MustRegisterSubcommand(cmd interface{}) *Subcommand { - return ctx.MustRegisterSubcommandCustom(cmd, "") -} - -// MustRegisterSubcommandCustom works similarly to MustRegisterSubcommand, but -// takes an extra argument for a command name override. -func (ctx *Context) MustRegisterSubcommandCustom(cmd interface{}, name string) *Subcommand { - s, err := ctx.RegisterSubcommandCustom(cmd, name) +// +// If no names are given or if the first name is empty, then the subcommand name +// will be derived from the struct name. If one name is given, then that name +// will override the struct name. Any other name values will be aliases. +// +// It is recommended to use this method to add subcommand aliases over manually +// altering the Aliases slice of each Subcommand, as it does collision checks +// against other subcommands as well. +func (ctx *Context) MustRegisterSubcommand(cmd interface{}, names ...string) *Subcommand { + s, err := ctx.RegisterSubcommand(cmd, names...) if err != nil { panic(err) } @@ -282,14 +285,9 @@ func (ctx *Context) MustRegisterSubcommandCustom(cmd interface{}, name string) * } // RegisterSubcommand registers and adds cmd to the list of subcommands. It will -// also return the resulting Subcommand. -func (ctx *Context) RegisterSubcommand(cmd interface{}) (*Subcommand, error) { - return ctx.RegisterSubcommandCustom(cmd, "") -} - -// RegisterSubcommand registers and adds cmd to the list of subcommands with a -// custom command name (optional). -func (ctx *Context) RegisterSubcommandCustom(cmd interface{}, name string) (*Subcommand, error) { +// also return the resulting Subcommand. Refer to MustRegisterSubcommand for the +// names argument. +func (ctx *Context) RegisterSubcommand(cmd interface{}, names ...string) (*Subcommand, error) { s, err := NewSubcommand(cmd) if err != nil { return nil, errors.Wrap(err, "failed to add subcommand") @@ -298,18 +296,36 @@ func (ctx *Context) RegisterSubcommandCustom(cmd interface{}, name string) (*Sub // Register the subcommand's name. s.NeedsName() - if name != "" { - s.Command = name + if len(names) > 0 && names[0] != "" { + s.Command = names[0] + } + + if len(names) > 1 { + // Copy the slice for expected behaviors. + s.Aliases = append([]string(nil), names[1:]...) } if err := s.InitCommands(ctx); err != nil { return nil, errors.Wrap(err, "failed to initialize subcommand") } - // Do a collision check - for _, sub := range ctx.subcommands { - if sub.Command == s.Command { - return nil, errors.New("new subcommand has duplicate name: " + s.Command) + // Check if the existing command name already exists. This could really be + // optimized, but since it's in a cold path, who cares. + var subcommandNames = append([]string{s.Command}, s.Aliases...) + + for _, name := range subcommandNames { + for _, sub := range ctx.subcommands { + // Check each alias against the subcommand name. + if sub.Command == name { + return nil, fmt.Errorf("new subcommand has duplicate name: %q", name) + } + + // Also check each alias against other subcommands' aliases. + for _, subalias := range sub.Aliases { + if subalias == name { + return nil, fmt.Errorf("new subcommand has duplicate alias: %q", name) + } + } } } @@ -317,6 +333,9 @@ func (ctx *Context) RegisterSubcommandCustom(cmd interface{}, name string) (*Sub return s, nil } +// emptyMentionTypes is used by Start() to not parse any mentions. +var emptyMentionTypes = []api.AllowedMentionType{} + // Start adds itself into the session handlers. This needs to be run. The // returned function is a delete function, which removes itself from the // Session handlers. @@ -358,9 +377,10 @@ func (ctx *Context) Start() func() { Content: ctx.SanitizeMessage(str), AllowedMentions: &api.AllowedMentions{ // Don't allow mentions. - Parse: []api.AllowedMentionType{}, + Parse: emptyMentionTypes, }, }) + if err != nil { ctx.ErrorLogger(err) @@ -418,14 +438,30 @@ func (ctx *Context) HelpGenerate(showHidden bool) string { if help == "" { continue } + help = IndentLines(help) - var header = "**" + sub.Command + "**" - if sub.Description != "" { - header += ": " + sub.Description + builder := strings.Builder{} + builder.WriteString("**") + builder.WriteString(sub.Command) + builder.WriteString("**") + + for _, alias := range sub.Aliases { + builder.WriteString("|") + builder.WriteString("**") + builder.WriteString(alias) + builder.WriteString("**") } - subhelps = append(subhelps, header+"\n"+help) + if sub.Description != "" { + builder.WriteString(": ") + builder.WriteString(sub.Description) + } + + builder.WriteByte('\n') + builder.WriteString(help) + + subhelps = append(subhelps, builder.String()) } if len(subhelps) > 0 { diff --git a/bot/ctx_call.go b/bot/ctx_call.go index 6393a7c..6a97bf2 100644 --- a/bot/ctx_call.go +++ b/bot/ctx_call.go @@ -335,42 +335,30 @@ Call: func (ctx *Context) findCommand(parts []string) ([]string, *MethodContext, *Subcommand, error) { // Main command entrypoint cannot have plumb. for _, c := range ctx.Commands { - if c.Command == parts[0] { + if searchStringAndSlice(parts[0], c.Command, c.Aliases) { return parts[1:], c, ctx.Subcommand, nil } - // Check for alias - for _, alias := range c.Aliases { - if alias == parts[0] { - return parts[1:], c, ctx.Subcommand, nil - } - } } // Can't find the command, look for subcommands if len(args) has a 2nd // entry. for _, s := range ctx.subcommands { - if s.Command != parts[0] { + if !searchStringAndSlice(parts[0], s.Command, s.Aliases) { continue } // Only actually plumb if we actually have a plumbed handler AND // 1. We only have one command handler OR // 2. We only have the subcommand name but no command. - if s.plumbed != nil && (len(s.Commands) == 1 || len(parts) <= 2) { + if s.IsPlumbed() && (len(s.Commands) == 1 || len(parts) <= 2) { return parts[1:], s.plumbed, s, nil } if len(parts) >= 2 { for _, c := range s.Commands { - if c.Command == parts[1] { + if searchStringAndSlice(parts[1], c.Command, c.Aliases) { return parts[2:], c, s, nil } - // Check for aliases - for _, alias := range c.Aliases { - if alias == parts[1] { - return parts[2:], c, s, nil - } - } } } @@ -395,6 +383,22 @@ func (ctx *Context) findCommand(parts []string) ([]string, *MethodContext, *Subc } } +// searchStringAndSlice searches if str is equal to isString or any of the given +// otherStrings. It is used for alias matching. +func searchStringAndSlice(str string, isString string, otherStrings []string) bool { + if str == isString { + return true + } + + for _, other := range otherStrings { + if other == str { + return true + } + } + + return false +} + func errNoBreak(err error) error { if errors.Is(err, Break) { return nil diff --git a/bot/ctx_test.go b/bot/ctx_test.go index 95c9fc0..c4bea2b 100644 --- a/bot/ctx_test.go +++ b/bot/ctx_test.go @@ -23,17 +23,17 @@ type testc struct { } func (t *testc) Setup(sub *Subcommand) { - sub.AddMiddleware("*,GetCounter", func(v interface{}) { + sub.AddMiddleware([]string{"*", "GetCounter"}, func(v interface{}) { t.Counter++ }) sub.AddMiddleware("*", func(*gateway.MessageCreateEvent) { t.Counter++ }) // stub middleware for testing - sub.AddMiddleware("OnTyping", func(*gateway.TypingStartEvent) { + sub.AddMiddleware(t.OnTyping, func(*gateway.TypingStartEvent) { t.Typed = 2 }) - sub.Hide("Hidden") + sub.Hide(t.Hidden) } func (t *testc) Hidden(*gateway.MessageCreateEvent) {} func (t *testc) Noop(*gateway.MessageCreateEvent) {} @@ -120,26 +120,6 @@ func TestContext(t *testing.T) { } }) - t.Run("help", func(t *testing.T) { - ctx.MustRegisterSubcommandCustom(&testc{}, "helper") - - h := ctx.Help() - if h == "" { - t.Fatal("Empty help?") - } - - if strings.Contains(h, "hidden") { - t.Fatal("Hidden command shown in help.") - } - - if !strings.Contains(h, "arikawa/bot test") { - t.Fatal("Name not found.") - } - if !strings.Contains(h, "Just a test.") { - t.Fatal("Description not found.") - } - }) - t.Run("middleware", func(t *testing.T) { ctx.HasPrefix = NewPrefix("pls do ") @@ -298,14 +278,42 @@ func TestContext(t *testing.T) { }) t.Run("register subcommand custom", func(t *testing.T) { - ctx.MustRegisterSubcommandCustom(&testc{}, "arikawa") + ctx.MustRegisterSubcommand(&testc{}, "arikawa", "a") }) t.Run("duplicate subcommand", func(t *testing.T) { - _, err := ctx.RegisterSubcommandCustom(&testc{}, "arikawa") + _, err := ctx.RegisterSubcommand(&testc{}, "arikawa") if err := err.Error(); !strings.Contains(err, "duplicate") { t.Fatal("Unexpected error:", err) } + + _, err = ctx.RegisterSubcommand(&testc{}, "a") + if err := err.Error(); !strings.Contains(err, "duplicate") { + t.Fatal("Unexpected error:", err) + } + }) + + t.Run("help", func(t *testing.T) { + ctx.MustRegisterSubcommand(&testc{}, "helper") + + h := ctx.Help() + if h == "" { + t.Fatal("Empty help?") + } + + if strings.Contains(h, "hidden") { + t.Fatal("Hidden command shown in help.") + } + + if !strings.Contains(h, "arikawa/bot test") { + t.Fatal("Name not found.") + } + if !strings.Contains(h, "Just a test.") { + t.Fatal("Description not found.") + } + if !strings.Contains(h, "**a**") { + t.Fatal("arikawa alias `a' not found.") + } }) t.Run("start", func(t *testing.T) { diff --git a/bot/subcommand.go b/bot/subcommand.go index ad9d51a..b465206 100644 --- a/bot/subcommand.go +++ b/bot/subcommand.go @@ -2,6 +2,7 @@ package bot import ( "reflect" + "runtime" "strings" "github.com/pkg/errors" @@ -72,6 +73,9 @@ type Subcommand struct { // Parsed command name: Command string + // Aliases is alternative way to call this subcommand in Discord. + Aliases []string + // SanitizeMessage is executed on the message content if the method returns // a string content or a SendMessageData. SanitizeMessage func(content string) string @@ -147,20 +151,74 @@ func lowerFirstLetter(name string) string { return strings.ToLower(string(name[0])) + name[1:] } -// FindCommand finds the MethodContext. It panics if methodName is not found. -func (sub *Subcommand) FindCommand(methodName string) *MethodContext { +// FindCommand finds the MethodContext using either the given method or the +// given method name. It panics if the given method is not found. +// +// There are two ways to use FindCommand: +// +// sub.FindCommand("MethodName") +// sub.FindCommand(thing.MethodName) +// +func (sub *Subcommand) FindCommand(method interface{}) *MethodContext { + return sub.findMethod(method, false) +} + +func (sub *Subcommand) findMethod(method interface{}, inclEvents bool) *MethodContext { + methodName, ok := method.(string) + if !ok { + methodName = runtimeMethodName(method) + } + for _, c := range sub.Commands { if c.MethodName == methodName { return c } } - panic("Can't find method " + methodName) + + if inclEvents { + for _, ev := range sub.Events { + if ev.MethodName == methodName { + return ev + } + } + } + + panic("can't find method " + methodName) } -// ChangeCommandInfo changes the matched methodName's Command and Description. -// Empty means unchanged. This function panics if methodName is not found. -func (sub *Subcommand) ChangeCommandInfo(methodName, cmd, desc string) { - var command = sub.FindCommand(methodName) +// runtimeMethodName returns the name of the method from the given method call. +// It is used as such: +// +// fmt.Println(methodName(t.Method_dash)) +// // Output: main.T.Method_dash-fm +// +func runtimeMethodName(v interface{}) string { + // https://github.com/diamondburned/arikawa/issues/146 + + ptr := reflect.ValueOf(v).Pointer() + + funcPC := runtime.FuncForPC(ptr) + if funcPC == nil { + panic("given method is not a function") + } + + funcName := funcPC.Name() + + // Do weird string parsing because Go wants us to. + nameParts := strings.Split(funcName, ".") + mName := nameParts[len(nameParts)-1] + nameParts = strings.Split(mName, "-") + if len(nameParts) > 1 { // extract the string before -fm if possible + mName = nameParts[len(nameParts)-2] + } + + return mName +} + +// ChangeCommandInfo changes the matched method's Command and Description. +// Empty means unchanged. This function panics if the given method is not found. +func (sub *Subcommand) ChangeCommandInfo(method interface{}, cmd, desc string) { + var command = sub.FindCommand(method) if cmd != "" { command.Command = cmd } @@ -186,12 +244,13 @@ func (sub *Subcommand) HelpShowHidden(showHidden bool) string { return sub.HelpGenerate(showHidden) } -// HelpGenerate auto-generates a help message. Use this only if you want to -// override the Subcommand's help, else use Help(). This function will show +// HelpGenerate auto-generates a help message, which contains only a list of +// commands. It does not print the subcommand header. Use this only if you want +// to override the Subcommand's help, else use Help(). This function will show // hidden commands if showHidden is true. func (sub *Subcommand) HelpGenerate(showHidden bool) string { // A wider space character. - const s = "\u2000" + const space = "\u2000" var buf strings.Builder @@ -200,22 +259,37 @@ func (sub *Subcommand) HelpGenerate(showHidden bool) string { continue } - buf.WriteString(sub.Command + " " + cmd.Command) + buf.WriteString(sub.Command) + + if !sub.IsPlumbed() { + buf.WriteByte(' ') + buf.WriteString(cmd.Command) + } + + for _, alias := range cmd.Aliases { + buf.WriteByte('|') + buf.WriteString(alias) + } // Write the usages first. - for _, usage := range cmd.Usage() { - // Is the last argument trailing? If so, append ellipsis. - if cmd.Variadic { - usage += "..." - } + var usages = cmd.Usage() + for _, usage := range usages { // Uses \u2000, which is wider than a space. - buf.WriteString(s + "__" + usage + "__") + buf.WriteString(space + "__") // const concat + buf.WriteString(usage) + buf.WriteString("__") + } + + // Is the last argument trailing? If so, append ellipsis. + if len(usages) > 0 && cmd.Variadic { + buf.WriteString("...") } // Write the description if there's any. if cmd.Description != "" { - buf.WriteString(": " + cmd.Description) + buf.WriteString(": ") + buf.WriteString(cmd.Description) } // Add a new line if this isn't the last command. @@ -229,8 +303,8 @@ func (sub *Subcommand) HelpGenerate(showHidden bool) string { // Hide marks a command as hidden, meaning it won't be shown in help and its // UnknownCommand errors will be suppressed. -func (sub *Subcommand) Hide(methodName string) { - sub.FindCommand(methodName).Hidden = true +func (sub *Subcommand) Hide(method interface{}) { + sub.FindCommand(method).Hidden = true } func (sub *Subcommand) reflectCommands() error { @@ -347,7 +421,7 @@ func (sub *Subcommand) parseCommands() error { // // Note that although technically all of the above function signatures are // acceptable, one should almost always return only an error. -func (sub *Subcommand) AddMiddleware(methodName string, middleware interface{}) { +func (sub *Subcommand) AddMiddleware(method, middleware interface{}) { var mw *MiddlewareContext // Allow *MiddlewareContext to be passed into. if v, ok := middleware.(*MiddlewareContext); ok { @@ -356,8 +430,18 @@ func (sub *Subcommand) AddMiddleware(methodName string, middleware interface{}) mw = ParseMiddleware(middleware) } - // Parse method name: - for _, method := range strings.Split(methodName, ",") { + switch v := method.(type) { + case string: + sub.addMiddleware(mw, strings.Split(v, ",")) + case []string: + sub.addMiddleware(mw, v) + default: + sub.findMethod(v, true).addMiddleware(mw) + } +} + +func (sub *Subcommand) addMiddleware(mw *MiddlewareContext, methods []string) { + for _, method := range methods { // Trim space. if method = strings.TrimSpace(method); method == "*" { // Append middleware to global middleware slice. @@ -365,19 +449,10 @@ func (sub *Subcommand) AddMiddleware(methodName string, middleware interface{}) continue } // Append middleware to that individual function. - sub.findMethod(method).addMiddleware(mw) + sub.findMethod(method, true).addMiddleware(mw) } } -func (sub *Subcommand) findMethod(name string) *MethodContext { - for _, ev := range sub.Events { - if ev.MethodName == name { - return ev - } - } - return sub.FindCommand(name) -} - func (sub *Subcommand) eventCallers(evT reflect.Type) (callers []caller) { // Search for global middlewares. for _, mw := range sub.globalmws { @@ -403,13 +478,29 @@ func (sub *Subcommand) eventCallers(evT reflect.Type) (callers []caller) { return } -// SetPlumb sets the method as the plumbed command. -func (sub *Subcommand) SetPlumb(methodName string) { - sub.plumbed = sub.FindCommand(methodName) +// IsPlumbed returns true if the subcommand is plumbed. +func (sub *Subcommand) IsPlumbed() bool { + return sub.plumbed != nil +} + +// SetPlumb sets the method as the plumbed command. If method is nil, then the +// plumbing is also disabled. +func (sub *Subcommand) SetPlumb(method interface{}) { + // Ensure that SetPlumb isn't being called on the main context. + if sub.Command == "" { + panic("invalid SetPlumb call on *Context") + } + + if method == nil { + sub.plumbed = nil + return + } + + sub.plumbed = sub.FindCommand(method) } // AddAliases add alias(es) to specific command (defined with commandName). -func (sub *Subcommand) AddAliases(commandName string, aliases ...string) { +func (sub *Subcommand) AddAliases(commandName interface{}, aliases ...string) { // Get command command := sub.FindCommand(commandName) @@ -428,7 +519,7 @@ func (sub *Subcommand) DeriveIntents() gateway.Intents { for _, command := range sub.Commands { intents |= command.intents() } - if sub.plumbed != nil { + if sub.IsPlumbed() { intents |= sub.plumbed.intents() } for _, middleware := range sub.globalmws {