Make slice wrapping pattern more consistent

and move *Args out into its own file

app.go

@@ -115,7 +115,7 @@ func (a *App) Setup() {
 
 	a.categories = NewCommandCategories()
 	for _, command := range a.Commands {
-		a.categories = a.categories.AddCommand(command.Category, command)
+		a.categories.AddCommand(command.Category, command)
 	}
 	sort.Sort(a.categories)
 
@@ -349,12 +349,10 @@ func (a *App) Categories() *CommandCategories {
 // Hidden=false
 func (a *App) VisibleCategories() []*CommandCategory {
 	ret := []*CommandCategory{}
-	for _, category := range a.categories.Categories {
+	for _, category := range a.categories.Categories() {
 		if visible := func() *CommandCategory {
-			for _, command := range category.Commands {
+			if len(category.VisibleCommands()) > 0 {
-				if !command.Hidden {
+				return category
-					return category
-				}
 			}
 			return nil
 		}(); visible != nil {

app_test.go

@@ -1143,17 +1143,17 @@ func TestApp_Run_Categories(t *testing.T) {
 	app.Run([]string{"categories"})
 
 	expect := &CommandCategories{
-		Categories: []*CommandCategory{
+		slice: []*CommandCategory{
 			{
 				Name: "1",
-				Commands: []*Command{
+				commands: []*Command{
 					app.Commands[0],
 					app.Commands[1],
 				},
 			},
 			{
 				Name: "2",
-				Commands: []*Command{
+				commands: []*Command{
 					app.Commands[2],
 				},
 			},
@@ -1196,13 +1196,13 @@ func TestApp_VisibleCategories(t *testing.T) {
 	expected := []*CommandCategory{
 		{
 			Name: "2",
-			Commands: []*Command{
+			commands: []*Command{
 				app.Commands[1],
 			},
 		},
 		{
 			Name: "3",
-			Commands: []*Command{
+			commands: []*Command{
 				app.Commands[2],
 			},
 		},
@@ -1236,7 +1236,7 @@ func TestApp_VisibleCategories(t *testing.T) {
 	expected = []*CommandCategory{
 		{
 			Name: "3",
-			Commands: []*Command{
+			commands: []*Command{
 				app.Commands[2],
 			},
 		},

args.go

@@ -0,0 +1,60 @@
+package cli
+
+import "errors"
+
+var (
+	argsRangeErr = errors.New("index out of range")
+)
+
+// Args wraps a string slice with some convenience methods
+type Args struct {
+	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]
+	}
+	return ""
+}
+
+// First returns the first argument, or else a blank 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 {
+	if a.Len() >= 2 {
+		return a.slice[1:]
+	}
+	return []string{}
+}
+
+// Len returns the length of the wrapped slice
+func (a *Args) Len() int {
+	return len(a.slice)
+}
+
+// Present checks if there are any arguments present
+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)
+	return ret
+}

category.go

@@ -1,49 +1,56 @@
 package cli
 
-// CommandCategories is a slice of *CommandCategory.
+// CommandCategories wraps a slice of *CommandCategory.
 type CommandCategories struct {
-	Categories []*CommandCategory
+	slice []*CommandCategory
 }
 
 func NewCommandCategories() *CommandCategories {
-	return &CommandCategories{Categories: []*CommandCategory{}}
+	return &CommandCategories{slice: []*CommandCategory{}}
+}
+
+func (c *CommandCategories) Less(i, j int) bool {
+	return c.slice[i].Name < c.slice[j].Name
+}
+
+func (c *CommandCategories) Len() int {
+	return len(c.slice)
+}
+
+func (c *CommandCategories) Swap(i, j int) {
+	c.slice[i], c.slice[j] = c.slice[j], c.slice[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 {
+			commandCategory.commands = append(commandCategory.commands, command)
+			return
+		}
+	}
+	c.slice = append(c.slice,
+		&CommandCategory{Name: category, commands: []*Command{command}})
+}
+
+// Categories returns a copy of the category slice
+func (c *CommandCategories) Categories() []*CommandCategory {
+	ret := make([]*CommandCategory, len(c.slice))
+	copy(ret, c.slice)
+	return ret
 }
 
 // CommandCategory is a category containing commands.
 type CommandCategory struct {
-	Name     string
+	Name string
-	Commands []*Command
-}
 
-func (c *CommandCategories) Less(i, j int) bool {
+	commands []*Command
-	return c.Categories[i].Name < c.Categories[j].Name
-}
-
-func (c *CommandCategories) Len() int {
-	return len(c.Categories)
-}
-
-func (c *CommandCategories) Swap(i, j int) {
-	c.Categories[i], c.Categories[j] = c.Categories[j], c.Categories[i]
-}
-
-// AddCommand adds a command to a category.
-func (c *CommandCategories) AddCommand(category string, command *Command) *CommandCategories {
-	for _, commandCategory := range c.Categories {
-		if commandCategory.Name == category {
-			commandCategory.Commands = append(commandCategory.Commands, command)
-			return c
-		}
-	}
-	c.Categories = append(c.Categories,
-		&CommandCategory{Name: category, Commands: []*Command{command}})
-	return c
 }
 
 // VisibleCommands returns a slice of the Commands with Hidden=false
 func (c *CommandCategory) VisibleCommands() []*Command {
 	ret := []*Command{}
-	for _, command := range c.Commands {
+	for _, command := range c.commands {
 		if !command.Hidden {
 			ret = append(ret, command)
 		}

command.go

@@ -195,7 +195,7 @@ func (c *Command) startApp(ctx *Context) error {
 
 	app.categories = NewCommandCategories()
 	for _, command := range c.Subcommands {
-		app.categories = app.categories.AddCommand(command.Category, command)
+		app.categories.AddCommand(command.Category, command)
 	}
 
 	sort.Sort(app.categories)

context.go

@@ -157,59 +157,6 @@ func (c *Context) NArg() int {
 	return c.Args().Len()
 }
 
-// Args wraps a string slice with some convenience methods
-type Args struct {
-	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]
-	}
-	return ""
-}
-
-// First returns the first argument, or else a blank 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 {
-	if a.Len() >= 2 {
-		return a.slice[1:]
-	}
-	return []string{}
-}
-
-// Len returns the length of the wrapped slice
-func (a *Args) Len() int {
-	return len(a.slice)
-}
-
-// Present checks if there are any arguments present
-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 errors.New("index out of range")
-	}
-	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)
-	return ret
-}
-
 func lookupFlagSet(name string, ctx *Context) *flag.FlagSet {
 	for _, c := range ctx.Lineage() {
 		if f := c.flagSet.Lookup(name); f != nil {

errors.go

@@ -16,18 +16,18 @@ var ErrWriter io.Writer = os.Stderr
 
 // MultiError is an error that wraps multiple errors.
 type MultiError struct {
-	Errors []error
+	errors []error
 }
 
 // NewMultiError creates a new MultiError. Pass in one or more errors.
 func NewMultiError(err ...error) MultiError {
-	return MultiError{Errors: err}
+	return MultiError{errors: err}
 }
 
 // Error implents the error interface.
 func (m MultiError) Error() string {
-	errs := make([]string, len(m.Errors))
+	errs := make([]string, len(m.errors))
-	for i, err := range m.Errors {
+	for i, err := range m.errors {
 		errs[i] = err.Error()
 	}
 
@@ -85,7 +85,7 @@ func HandleExitCoder(err error) {
 	}
 
 	if multiErr, ok := err.(MultiError); ok {
-		for _, merr := range multiErr.Errors {
+		for _, merr := range multiErr.errors {
 			HandleExitCoder(merr)
 		}
 	}