Repository: alecthomas/kong
Pull request #188 (2021-07-11 11:27:59)

Allow default commands with cmds/args/flags
Allow default commands to have sub-commands, args and flags when tagged 
with `default:"withargs"`. This makes specifying the name of the command
on the CLI completely optional as long as the args to that command are not
ambiguous with other commands.

Re-work the validation of the CLI grammar so that the constraints on default
commands are checked during `build()` and not `trace()` - this seems to be
more correct and where other parts of the grammar are validated. Also add
some missing validation for default commands with positional args - only
branching args were disallowed, but the error message seemed to imply that
all args should be disallowed.

camh-: PTAL

Patch:
diff --git a/.golangci.yml b/.golangci.yml
index 0f7ad82..f7be904 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -35,15 +35,13 @@ linters:
     - tagliatelle
     - thelper
     - godox
+    - goconst
 
 linters-settings:
   govet:
     check-shadowing: true
   dupl:
     threshold: 100
-  goconst:
-    min-len: 5
-    min-occurrences: 3
   gocyclo:
     min-complexity: 20
   exhaustive:
diff --git a/README.md b/README.md
index 03996db..faae736 100644
--- a/README.md
+++ b/README.md
@@ -302,7 +302,7 @@ type CLI struct {
 }
 ```
 
-If a sub-command is tagged with `default:"1"` it will be selected if there are no further arguments.
+If a sub-command is tagged with `default:"1"` it will be selected if there are no further arguments. If a sub-command is tagged with `default:"withargs"` it will be selected even if there are further arguments or flags and those arguments or flags are valid for the sub-command. This allows the user to omit the sub-command name on the CLI if its arguments/flags are not ambiguous with the sibling commands or flags.
 
 ## Branching positional arguments
 
@@ -435,6 +435,7 @@ Tag                    | Description
 `placeholder:"X"`      | Placeholder text.
 `default:"X"`          | Default value.
 `default:"1"`          | On a command, make it the default.
+`default:"withargs"`   | On a command, make it the default and allow args/flags from that command
 `short:"X"`            | Short name, if flag.
 `aliases:"X,Y"`        | One or more aliases (for cmd).
 `required:""`          | If present, flag/arg is required.
diff --git a/build.go b/build.go
index 22f9839..35e9411 100644
--- a/build.go
+++ b/build.go
@@ -141,6 +141,7 @@ func buildNode(k *Kong, v reflect.Value, typ NodeType, seenFlags map[string]bool
 
 func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.StructField, fv reflect.Value, tag *Tag, name string, seenFlags map[string]bool) {
 	child := buildNode(k, fv, typ, seenFlags)
+	child.Name = name
 	child.Tag = tag
 	child.Parent = node
 	child.Help = tag.Help
@@ -158,21 +159,23 @@ func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.S
 		if len(child.Positional) == 0 {
 			failField(v, ft, "positional branch must have at least one child positional argument named %q", name)
 		}
+		if child.Positional[0].Name != name {
+			failField(v, ft, "first field in positional branch must have the same name as the parent field (%s).", child.Name)
+		}
 
-		value := child.Positional[0]
+		child.Argument = child.Positional[0]
 		child.Positional = child.Positional[1:]
 		if child.Help == "" {
-			child.Help = value.Help
+			child.Help = child.Argument.Help
 		}
-
-		child.Name = value.Name
-		if child.Name != name {
-			failField(v, ft, "first field in positional branch must have the same name as the parent field (%s).", child.Name)
+	} else if tag.Default != "" {
+		if node.DefaultCmd != nil {
+			failField(v, ft, "can't have more than one default command under %s", node.Summary())
 		}
-
-		child.Argument = value
-	} else {
-		child.Name = name
+		if tag.Default != "withargs" && (len(child.Children) > 0 || len(child.Positional) > 0) {
+			failField(v, ft, "default command %s must not have subcommands or arguments", child.Summary())
+		}
+		node.DefaultCmd = child
 	}
 	node.Children = append(node.Children, child)
 
diff --git a/context.go b/context.go
index ea91e5c..0080b16 100644
--- a/context.go
+++ b/context.go
@@ -343,7 +343,14 @@ func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
 	positional := 0
 
 	flags := []*Flag{}
-	for _, group := range node.AllFlags(false) {
+	flagNode := node
+	if node.DefaultCmd != nil && node.DefaultCmd.Tag.Default == "withargs" {
+		// Add flags of the default command if the current node has one
+		// and that default command allows args / flags without explicitly
+		// naming the command on the CLI.
+		flagNode = node.DefaultCmd
+	}
+	for _, group := range flagNode.AllFlags(false) {
 		flags = append(flags, group...)
 	}
 
@@ -483,6 +490,17 @@ func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
 				}
 			}
 
+			// If there is a default command that allows args and nothing else
+			// matches, take the branch of the default command
+			if node.DefaultCmd != nil && node.DefaultCmd.Tag.Default == "withargs" {
+				c.Path = append(c.Path, &Path{
+					Parent:  node,
+					Command: node.DefaultCmd,
+					Flags:   node.DefaultCmd.Flags,
+				})
+				return c.trace(node.DefaultCmd)
+			}
+
 			return findPotentialCandidates(token.String(), candidates, "unexpected argument %s", token)
 		default:
 			return fmt.Errorf("unexpected token %s", token)
@@ -499,21 +517,12 @@ func (c *Context) maybeSelectDefault(flags []*Flag, node *Node) error {
 			return nil
 		}
 	}
-	var defaultNode *Path
-	for _, child := range node.Children {
-		if child.Type == CommandNode && child.Tag.Default != "" {
-			if defaultNode != nil {
-				return fmt.Errorf("can't have more than one default command under %s", node.Summary())
-			}
-			defaultNode = &Path{
-				Parent:  child,
-				Command: child,
-				Flags:   child.Flags,
-			}
-		}
-	}
-	if defaultNode != nil {
-		c.Path = append(c.Path, defaultNode)
+	if node.DefaultCmd != nil {
+		c.Path = append(c.Path, &Path{
+			Parent:  node.DefaultCmd,
+			Command: node.DefaultCmd,
+			Flags:   node.DefaultCmd.Flags,
+		})
 	}
 	return nil
 }
@@ -794,7 +803,6 @@ func checkMissingChildren(node *Node) error {
 		missing = append(missing, strconv.Quote(strings.Join(missingArgs, " ")))
 	}
 
-	haveDefault := 0
 	for _, child := range node.Children {
 		if child.Hidden {
 			continue
@@ -805,19 +813,10 @@ func checkMissingChildren(node *Node) error {
 			}
 			missing = append(missing, strconv.Quote(child.Summary()))
 		} else {
-			if child.Tag.Default != "" {
-				if len(child.Children) > 0 {
-					return fmt.Errorf("default command %s must not have subcommands or arguments", child.Summary())
-				}
-				haveDefault++
-			}
 			missing = append(missing, strconv.Quote(child.Name))
 		}
 	}
-	if haveDefault > 1 {
-		return fmt.Errorf("more than one default command found under %s", node.Summary())
-	}
-	if len(missing) == 0 || haveDefault > 0 {
+	if len(missing) == 0 {
 		return nil
 	}
 
diff --git a/model.go b/model.go
index 450f61c..e0263d7 100644
--- a/model.go
+++ b/model.go
@@ -51,6 +51,7 @@ type Node struct {
 	Flags      []*Flag
 	Positional []*Positional
 	Children   []*Node
+	DefaultCmd *Node
 	Target     reflect.Value // Pointer to the value in the grammar that this Node is associated with.
 	Tag        *Tag
 	Aliases    []string


Test patch:
diff --git a/kong_test.go b/kong_test.go
index 8418355..33a2c52 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -1020,9 +1020,112 @@ func TestMultipleDefaultCommands(t *testing.T) {
 		One struct{} `cmd:"" default:"1"`
 		Two struct{} `cmd:"" default:"1"`
 	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.Two: can't have more than one default command under  <command>")
+}
+
+func TestDefaultCommandWithSubCommand(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct{} `cmd:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <command> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedSubCommand(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct{} `cmd:""`
+		} `cmd:"" default:"withargs"`
+	}
 	p := mustNew(t, &cli)
-	_, err := p.Parse([]string{})
-	require.EqualError(t, err, "can't have more than one default command under  <command>")
+	ctx, err := p.Parse([]string{"two"})
+	require.NoError(t, err)
+	require.Equal(t, "one two", ctx.Command())
+}
+
+func TestDefaultCommandWithArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Arg string `arg:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <arg> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Arg  string `arg:""`
+			Flag string
+		} `cmd:"" default:"withargs"`
+	}
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"arg", "--flag=value"})
+	require.NoError(t, err)
+	require.Equal(t, "arg", cli.One.Arg)
+	require.Equal(t, "value", cli.One.Flag)
+}
+
+func TestDefaultCommandWithBranchingArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct {
+				Two string `arg:""`
+			} `arg:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <command> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedBranchingArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct {
+				Two  string `arg:""`
+				Flag string
+			} `arg:""`
+		} `cmd:"" default:"withargs"`
+	}
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"arg", "--flag=value"})
+	require.NoError(t, err)
+	require.Equal(t, "arg", cli.One.Two.Two)
+	require.Equal(t, "value", cli.One.Two.Flag)
+}
+
+func TestDefaultCommandPrecedence(t *testing.T) {
+	var cli struct {
+		Two struct {
+			Arg  string `arg:""`
+			Flag bool
+		} `cmd:"" default:"withargs"`
+		One struct{} `cmd:""`
+	}
+	p := mustNew(t, &cli)
+
+	// A named command should take precedence over a default command with arg
+	ctx, err := p.Parse([]string{"one"})
+	require.NoError(t, err)
+	require.Equal(t, "one", ctx.Command())
+
+	// An explicitly named command with arg should parse, even if labeled default:"witharg"
+	ctx, err = p.Parse([]string{"two", "arg"})
+	require.NoError(t, err)
+	require.Equal(t, "two <arg>", ctx.Command())
+
+	// An arg to a default command that does not match another command should select the default
+	ctx, err = p.Parse([]string{"arg"})
+	require.NoError(t, err)
+	require.Equal(t, "two <arg>", ctx.Command())
+
+	// A flag on a default command should not be valid on a sibling command
+	_, err = p.Parse([]string{"one", "--flag"})
+	require.EqualError(t, err, "unknown flag --flag")
 }
 
 func TestLoneHpyhen(t *testing.T) {