Replace Arg with RequiredArg and OptionalArg and update help te…

…xt (#121)

* Replace Arg with RequiredArg and OptionalArg

* Include required in help text

* Update demo gifs/images

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
 }

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 {

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
 		})
 	}
 }

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"),

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 = "<required>"
+
 // 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,

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:

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: