Skip to content

Commit

Permalink
Include required in help text
Browse files Browse the repository at this point in the history
  • Loading branch information
FollowTheProcess committed Jan 4, 2025
1 parent d017e73 commit 5491b34
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 15 deletions.
13 changes: 8 additions & 5 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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("[")
Expand All @@ -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 {
Expand Down
54 changes: 50 additions & 4 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,32 @@ 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{
Expand Down Expand Up @@ -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
}{
{
Expand All @@ -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",
},
Expand Down Expand Up @@ -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
})
}
}
Expand Down
25 changes: 21 additions & 4 deletions option.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -441,11 +447,17 @@ func Flag[T Flaggable](p *T, name string, short rune, value T, usage string) Opt
//
// Arguments added to the command may be retrieved by name from within command logic with [Command.Arg].
func RequiredArg(name, description string) Option {
// TODO(@FollowTheProcess): Validation
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,
name: name,
description: description,
defaultValue: requiredArgMarker, // Internal marker
}
cfg.positionalArgs = append(cfg.positionalArgs, arg)
return nil
Expand Down Expand Up @@ -477,8 +489,13 @@ func RequiredArg(name, description string) Option {
//
// Arguments added to the command may be retrieved by name from within command logic with [Command.Arg].
func OptionalArg(name, description, value string) Option {
// TODO(@FollowTheProcess): Validation, value cannot be ""
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,
Expand Down
2 changes: 1 addition & 1 deletion testdata/snapshots/TestHelp/with_named_arguments.snap.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion testdata/snapshots/TestHelp/with_verbosity_count.snap.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 5491b34

Please sign in to comment.