Merge pull request #231 from alexflint/subcommand-aliases

add subcommand aliases

6 files changed, 227 insertions, 112 deletions

diff --git a/parse.go b/parse.go
index 0bdddc7..169e8cc 100644
--- a/parse.go
+++ b/parse.go
@@ -62,6 +62,7 @@ type spec struct {
 // command represents a named subcommand, or the top-level command
 type command struct {
 	name        string
+	aliases     []string
 	help        string
 	dest        path
 	specs       []*spec
@@ -153,7 +154,7 @@ type Parser struct {
 	epilogue    string
 
 	// the following field changes during processing of command line arguments
-	lastCmd *command
+	subcommand []string
 }
 
 // Versioned is the interface that the destination struct should implement to
@@ -384,18 +385,24 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) {
 				}
 			case key == "subcommand":
 				// decide on a name for the subcommand
-				cmdname := value
-				if cmdname == "" {
-					cmdname = strings.ToLower(field.Name)
+				var cmdnames []string
+				if value == "" {
+					cmdnames = []string{strings.ToLower(field.Name)}
+				} else {
+					cmdnames = strings.Split(value, "|")
+				}
+				for i := range cmdnames {
+					cmdnames[i] = strings.TrimSpace(cmdnames[i])
 				}
 
 				// parse the subcommand recursively
-				subcmd, err := cmdFromStruct(cmdname, subdest, field.Type)
+				subcmd, err := cmdFromStruct(cmdnames[0], subdest, field.Type)
 				if err != nil {
 					errs = append(errs, err.Error())
 					return false
 				}
 
+				subcmd.aliases = cmdnames[1:]
 				subcmd.parent = &cmd
 				subcmd.help = field.Tag.Get("help")
 
@@ -514,13 +521,13 @@ func (p *Parser) MustParse(args []string) {
 	err := p.Parse(args)
 	switch {
 	case err == ErrHelp:
-		p.writeHelpForSubcommand(p.config.Out, p.lastCmd)
+		p.WriteHelpForSubcommand(p.config.Out, p.subcommand...)
 		p.config.Exit(0)
 	case err == ErrVersion:
 		fmt.Fprintln(p.config.Out, p.version)
 		p.config.Exit(0)
 	case err != nil:
-		p.failWithSubcommand(err.Error(), p.lastCmd)
+		p.FailSubcommand(err.Error(), p.subcommand...)
 	}
 }
 
@@ -577,7 +584,7 @@ func (p *Parser) process(args []string) error {
 
 	// union of specs for the chain of subcommands encountered so far
 	curCmd := p.cmd
-	p.lastCmd = curCmd
+	p.subcommand = nil
 
 	// make a copy of the specs because we will add to this list each time we expand a subcommand
 	specs := make([]*spec, len(curCmd.specs))
@@ -648,7 +655,7 @@ func (p *Parser) process(args []string) error {
 			}
 
 			curCmd = subcmd
-			p.lastCmd = curCmd
+			p.subcommand = append(p.subcommand, arg)
 			continue
 		}
 
@@ -842,6 +849,11 @@ func findSubcommand(cmds []*command, name string) *command {
 		if cmd.name == name {
 			return cmd
 		}
+		for _, alias := range cmd.aliases {
+			if alias == name {
+				return cmd
+			}
+		}
 	}
 	return nil
 }
diff --git a/parse_test.go b/parse_test.go
index 06e7a76..d53b483 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -883,7 +883,8 @@ func TestEnvironmentVariableInSubcommandIgnored(t *testing.T) {
 	require.NoError(t, err)
 
 	err = p.Parse([]string{"sub"})
-	assert.NoError(t, err)
+	require.NoError(t, err)
+	require.NotNil(t, args.Sub)
 	assert.Equal(t, "", args.Sub.Foo)
 }
 
@@ -1731,7 +1732,8 @@ func TestSubcommandGlobalFlag_InCommand_Strict_Inner(t *testing.T) {
 	require.NoError(t, err)
 
 	err = p.Parse([]string{"sub", "-g"})
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.False(t, args.Global)
+	require.NotNil(t, args.Sub)
 	assert.True(t, args.Sub.Guard)
 }
diff --git a/subcommand.go b/subcommand.go
index dff732c..da6ed11 100644
--- a/subcommand.go
+++ b/subcommand.go
@@ -1,5 +1,7 @@
 package arg
 
+import "fmt"
+
 // Subcommand returns the user struct for the subcommand selected by
 // the command line arguments most recently processed by the parser.
 // The return value is always a pointer to a struct. If no subcommand
@@ -7,31 +9,35 @@ package arg
 // no command line arguments have been processed by this parser then it
 // returns nil.
 func (p *Parser) Subcommand() interface{} {
-	if p.lastCmd == nil || p.lastCmd.parent == nil {
+	if len(p.subcommand) == 0 {
+		return nil
+	}
+	cmd, err := p.lookupCommand(p.subcommand...)
+	if err != nil {
 		return nil
 	}
-	return p.val(p.lastCmd.dest).Interface()
+	return p.val(cmd.dest).Interface()
 }
 
 // SubcommandNames returns the sequence of subcommands specified by the
 // user. If no subcommands were given then it returns an empty slice.
 func (p *Parser) SubcommandNames() []string {
-	if p.lastCmd == nil {
-		return nil
-	}
-
-	// make a list of ancestor commands
-	var ancestors []string
-	cur := p.lastCmd
-	for cur.parent != nil { // we want to exclude the root
-		ancestors = append(ancestors, cur.name)
-		cur = cur.parent
-	}
+	return p.subcommand
+}
 
-	// reverse the list
-	out := make([]string, len(ancestors))
-	for i := 0; i < len(ancestors); i++ {
-		out[i] = ancestors[len(ancestors)-i-1]
+// lookupCommand finds a subcommand based on a sequence of subcommand names. The
+// first string should be a top-level subcommand, the next should be a child
+// subcommand of that subcommand, and so on. If no strings are given then the
+// root command is returned. If no such subcommand exists then an error is
+// returned.
+func (p *Parser) lookupCommand(path ...string) (*command, error) {
+	cmd := p.cmd
+	for _, name := range path {
+		found := findSubcommand(cmd.subcommands, name)
+		if found == nil {
+			return nil, fmt.Errorf("%q is not a subcommand of %s", name, cmd.name)
+		}
+		cmd = found
 	}
-	return out
+	return cmd, nil
 }
diff --git a/subcommand_test.go b/subcommand_test.go
index 2c61dd3..00efae0 100644
--- a/subcommand_test.go
+++ b/subcommand_test.go
@@ -83,6 +83,19 @@ func TestNamedSubcommand(t *testing.T) {
 	assert.Equal(t, []string{"ls"}, p.SubcommandNames())
 }
 
+func TestSubcommandAliases(t *testing.T) {
+	type listCmd struct {
+	}
+	var args struct {
+		List *listCmd `arg:"subcommand:list|ls"`
+	}
+	p, err := pparse("ls", &args)
+	require.NoError(t, err)
+	assert.NotNil(t, args.List)
+	assert.Equal(t, args.List, p.Subcommand())
+	assert.Equal(t, []string{"ls"}, p.SubcommandNames())
+}
+
 func TestEmptySubcommand(t *testing.T) {
 	type listCmd struct {
 	}
@@ -113,6 +126,23 @@ func TestTwoSubcommands(t *testing.T) {
 	assert.Equal(t, []string{"list"}, p.SubcommandNames())
 }
 
+func TestTwoSubcommandsWithAliases(t *testing.T) {
+	type getCmd struct {
+	}
+	type listCmd struct {
+	}
+	var args struct {
+		Get  *getCmd  `arg:"subcommand:get|g"`
+		List *listCmd `arg:"subcommand:list|ls"`
+	}
+	p, err := pparse("ls", &args)
+	require.NoError(t, err)
+	assert.Nil(t, args.Get)
+	assert.NotNil(t, args.List)
+	assert.Equal(t, args.List, p.Subcommand())
+	assert.Equal(t, []string{"ls"}, p.SubcommandNames())
+}
+
 func TestSubcommandsWithOptions(t *testing.T) {
 	type getCmd struct {
 		Name string
@@ -275,6 +305,60 @@ func TestNestedSubcommands(t *testing.T) {
 	}
 }
 
+func TestNestedSubcommandsWithAliases(t *testing.T) {
+	type child struct{}
+	type parent struct {
+		Child *child `arg:"subcommand:child|ch"`
+	}
+	type grandparent struct {
+		Parent *parent `arg:"subcommand:parent|pa"`
+	}
+	type root struct {
+		Grandparent *grandparent `arg:"subcommand:grandparent|gp"`
+	}
+
+	{
+		var args root
+		p, err := pparse("gp parent child", &args)
+		require.NoError(t, err)
+		require.NotNil(t, args.Grandparent)
+		require.NotNil(t, args.Grandparent.Parent)
+		require.NotNil(t, args.Grandparent.Parent.Child)
+		assert.Equal(t, args.Grandparent.Parent.Child, p.Subcommand())
+		assert.Equal(t, []string{"gp", "parent", "child"}, p.SubcommandNames())
+	}
+
+	{
+		var args root
+		p, err := pparse("grandparent pa", &args)
+		require.NoError(t, err)
+		require.NotNil(t, args.Grandparent)
+		require.NotNil(t, args.Grandparent.Parent)
+		require.Nil(t, args.Grandparent.Parent.Child)
+		assert.Equal(t, args.Grandparent.Parent, p.Subcommand())
+		assert.Equal(t, []string{"grandparent", "pa"}, p.SubcommandNames())
+	}
+
+	{
+		var args root
+		p, err := pparse("grandparent", &args)
+		require.NoError(t, err)
+		require.NotNil(t, args.Grandparent)
+		require.Nil(t, args.Grandparent.Parent)
+		assert.Equal(t, args.Grandparent, p.Subcommand())
+		assert.Equal(t, []string{"grandparent"}, p.SubcommandNames())
+	}
+
+	{
+		var args root
+		p, err := pparse("", &args)
+		require.NoError(t, err)
+		require.Nil(t, args.Grandparent)
+		assert.Nil(t, p.Subcommand())
+		assert.Empty(t, p.SubcommandNames())
+	}
+}
+
 func TestSubcommandsWithPositionals(t *testing.T) {
 	type listCmd struct {
 		Pattern string `arg:"positional"`
@@ -411,3 +495,14 @@ func TestValForNilStruct(t *testing.T) {
 	v := p.val(path{fields: []reflect.StructField{subField, subField}})
 	assert.False(t, v.IsValid())
 }
+
+func TestSubcommandInvalidInternal(t *testing.T) {
+	// this situation should never arise in practice but still good to test for it
+	var cmd struct{}
+	p, err := NewParser(Config{}, &cmd)
+	require.NoError(t, err)
+
+	p.subcommand = []string{"should", "never", "happen"}
+	sub := p.Subcommand()
+	assert.Nil(t, sub)
+}
diff --git a/usage.go b/usage.go
index a9f9844..0c68ee7 100644
--- a/usage.go
+++ b/usage.go
@@ -11,7 +11,7 @@ const colWidth = 25
 
 // Fail prints usage information to stderr and exits with non-zero status
 func (p *Parser) Fail(msg string) {
-	p.failWithSubcommand(msg, p.cmd)
+	p.FailSubcommand(msg)
 }
 
 // FailSubcommand prints usage information for a specified subcommand to stderr,
@@ -21,28 +21,19 @@ func (p *Parser) Fail(msg string) {
 // a sequence of subcommand names starting with the top-level subcommand and so
 // on down the tree.
 func (p *Parser) FailSubcommand(msg string, subcommand ...string) error {
-	cmd, err := p.lookupCommand(subcommand...)
+	err := p.WriteUsageForSubcommand(p.config.Out, subcommand...)
 	if err != nil {
 		return err
 	}
-	p.failWithSubcommand(msg, cmd)
-	return nil
-}
 
-// failWithSubcommand prints usage information for the given subcommand to stderr and exits with non-zero status
-func (p *Parser) failWithSubcommand(msg string, cmd *command) {
-	p.writeUsageForSubcommand(p.config.Out, cmd)
 	fmt.Fprintln(p.config.Out, "error:", msg)
 	p.config.Exit(-1)
+	return nil
 }
 
 // WriteUsage writes usage information to the given writer
 func (p *Parser) WriteUsage(w io.Writer) {
-	cmd := p.cmd
-	if p.lastCmd != nil {
-		cmd = p.lastCmd
-	}
-	p.writeUsageForSubcommand(w, cmd)
+	p.WriteUsageForSubcommand(w, p.subcommand...)
 }
 
 // WriteUsageForSubcommand writes the usage information for a specified
@@ -55,12 +46,7 @@ func (p *Parser) WriteUsageForSubcommand(w io.Writer, subcommand ...string) erro
 	if err != nil {
 		return err
 	}
-	p.writeUsageForSubcommand(w, cmd)
-	return nil
-}
 
-// writeUsageForSubcommand writes usage information for the given subcommand
-func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) {
 	var positionals, longOptions, shortOptions []*spec
 	for _, spec := range cmd.specs {
 		switch {
@@ -77,18 +63,10 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) {
 		fmt.Fprintln(w, p.version)
 	}
 
-	// make a list of ancestor commands so that we print with full context
-	var ancestors []string
-	ancestor := cmd
-	for ancestor != nil {
-		ancestors = append(ancestors, ancestor.name)
-		ancestor = ancestor.parent
-	}
-
 	// print the beginning of the usage string
-	fmt.Fprint(w, "Usage:")
-	for i := len(ancestors) - 1; i >= 0; i-- {
-		fmt.Fprint(w, " "+ancestors[i])
+	fmt.Fprintf(w, "Usage: %s", p.cmd.name)
+	for _, s := range subcommand {
+		fmt.Fprint(w, " "+s)
 	}
 
 	// write the option component of the usage message
@@ -149,47 +127,66 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) {
 	}
 
 	fmt.Fprint(w, "\n")
+	return nil
 }
 
-func printTwoCols(w io.Writer, left, help string, defaultVal string, envVal string) {
-	lhs := "  " + left
+// print prints a line like this:
+//
+//	--option FOO            A description of the option [default: 123]
+//
+// If the text on the left is longer than a certain threshold, the description is moved to the next line:
+//
+//	--verylongoptionoption VERY_LONG_VARIABLE
+//	                        A description of the option [default: 123]
+//
+// If multiple "extras" are provided then they are put inside a single set of square brackets:
+//
+//	--option FOO            A description of the option [default: 123, env: FOO]
+func print(w io.Writer, item, description string, bracketed ...string) {
+	lhs := "  " + item
 	fmt.Fprint(w, lhs)
-	if help != "" {
+	if description != "" {
 		if len(lhs)+2 < colWidth {
 			fmt.Fprint(w, strings.Repeat(" ", colWidth-len(lhs)))
 		} else {
 			fmt.Fprint(w, "\n"+strings.Repeat(" ", colWidth))
 		}
-		fmt.Fprint(w, help)
+		fmt.Fprint(w, description)
 	}
 
-	bracketsContent := []string{}
+	var brack string
+	for _, s := range bracketed {
+		if s != "" {
+			if brack != "" {
+				brack += ", "
+			}
+			brack += s
+		}
+	}
 
-	if defaultVal != "" {
-		bracketsContent = append(bracketsContent,
-			fmt.Sprintf("default: %s", defaultVal),
-		)
+	if brack != "" {
+		fmt.Fprintf(w, " [%s]", brack)
 	}
+	fmt.Fprint(w, "\n")
+}
 
-	if envVal != "" {
-		bracketsContent = append(bracketsContent,
-			fmt.Sprintf("env: %s", envVal),
-		)
+func withDefault(s string) string {
+	if s == "" {
+		return ""
 	}
+	return "default: " + s
+}
 
-	if len(bracketsContent) > 0 {
-		fmt.Fprintf(w, " [%s]", strings.Join(bracketsContent, ", "))
+func withEnv(env string) string {
+	if env == "" {
+		return ""
 	}
-	fmt.Fprint(w, "\n")
+	return "env: " + env
 }
 
 // WriteHelp writes the usage string followed by the full help string for each option
 func (p *Parser) WriteHelp(w io.Writer) {
-	cmd := p.cmd
-	if p.lastCmd != nil {
-		cmd = p.lastCmd
-	}
-	p.writeHelpForSubcommand(w, cmd)
+	p.WriteHelpForSubcommand(w, p.subcommand...)
 }
 
 // WriteHelpForSubcommand writes the usage string followed by the full help
@@ -202,12 +199,7 @@ func (p *Parser) WriteHelpForSubcommand(w io.Writer, subcommand ...string) error
 	if err != nil {
 		return err
 	}
-	p.writeHelpForSubcommand(w, cmd)
-	return nil
-}
 
-// writeHelp writes the usage string for the given subcommand
-func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 	var positionals, longOptions, shortOptions, envOnlyOptions []*spec
 	var hasVersionOption bool
 	for _, spec := range cmd.specs {
@@ -226,13 +218,13 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 	if p.description != "" {
 		fmt.Fprintln(w, p.description)
 	}
-	p.writeUsageForSubcommand(w, cmd)
+	p.WriteUsageForSubcommand(w, subcommand...)
 
 	// write the list of positionals
 	if len(positionals) > 0 {
 		fmt.Fprint(w, "\nPositional arguments:\n")
 		for _, spec := range positionals {
-			printTwoCols(w, spec.placeholder, spec.help, "", "")
+			print(w, spec.placeholder, spec.help)
 		}
 	}
 
@@ -296,13 +288,15 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) {
 	if len(cmd.subcommands) > 0 {
 		fmt.Fprint(w, "\nCommands:\n")
 		for _, subcmd := range cmd.subcommands {
-			printTwoCols(w, subcmd.name, subcmd.help, "", "")
+			names := append([]string{subcmd.name}, subcmd.aliases...)
+			print(w, strings.Join(names, ", "), subcmd.help)
 		}
 	}
 
 	if p.epilogue != "" {
 		fmt.Fprintln(w, "\n"+p.epilogue)
 	}
+	return nil
 }
 
 func (p *Parser) printOption(w io.Writer, spec *spec) {
@@ -314,7 +308,7 @@ func (p *Parser) printOption(w io.Writer, spec *spec) {
 		ways = append(ways, synopsis(spec, "-"+spec.short))
 	}
 	if len(ways) > 0 {
-		printTwoCols(w, strings.Join(ways, ", "), spec.help, spec.defaultString, spec.env)
+		print(w, strings.Join(ways, ", "), spec.help, withDefault(spec.defaultString), withEnv(spec.env))
 	}
 }
 
@@ -330,29 +324,7 @@ func (p *Parser) printEnvOnlyVar(w io.Writer, spec *spec) {
 		ways = append(ways, spec.help)
 	}
 
-	printTwoCols(w, spec.env, strings.Join(ways, " "), spec.defaultString, "")
-}
-
-// lookupCommand finds a subcommand based on a sequence of subcommand names. The
-// first string should be a top-level subcommand, the next should be a child
-// subcommand of that subcommand, and so on. If no strings are given then the
-// root command is returned. If no such subcommand exists then an error is
-// returned.
-func (p *Parser) lookupCommand(path ...string) (*command, error) {
-	cmd := p.cmd
-	for _, name := range path {
-		var found *command
-		for _, child := range cmd.subcommands {
-			if child.name == name {
-				found = child
-			}
-		}
-		if found == nil {
-			return nil, fmt.Errorf("%q is not a subcommand of %s", name, cmd.name)
-		}
-		cmd = found
-	}
-	return cmd, nil
+	print(w, spec.env, strings.Join(ways, " "), withDefault(spec.defaultString))
 }
 
 func synopsis(spec *spec, form string) string {
diff --git a/usage_test.go b/usage_test.go
index 1a64ad4..b1693a9 100644
--- a/usage_test.go
+++ b/usage_test.go
@@ -450,6 +450,8 @@ Global options:
 
 	_ = p.Parse([]string{"child", "nested", "value"})
 
+	assert.Equal(t, []string{"child", "nested"}, p.SubcommandNames())
+
 	var help bytes.Buffer
 	p.WriteHelp(&help)
 	assert.Equal(t, expectedHelp[1:], help.String())
@@ -471,7 +473,7 @@ func TestNonexistentSubcommand(t *testing.T) {
 	var args struct {
 		sub *struct{} `arg:"subcommand"`
 	}
-	p, err := NewParser(Config{}, &args)
+	p, err := NewParser(Config{Exit: func(int) {}}, &args)
 	require.NoError(t, err)
 
 	var b bytes.Buffer
@@ -687,3 +689,29 @@ Options:
 	p.WriteHelp(&help)
 	assert.Equal(t, expectedHelp[1:], help.String())
 }
+
+func TestHelpShowsSubcommandAliases(t *testing.T) {
+	expectedHelp := `
+Usage: example <command> [<args>]
+
+Options:
+  --help, -h             display this help and exit
+
+Commands:
+  remove, rm, r          remove something from somewhere
+  simple                 do something simple
+  halt, stop             stop now
+`
+
+	var args struct {
+		Remove *struct{} `arg:"subcommand:remove|rm|r" help:"remove something from somewhere"`
+		Simple *struct{} `arg:"subcommand" help:"do something simple"`
+		Stop   *struct{} `arg:"subcommand:halt|stop" help:"stop now"`
+	}
+	p, err := NewParser(Config{Program: "example"}, &args)
+	require.NoError(t, err)
+
+	var help bytes.Buffer
+	p.WriteHelp(&help)
+	assert.Equal(t, expectedHelp[1:], help.String())
+}