From 3d75e9e7112f8e14896483dfefbf734ff05952cf Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Wed, 25 May 2016 12:05:14 -0400 Subject: [PATCH] Go with interfaces + private opaque types rather than public types that wrap slices --- app.go | 27 +++++++----------- app_test.go | 59 ++++++++++++++++++-------------------- args.go | 62 ++++++++++++++++++++-------------------- category.go | 76 +++++++++++++++++++++++++++++++++---------------- command.go | 8 +++--- context.go | 7 +++-- errors.go | 50 +++++++++++++++++++------------- errors_test.go | 6 ++-- help.go | 21 +++++++++++--- help_test.go | 8 +++--- helpers_test.go | 4 +++ 11 files changed, 187 insertions(+), 141 deletions(-) diff --git a/app.go b/app.go index 15ee743..b16f991 100644 --- a/app.go +++ b/app.go @@ -36,8 +36,8 @@ type App struct { HideHelp bool // Boolean to hide built-in version flag and the VERSION section of help HideVersion bool - // Populate on app startup, only gettable through method Categories() - categories *CommandCategories + // Categories contains the categorized commands and is populated on app startup + Categories CommandCategories // An action to execute when the bash-completion flag is set BashComplete BashCompleteFunc // An action to execute before any subcommands are run, but after the context is ready @@ -113,11 +113,11 @@ func (a *App) Setup() { } a.Commands = newCmds - a.categories = NewCommandCategories() + a.Categories = newCommandCategories() for _, command := range a.Commands { - a.categories.AddCommand(command.Category, command) + a.Categories.AddCommand(command.Category, command) } - sort.Sort(a.categories) + sort.Sort(a.Categories.(*commandCategories)) // append help to commands if a.Command(helpCommand.Name) == nil && !a.HideHelp { @@ -184,7 +184,7 @@ func (a *App) Run(arguments []string) (err error) { defer func() { if afterErr := a.After(context); afterErr != nil { if err != nil { - err = NewMultiError(err, afterErr) + err = newMultiError(err, afterErr) } else { err = afterErr } @@ -296,7 +296,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if afterErr != nil { HandleExitCoder(err) if err != nil { - err = NewMultiError(err, afterErr) + err = newMultiError(err, afterErr) } else { err = afterErr } @@ -340,17 +340,12 @@ func (a *App) Command(name string) *Command { return nil } -// Categories returns a slice containing all the categories with the commands they contain -func (a *App) Categories() *CommandCategories { - return a.categories -} - // VisibleCategories returns a slice of categories and commands that are // Hidden=false -func (a *App) VisibleCategories() []*CommandCategory { - ret := []*CommandCategory{} - for _, category := range a.categories.Categories() { - if visible := func() *CommandCategory { +func (a *App) VisibleCategories() []CommandCategory { + ret := []CommandCategory{} + for _, category := range a.Categories.Categories() { + if visible := func() CommandCategory { if len(category.VisibleCommands()) > 0 { return category } diff --git a/app_test.go b/app_test.go index 7bac196..99bd6a3 100644 --- a/app_test.go +++ b/app_test.go @@ -216,7 +216,7 @@ func TestApp_RunAsSubcommandParseFlags(t *testing.T) { func TestApp_CommandWithFlagBeforeTerminator(t *testing.T) { var parsedOption string - var args *Args + var args Args app := NewApp() command := &Command{ @@ -241,7 +241,7 @@ func TestApp_CommandWithFlagBeforeTerminator(t *testing.T) { } func TestApp_CommandWithDash(t *testing.T) { - var args *Args + var args Args app := NewApp() command := &Command{ @@ -260,7 +260,7 @@ func TestApp_CommandWithDash(t *testing.T) { } func TestApp_CommandWithNoFlagBeforeTerminator(t *testing.T) { - var args *Args + var args Args app := NewApp() command := &Command{ @@ -1142,25 +1142,24 @@ func TestApp_Run_Categories(t *testing.T) { app.Run([]string{"categories"}) - expect := &CommandCategories{ - slice: []*CommandCategory{ - { - Name: "1", - commands: []*Command{ - app.Commands[0], - app.Commands[1], - }, - }, - { - Name: "2", - commands: []*Command{ - app.Commands[2], - }, + expect := commandCategories([]*commandCategory{ + { + name: "1", + commands: []*Command{ + app.Commands[0], + app.Commands[1], }, }, - } - if !reflect.DeepEqual(app.Categories(), expect) { - t.Fatalf("expected categories %#v, to equal %#v", app.Categories(), expect) + { + name: "2", + commands: []*Command{ + app.Commands[2], + }, + }, + }) + + if !reflect.DeepEqual(app.Categories, &expect) { + t.Fatalf("expected categories %#v, to equal %#v", app.Categories, &expect) } output := buf.String() @@ -1193,15 +1192,15 @@ func TestApp_VisibleCategories(t *testing.T) { }, } - expected := []*CommandCategory{ - { - Name: "2", + expected := []CommandCategory{ + &commandCategory{ + name: "2", commands: []*Command{ app.Commands[1], }, }, - { - Name: "3", + &commandCategory{ + name: "3", commands: []*Command{ app.Commands[2], }, @@ -1233,9 +1232,9 @@ func TestApp_VisibleCategories(t *testing.T) { }, } - expected = []*CommandCategory{ - { - Name: "3", + expected = []CommandCategory{ + &commandCategory{ + name: "3", commands: []*Command{ app.Commands[2], }, @@ -1268,10 +1267,8 @@ func TestApp_VisibleCategories(t *testing.T) { }, } - expected = []*CommandCategory{} - app.Setup() - expect(t, expected, app.VisibleCategories()) + expect(t, []CommandCategory{}, app.VisibleCategories()) } func TestApp_Run_DoesNotOverwriteErrorFromBefore(t *testing.T) { diff --git a/args.go b/args.go index ab44f43..5618a47 100644 --- a/args.go +++ b/args.go @@ -6,55 +6,55 @@ var ( argsRangeErr = errors.New("index out of range") ) -// Args wraps a string slice with some convenience methods -type Args struct { - slice []string +type Args interface { + // Get returns the nth argument, or else a blank string + Get(n int) string + // First returns the first argument, or else a blank string + First() string + // Tail returns the rest of the arguments (not the first one) + // or else an empty string slice + Tail() []string + // Len returns the length of the wrapped slice + Len() int + // Present checks if there are any arguments present + Present() bool + // Slice returns a copy of the internal slice + Slice() []string } -// Get returns the nth argument, or else a blank string -func (a *Args) Get(n int) string { - if len(a.slice) > n { - return a.slice[n] +type args []string + +func (a *args) Get(n int) string { + if len(*a) > n { + return (*a)[n] } return "" } -// First returns the first argument, or else a blank string -func (a *Args) First() string { +func (a *args) First() string { return a.Get(0) } -// Tail returns the rest of the arguments (not the first one) -// or else an empty string slice -func (a *Args) Tail() []string { +func (a *args) Tail() []string { if a.Len() >= 2 { - return a.slice[1:] + tail := []string((*a)[1:]) + ret := make([]string, len(tail)) + copy(ret, tail) + return ret } return []string{} } -// Len returns the length of the wrapped slice -func (a *Args) Len() int { - return len(a.slice) +func (a *args) Len() int { + return len(*a) } -// Present checks if there are any arguments present -func (a *Args) Present() bool { +func (a *args) Present() bool { return a.Len() != 0 } -// Swap swaps arguments at the given indexes -func (a *Args) Swap(from, to int) error { - if from >= a.Len() || to >= a.Len() { - return argsRangeErr - } - a.slice[from], a.slice[to] = a.slice[to], a.slice[from] - return nil -} - -// Slice returns a copy of the internal slice -func (a *Args) Slice() []string { - ret := make([]string, len(a.slice)) - copy(ret, a.slice) +func (a *args) Slice() []string { + ret := make([]string, len(*a)) + copy(ret, []string(*a)) return ret } diff --git a/category.go b/category.go index 5899338..f2cb939 100644 --- a/category.go +++ b/category.go @@ -1,54 +1,80 @@ package cli -// CommandCategories wraps a slice of *CommandCategory. -type CommandCategories struct { - slice []*CommandCategory +type CommandCategories interface { + // AddCommand adds a command to a category, creating a new category if necessary. + AddCommand(category string, command *Command) + // Categories returns a copy of the category slice + Categories() []CommandCategory } -func NewCommandCategories() *CommandCategories { - return &CommandCategories{slice: []*CommandCategory{}} +type commandCategories []*commandCategory + +func newCommandCategories() CommandCategories { + ret := commandCategories([]*commandCategory{}) + return &ret } -func (c *CommandCategories) Less(i, j int) bool { - return c.slice[i].Name < c.slice[j].Name +func (c *commandCategories) Less(i, j int) bool { + return (*c)[i].Name() < (*c)[j].Name() } -func (c *CommandCategories) Len() int { - return len(c.slice) +func (c *commandCategories) Len() int { + return len(*c) } -func (c *CommandCategories) Swap(i, j int) { - c.slice[i], c.slice[j] = c.slice[j], c.slice[i] +func (c *commandCategories) Swap(i, j int) { + (*c)[i], (*c)[j] = (*c)[j], (*c)[i] } -// AddCommand adds a command to a category, creating a new category if necessary. -func (c *CommandCategories) AddCommand(category string, command *Command) { - for _, commandCategory := range c.slice { - if commandCategory.Name == category { +func (c *commandCategories) AddCommand(category string, command *Command) { + for _, commandCategory := range []*commandCategory(*c) { + if commandCategory.name == category { commandCategory.commands = append(commandCategory.commands, command) return } } - c.slice = append(c.slice, - &CommandCategory{Name: category, commands: []*Command{command}}) + newVal := commandCategories(append(*c, + &commandCategory{name: category, commands: []*Command{command}})) + (*c) = newVal } -// Categories returns a copy of the category slice -func (c *CommandCategories) Categories() []*CommandCategory { - ret := make([]*CommandCategory, len(c.slice)) - copy(ret, c.slice) +func (c *commandCategories) Categories() []CommandCategory { + ret := []CommandCategory{} + for _, cat := range *c { + ret = append(ret, cat) + } return ret } // CommandCategory is a category containing commands. -type CommandCategory struct { - Name string +type CommandCategory interface { + // Name returns the category name string + Name() string + // VisibleCommands returns a slice of the Commands with Hidden=false + VisibleCommands() []*Command +} +type commandCategory struct { + name string commands []*Command } -// VisibleCommands returns a slice of the Commands with Hidden=false -func (c *CommandCategory) VisibleCommands() []*Command { +func newCommandCategory(name string) *commandCategory { + return &commandCategory{ + name: name, + commands: []*Command{}, + } +} + +func (c *commandCategory) Name() string { + return c.name +} + +func (c *commandCategory) VisibleCommands() []*Command { + if c.commands == nil { + c.commands = []*Command{} + } + ret := []*Command{} for _, command := range c.commands { if !command.Hidden { diff --git a/command.go b/command.go index c8f6099..f05f1e2 100644 --- a/command.go +++ b/command.go @@ -120,7 +120,7 @@ func (c *Command) Run(ctx *Context) (err error) { if afterErr != nil { HandleExitCoder(err) if err != nil { - err = NewMultiError(err, afterErr) + err = newMultiError(err, afterErr) } else { err = afterErr } @@ -193,12 +193,12 @@ func (c *Command) startApp(ctx *Context) error { app.Compiled = ctx.App.Compiled app.Writer = ctx.App.Writer - app.categories = NewCommandCategories() + app.Categories = newCommandCategories() for _, command := range c.Subcommands { - app.categories.AddCommand(command.Category, command) + app.Categories.AddCommand(command.Category, command) } - sort.Sort(app.categories) + sort.Sort(app.Categories.(*commandCategories)) // bash completion app.EnableBashCompletion = ctx.App.EnableBashCompletion diff --git a/context.go b/context.go index 39aa9be..b43c957 100644 --- a/context.go +++ b/context.go @@ -10,7 +10,7 @@ import ( // Context is a type that is passed through to // each Handler action in a cli application. Context -// can be used to retrieve context-specific Args and +// can be used to retrieve context-specific args and // parsed command-line options. type Context struct { App *App @@ -148,8 +148,9 @@ func (c *Context) Lineage() []*Context { } // Args returns the command line arguments associated with the context. -func (c *Context) Args() *Args { - return &Args{slice: c.flagSet.Args()} +func (c *Context) Args() Args { + ret := args(c.flagSet.Args()) + return &ret } // NArg returns the number of the command line arguments. diff --git a/errors.go b/errors.go index a8091f4..ba01537 100644 --- a/errors.go +++ b/errors.go @@ -15,25 +15,39 @@ var OsExiter = os.Exit var ErrWriter io.Writer = os.Stderr // MultiError is an error that wraps multiple errors. -type MultiError struct { - errors []error +type MultiError interface { + error + // Errors returns a copy of the errors slice + Errors() []error } // NewMultiError creates a new MultiError. Pass in one or more errors. -func NewMultiError(err ...error) MultiError { - return MultiError{errors: err} +func newMultiError(err ...error) MultiError { + ret := multiError(err) + return &ret } -// Error implents the error interface. -func (m MultiError) Error() string { - errs := make([]string, len(m.errors)) - for i, err := range m.errors { +type multiError []error + +// Error implements the error interface. +func (m *multiError) Error() string { + errs := make([]string, len(*m)) + for i, err := range *m { errs[i] = err.Error() } return strings.Join(errs, "\n") } +// Errors returns a copy of the errors slice +func (m *multiError) Errors() []error { + errs := make([]error, len(*m)) + for _, err := range *m { + errs = append(errs, err) + } + return errs +} + // ExitCoder is the interface checked by `App` and `Command` for a custom exit // code type ExitCoder interface { @@ -41,29 +55,25 @@ type ExitCoder interface { ExitCode() int } -// ExitError fulfills both the builtin `error` interface and `ExitCoder` -type ExitError struct { +type exitError struct { exitCode int message string } -// NewExitError makes a new *ExitError -func NewExitError(message string, exitCode int) *ExitError { - return &ExitError{ +// Exit wraps a message and exit code into an ExitCoder suitable for handling by +// HandleExitCoder +func Exit(message string, exitCode int) ExitCoder { + return &exitError{ exitCode: exitCode, message: message, } } -// Error returns the string message, fulfilling the interface required by -// `error` -func (ee *ExitError) Error() string { +func (ee *exitError) Error() string { return ee.message } -// ExitCode returns the exit code, fulfilling the interface required by -// `ExitCoder` -func (ee *ExitError) ExitCode() int { +func (ee *exitError) ExitCode() int { return ee.exitCode } @@ -85,7 +95,7 @@ func HandleExitCoder(err error) { } if multiErr, ok := err.(MultiError); ok { - for _, merr := range multiErr.errors { + for _, merr := range multiErr.Errors() { HandleExitCoder(merr) } } diff --git a/errors_test.go b/errors_test.go index 8f5f284..5b4981f 100644 --- a/errors_test.go +++ b/errors_test.go @@ -34,7 +34,7 @@ func TestHandleExitCoder_ExitCoder(t *testing.T) { defer func() { OsExiter = os.Exit }() - HandleExitCoder(NewExitError("galactic perimeter breach", 9)) + HandleExitCoder(Exit("galactic perimeter breach", 9)) expect(t, exitCode, 9) expect(t, called, true) @@ -51,8 +51,8 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { defer func() { OsExiter = os.Exit }() - exitErr := NewExitError("galactic perimeter breach", 9) - err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr) + exitErr := Exit("galactic perimeter breach", 9) + err := newMultiError(errors.New("wowsa"), errors.New("egad"), exitErr) HandleExitCoder(err) expect(t, exitCode, 9) diff --git a/help.go b/help.go index d02f7fd..c21639d 100644 --- a/help.go +++ b/help.go @@ -149,7 +149,7 @@ func ShowCommandHelp(ctx *Context, command string) error { } if ctx.App.CommandNotFound == nil { - return NewExitError(fmt.Sprintf("No help topic for '%v'", command), 3) + return Exit(fmt.Sprintf("No help topic for '%v'", command), 3) } ctx.App.CommandNotFound(ctx, command) @@ -201,15 +201,28 @@ func printHelp(out io.Writer, templ string, data interface{}) { w := tabwriter.NewWriter(out, 0, 8, 1, '\t', 0) t := template.Must(template.New("help").Funcs(funcMap).Parse(templ)) + + errDebug := os.Getenv("CLI_TEMPLATE_ERROR_DEBUG") != "" + + defer func() { + if r := recover(); r != nil { + if errDebug { + fmt.Fprintf(ErrWriter, "CLI TEMPLATE PANIC: %#v\n", r) + } + if os.Getenv("CLI_TEMPLATE_REPANIC") != "" { + panic(r) + } + } + }() + err := t.Execute(w, data) if err != nil { - // If the writer is closed, t.Execute will fail, and there's nothing - // we can do to recover. - if os.Getenv("CLI_TEMPLATE_ERROR_DEBUG") != "" { + if errDebug { fmt.Fprintf(ErrWriter, "CLI TEMPLATE ERROR: %#v\n", err) } return } + w.Flush() } diff --git a/help_test.go b/help_test.go index 4d1dedc..b81701c 100644 --- a/help_test.go +++ b/help_test.go @@ -129,9 +129,9 @@ func Test_helpCommand_Action_ErrorIfNoTopic(t *testing.T) { t.Fatalf("expected error from helpCommand.Action(), but got nil") } - exitErr, ok := err.(*ExitError) + exitErr, ok := err.(*exitError) if !ok { - t.Fatalf("expected ExitError from helpCommand.Action(), but instead got: %v", err.Error()) + t.Fatalf("expected *exitError from helpCommand.Action(), but instead got: %v", err.Error()) } if !strings.HasPrefix(exitErr.Error(), "No help topic for") { @@ -157,9 +157,9 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) { t.Fatalf("expected error from helpCommand.Action(), but got nil") } - exitErr, ok := err.(*ExitError) + exitErr, ok := err.(*exitError) if !ok { - t.Fatalf("expected ExitError from helpCommand.Action(), but instead got: %v", err.Error()) + t.Fatalf("expected *exitError from helpCommand.Action(), but instead got: %v", err.Error()) } if !strings.HasPrefix(exitErr.Error(), "No help topic for") { diff --git a/helpers_test.go b/helpers_test.go index 109ea7a..bcfa46b 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -12,6 +12,10 @@ var ( wd, _ = os.Getwd() ) +func init() { + os.Setenv("CLI_TEMPLATE_REPANIC", "1") +} + func expect(t *testing.T, a interface{}, b interface{}) { _, fn, line, _ := runtime.Caller(1) fn = strings.Replace(fn, wd+"/", "", -1)