diff --git a/args.go b/args.go index 1a10b26..a2a756d 100644 --- a/args.go +++ b/args.go @@ -148,5 +148,5 @@ type positionalArg struct { name string // The name of the argument description string // A short description of the argument value string // The actual parsed value from the command line - defaultValue string // The default value to be used if not set, a default of "" marks the arg as required + defaultValue string // The default value to be used if not set, only set by the OptionalArg option } diff --git a/command.go b/command.go index 89b883a..6bbb53a 100644 --- a/command.go +++ b/command.go @@ -260,7 +260,7 @@ func (c *Command) Execute() error { // If we've fallen off the end of argsWithoutFlags and the positionalArg at this // index does not have a default, it means the arg was required but not provided - if arg.defaultValue == "" { + if arg.defaultValue == requiredArgMarker { return fmt.Errorf("missing required argument %q, expected at position %d", arg.name, i) } // It does have a default, so use that instead @@ -574,7 +574,7 @@ func defaultHelp(cmd *Command) error { func writePositionalArgs(cmd *Command, s *strings.Builder) { for _, arg := range cmd.positionalArgs { displayName := strings.ToUpper(arg.name) - if arg.defaultValue != "" { + if arg.defaultValue != requiredArgMarker { // If it has a default, it's an optional argument so wrap it // in brackets e.g. [FILE] s.WriteString("[") @@ -596,10 +596,13 @@ func writeArgumentsSection(cmd *Command, s *strings.Builder) error { s.WriteString("\n") tab := table.New(s) for _, arg := range cmd.positionalArgs { - if arg.defaultValue != "" { - tab.Row(" %s\t%s [default %s]\n", colour.Bold(arg.name), arg.description, arg.defaultValue) - } else { + switch arg.defaultValue { + case requiredArgMarker: + tab.Row(" %s\t%s [required]\n", colour.Bold(arg.name), arg.description) + case "": tab.Row(" %s\t%s\n", colour.Bold(arg.name), arg.description) + default: + tab.Row(" %s\t%s [default %s]\n", colour.Bold(arg.name), arg.description, arg.defaultValue) } } if err := tab.Flush(); err != nil { diff --git a/command_test.go b/command_test.go index bd164f4..c54e92c 100644 --- a/command_test.go +++ b/command_test.go @@ -232,7 +232,7 @@ func TestPositionalArgs(t *testing.T) { return cli.New( "sub", cli.Short("Sub command"), - cli.Arg("subarg", "Argument given to a subcommand", ""), + cli.RequiredArg("subarg", "Argument given to a subcommand"), cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "Hello from sub command, subarg: %s", cmd.Arg("subarg")) return nil @@ -251,7 +251,7 @@ func TestPositionalArgs(t *testing.T) { { name: "required and given", options: []cli.Option{ - cli.Arg("file", "The path to a file", ""), // "" means required + cli.RequiredArg("file", "The path to a file"), cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) return nil @@ -264,7 +264,7 @@ func TestPositionalArgs(t *testing.T) { { name: "required but missing", options: []cli.Option{ - cli.Arg("file", "The path to a file", ""), // "" means required + cli.RequiredArg("file", "The path to a file"), cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) return nil @@ -278,7 +278,7 @@ func TestPositionalArgs(t *testing.T) { { name: "optional and given", options: []cli.Option{ - cli.Arg("file", "The path to a file", "default.txt"), // This time it has a default + cli.OptionalArg("file", "The path to a file", "default.txt"), // This time it has a default cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) return nil @@ -288,10 +288,36 @@ func TestPositionalArgs(t *testing.T) { args: []string{"something.txt"}, wantErr: false, }, + { + name: "optional given with empty string default", + options: []cli.Option{ + cli.OptionalArg("file", "The path to a file", ""), // Default is empty string + cli.Run(func(cmd *cli.Command, args []string) error { + fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) + return nil + }), + }, + stdout: "file was something.txt\n", + args: []string{"something.txt"}, + wantErr: false, + }, + { + name: "optional missing with empty string default", + options: []cli.Option{ + cli.OptionalArg("file", "The path to a file", ""), // Default is empty string + cli.Run(func(cmd *cli.Command, args []string) error { + fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) + return nil + }), + }, + stdout: "file was \n", // Empty string + args: []string{}, + wantErr: false, + }, { name: "optional and missing", options: []cli.Option{ - cli.Arg("file", "The path to a file", "default.txt"), // This time it has a default + cli.OptionalArg("file", "The path to a file", "default.txt"), // This time it has a default cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "file was %s\n", cmd.Arg("file")) return nil @@ -304,9 +330,9 @@ func TestPositionalArgs(t *testing.T) { { name: "several args all given", options: []cli.Option{ - cli.Arg("src", "The path to the source file", ""), // File required as first arg - cli.Arg("dest", "The destination path", "dest.txt"), // Dest has a default - cli.Arg("something", "Another arg", ""), // Required again + cli.RequiredArg("src", "The path to the source file"), // File required as first arg + cli.OptionalArg("dest", "The destination path", "dest.txt"), // Dest has a default + cli.RequiredArg("something", "Another arg"), // Required again cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "src: %s, dest: %s, something: %s\n", cmd.Arg("src"), cmd.Arg("dest"), cmd.Arg("something")) return nil @@ -319,9 +345,9 @@ func TestPositionalArgs(t *testing.T) { { name: "several args one missing", options: []cli.Option{ - cli.Arg("src", "The path to the source file", ""), // File required as first arg - cli.Arg("dest", "The destination path", "default-dest.txt"), // Dest has a default - cli.Arg("something", "Another arg", ""), // Required again + cli.RequiredArg("src", "The path to the source file"), // File required as first arg + cli.OptionalArg("dest", "The destination path", "default-dest.txt"), // Dest has a default + cli.RequiredArg("something", "Another arg"), // Required again cli.Run(func(cmd *cli.Command, args []string) error { fmt.Fprintf(cmd.Stdout(), "src: %s, dest: %s, something: %s\n", cmd.Arg("src"), cmd.Arg("dest"), cmd.Arg("something")) return nil @@ -436,8 +462,8 @@ func TestHelp(t *testing.T) { name: "with named arguments", options: []cli.Option{ cli.OverrideArgs([]string{"--help"}), - cli.Arg("src", "The file to copy", ""), // This one is required - cli.Arg("dest", "Destination to copy to", "./dest"), // This one is optional + cli.RequiredArg("src", "The file to copy"), // This one is required + cli.OptionalArg("dest", "Destination to copy to", "./dest"), // This one is optional cli.Run(func(cmd *cli.Command, args []string) error { return nil }), }, wantErr: false, @@ -446,8 +472,8 @@ func TestHelp(t *testing.T) { name: "with verbosity count", options: []cli.Option{ cli.OverrideArgs([]string{"--help"}), - cli.Arg("src", "The file to copy", ""), // This one is required - cli.Arg("dest", "Destination to copy to", "./dest"), // This one is optional + cli.RequiredArg("src", "The file to copy"), // This one is required + cli.OptionalArg("dest", "Destination to copy to", "./dest"), // This one is optional cli.Flag(new(cli.FlagCount), "verbosity", 'v', 0, "Increase the verbosity level"), cli.Run(func(cmd *cli.Command, args []string) error { return nil }), }, @@ -682,7 +708,7 @@ func TestVersion(t *testing.T) { func TestOptionValidation(t *testing.T) { tests := []struct { name string // Name of the test case - errMsg string // If we wanted an error, what should it say + errMsg string // Expected error message options []cli.Option // Options to apply to the command }{ { @@ -706,7 +732,7 @@ func TestOptionValidation(t *testing.T) { errMsg: "cannot set Stdout to nil\ncannot set Stderr to nil\ncannot set Stdin to nil", }, { - name: "nil args", + name: "nil override args", options: []cli.Option{cli.OverrideArgs(nil)}, errMsg: "cannot set Args to nil", }, @@ -761,13 +787,33 @@ func TestOptionValidation(t *testing.T) { options: []cli.Option{cli.Long("")}, errMsg: "cannot set command long description to an empty string", }, + { + name: "empty required arg name", + options: []cli.Option{cli.RequiredArg("", "empty required arg")}, + errMsg: "invalid name for positional argument, must be non-empty string", + }, + { + name: "empty required arg description", + options: []cli.Option{cli.RequiredArg("name", "")}, + errMsg: "invalid description for positional argument, must be non-empty string", + }, + { + name: "empty optional arg name", + options: []cli.Option{cli.OptionalArg("", "empty required arg", "")}, + errMsg: "invalid name for positional argument, must be non-empty string", + }, + { + name: "empty optional arg description", + options: []cli.Option{cli.OptionalArg("name", "", "")}, + errMsg: "invalid description for positional argument, must be non-empty string", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { _, err := cli.New("test", tt.options...) - test.Err(t, err) - test.Equal(t, err.Error(), tt.errMsg) + test.Err(t, err) // Invalid option should have triggered an error + test.Equal(t, err.Error(), tt.errMsg) // Error message was not as expected }) } } diff --git a/examples/namedargs/main.go b/examples/namedargs/main.go index b9893db..6ead629 100644 --- a/examples/namedargs/main.go +++ b/examples/namedargs/main.go @@ -18,8 +18,8 @@ func run() error { cmd, err := cli.New( "copy", // A fictional copy command cli.Short("Copy a file from a src to a destination"), - cli.Arg("src", "The file to copy from", ""), // "" as default value means required - cli.Arg("dest", "The destination to copy to", "./dest"), // dest has a default if not provided + cli.RequiredArg("src", "The file to copy from"), // src is required, failure to provide it will error + cli.OptionalArg("dest", "The destination to copy to", "./dest"), // dest has a default if not provided cli.Stdout(os.Stdout), cli.Example("Copy a file to somewhere", "copy src.txt ./some/where/else"), cli.Example("Use the default destination", "copy src.txt"), diff --git a/option.go b/option.go index 92c465c..56dcd54 100644 --- a/option.go +++ b/option.go @@ -44,6 +44,12 @@ func (o option) apply(cfg *config) error { return o(cfg) } +// requiredArgMarker is a special string designed to be used as the default value for +// a required positional argument. This is so we know the argument was required, but still +// permits the use of the empty string "" as a default. Without this marker, omitting the default +// value or setting it to the string zero value would accidentally mark it as required. +const requiredArgMarker = "" + // config represents the internal configuration of a [Command]. type config struct { stdin io.Reader @@ -418,29 +424,78 @@ func Flag[T Flaggable](p *T, name string, short rune, value T, usage string) Opt return option(f) } -// Arg is an [Option] that adds a named positional argument to a [Command]. +// RequiredArg is an [Option] that adds a required named positional argument to a [Command]. +// +// A required named argument is given a name, and a description that will be shown in +// the help text. Failure to provide this argument on the command line when the command is +// invoked will result in an error from [Command.Execute]. +// +// The order of calls matters, each call to RequiredArg effectively appends a required, named +// positional argument to the command so the following: +// +// cli.New( +// "cp", +// cli.RequiredArg("src", "The file to copy"), +// cli.RequiredArg("dest", "Where to copy to"), +// ) // -// A named argument is given a name, description and a default value, a default of "" -// marks the argument as required. +// results in a command that will expect the following args *in order* +// +// cp src.txt dest.txt +// +// If the argument should have a default value if not specified on the command line, use [OptionalArg]. +// +// Arguments added to the command may be retrieved by name from within command logic with [Command.Arg]. +func RequiredArg(name, description string) Option { + f := func(cfg *config) error { + if name == "" { + return errors.New("invalid name for positional argument, must be non-empty string") + } + if description == "" { + return errors.New("invalid description for positional argument, must be non-empty string") + } + arg := positionalArg{ + name: name, + description: description, + defaultValue: requiredArgMarker, // Internal marker + } + cfg.positionalArgs = append(cfg.positionalArgs, arg) + return nil + } + + return option(f) +} + +// OptionalArg is an [Option] that adds a named positional argument, with a default value, to a [Command]. // -// If an argument without a default is provided on the command line, the provided value is used, if -// it doesn't have a default and the value isn't given on the command line, [Command.Execute] will -// return an informative error saying which one is missing. +// An optional named argument is given a name, a description, and a default value that will be shown in +// the help text. If the argument isn't given when the command is invoke, the default value is used +// in it's place. // -// This is the only [Option] for which the order of calls matter, each call to Arg effectively appends a -// named positional argument so the following: +// The order of calls matters, each call to OptionalArg effectively appends an optional, named +// positional argument to the command so the following: // -// cli.New("cp", cli.Arg("src", "The file to copy", ""), cli.Arg("dest", "Where to copy to", "")) +// cli.New( +// "cp", +// cli.OptionalArg("src", "The file to copy", "./default-src.txt"), +// cli.OptionalArg("dest", "Where to copy to", "./default-dest.txt"), +// ) // // results in a command that will expect the following args *in order* // // cp src.txt dest.txt // -// Arguments added to the command with Arg may be retrieved by name from within command logic with [Command.Arg]. -func Arg(name, description, value string) Option { - // TODO(@FollowTheProcess): Not entirely happy with the "" meaning required - // I wonder if we should have Arg and ArgWithDefault? +// If the argument should be required (e.g. no sensible default), use [RequiredArg]. +// +// Arguments added to the command may be retrieved by name from within command logic with [Command.Arg]. +func OptionalArg(name, description, value string) Option { f := func(cfg *config) error { + if name == "" { + return errors.New("invalid name for positional argument, must be non-empty string") + } + if description == "" { + return errors.New("invalid description for positional argument, must be non-empty string") + } arg := positionalArg{ name: name, description: description, diff --git a/testdata/snapshots/TestHelp/with_named_arguments.snap.txt b/testdata/snapshots/TestHelp/with_named_arguments.snap.txt index 65a6578..2642bea 100644 --- a/testdata/snapshots/TestHelp/with_named_arguments.snap.txt +++ b/testdata/snapshots/TestHelp/with_named_arguments.snap.txt @@ -3,7 +3,7 @@ A placeholder for something cool Usage: test [OPTIONS] SRC [DEST] Arguments: - src The file to copy + src The file to copy [required] dest Destination to copy to [default ./dest] Options: diff --git a/testdata/snapshots/TestHelp/with_verbosity_count.snap.txt b/testdata/snapshots/TestHelp/with_verbosity_count.snap.txt index 193adfd..969ae9b 100644 --- a/testdata/snapshots/TestHelp/with_verbosity_count.snap.txt +++ b/testdata/snapshots/TestHelp/with_verbosity_count.snap.txt @@ -3,7 +3,7 @@ A placeholder for something cool Usage: test [OPTIONS] SRC [DEST] Arguments: - src The file to copy + src The file to copy [required] dest Destination to copy to [default ./dest] Options: