From 2dbe16f1dbee500245ed365cc279821c615751af Mon Sep 17 00:00:00 2001 From: Olivier Cano Date: Wed, 11 Mar 2020 13:13:34 +0100 Subject: [PATCH] feat(core): positional argument (#759) --- internal/core/arg_specs.go | 20 ++++ internal/core/cobra_builder.go | 3 + internal/core/cobra_usage_builder.go | 2 +- internal/core/cobra_utils.go | 64 +++++++++++ internal/core/cobra_utils_test.go | 104 +++++++++++++++++- ...sitional-arg-full-command#01.stderr.golden | 14 +++ 6 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 internal/core/testdata/test-positional-arg-full-command#01.stderr.golden diff --git a/internal/core/arg_specs.go b/internal/core/arg_specs.go index 7c2c8fe10c..1160137700 100644 --- a/internal/core/arg_specs.go +++ b/internal/core/arg_specs.go @@ -8,6 +8,19 @@ import ( type ArgSpecs []*ArgSpec +func (s ArgSpecs) GetPositionalArg() *ArgSpec { + var positionalArg *ArgSpec + for _, argSpec := range s { + if argSpec.Positional { + if positionalArg != nil { + panic(fmt.Errorf("more than one positional parameter detected: %s and %s are flagged as positional arg", positionalArg.Name, argSpec.Name)) + } + positionalArg = argSpec + } + } + return positionalArg +} + func (s ArgSpecs) GetByName(name string) *ArgSpec { for _, spec := range s { if spec.Name == name { @@ -62,6 +75,13 @@ type ArgSpec struct { // ValidateFunc validates an argument. ValidateFunc ArgSpecValidateFunc + + // Positional defines whether the argument is a positional argument. NB: a positional argument is required. + Positional bool +} + +func (a *ArgSpec) Prefix() string { + return a.Name + "=" } type DefaultFunc func() (value string, doc string) diff --git a/internal/core/cobra_builder.go b/internal/core/cobra_builder.go index 15dc13c9ed..67a7f1f4f9 100644 --- a/internal/core/cobra_builder.go +++ b/internal/core/cobra_builder.go @@ -127,6 +127,9 @@ func (b *cobraBuilder) hydrateCobra(cobraCmd *cobra.Command, cmd *Command) { if cobraCmd.HasAvailableSubCommands() || len(cobraCmd.Commands()) > 0 { cobraCmd.Annotations["CommandUsage"] += " " } + if positionalArg := cmd.ArgSpecs.GetPositionalArg(); positionalArg != nil { + cobraCmd.Annotations["CommandUsage"] += " <" + positionalArg.Name + ">" + } if cobraCmd.HasAvailableLocalFlags() || cobraCmd.HasAvailableFlags() || cobraCmd.LocalFlags() != nil { cobraCmd.Annotations["CommandUsage"] += " [flags]" } diff --git a/internal/core/cobra_usage_builder.go b/internal/core/cobra_usage_builder.go index fba3ad9bd0..5db19de8fe 100644 --- a/internal/core/cobra_usage_builder.go +++ b/internal/core/cobra_usage_builder.go @@ -59,7 +59,7 @@ func _buildUsageArgs(w io.Writer, argSpecs ArgSpecs) error { _, doc := argSpec.Default() argSpecUsageLeftPart = fmt.Sprintf("%s=%s", argSpecUsageLeftPart, doc) } - if !argSpec.Required { + if !argSpec.Required && !argSpec.Positional { argSpecUsageLeftPart = fmt.Sprintf("[%s]", argSpecUsageLeftPart) } diff --git a/internal/core/cobra_utils.go b/internal/core/cobra_utils.go index 971ab0a07e..3a00416f04 100644 --- a/internal/core/cobra_utils.go +++ b/internal/core/cobra_utils.go @@ -25,6 +25,11 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string) // unmarshalled arguments will be store in this interface cmdArgs := reflect.New(cmd.ArgsType).Interface() + // Handle positional argument by catching first argument `` and rewrite it to `=`. + if err = handlePositionalArg(cmd, rawArgs); err != nil { + return err + } + // Apply default values on missing args. rawArgs = ApplyDefaultValues(cmd.ArgSpecs, rawArgs) @@ -78,6 +83,65 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string) } } +// handlePositionalArg will catch positional argument if command has one. +// When a positional argument is found it will mutate its value in rawArgs to match the argument unmarshaller format. +// E.g.: '[value b=true c=1]' will be mutated to '[a=value b=true c=1]'. +// It returns errors when: +// - no positional argument is found. +// - an unknown positional argument exists in the comand. +// - an argument duplicates a positional argument. +func handlePositionalArg(cmd *Command, rawArgs []string) error { + positionalArg := cmd.ArgSpecs.GetPositionalArg() + + // Command does not have a positional argument. + if positionalArg == nil { + return nil + } + + // Positional argument is found condition. + positionalArgumentFound := len(rawArgs) > 0 && !strings.Contains(rawArgs[0], "=") + + // Argument exists but is not positional. + for i, arg := range rawArgs { + if strings.HasPrefix(arg, positionalArg.Prefix()) { + argumentValue := strings.TrimLeft(arg, positionalArg.Prefix()) + otherArgs := append(rawArgs[:i], rawArgs[i+1:]...) + return &CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: positionalArgHint(cmd, argumentValue, otherArgs, positionalArgumentFound), + } + } + } + + // If positional argument is found, prefix it with `arg-name=`. + if positionalArgumentFound { + rawArgs[0] = positionalArg.Prefix() + rawArgs[0] + return nil + } + + // No positional argument found. + return &CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: positionalArgHint(cmd, "<"+positionalArg.Name+">", rawArgs, false), + } +} + +// positionalArgHint formats the positional argument error hint. +func positionalArgHint(cmd *Command, hintValue string, otherArgs []string, positionalArgumentFound bool) string { + suggestedArgs := []string{} + + // If no positional argument exists, suggest one. + if !positionalArgumentFound { + suggestedArgs = append(suggestedArgs, hintValue) + } + + // Suggest to use the other arguments. + suggestedArgs = append(suggestedArgs, otherArgs...) + + suggestedCommand := append([]string{"scw", cmd.getPath()}, suggestedArgs...) + return "Try running: " + strings.Join(suggestedCommand, " ") +} + func handleUnmarshalErrors(cmd *Command, unmarshalErr *args.UnmarshalArgError) error { wrappedErr := errors.Unwrap(unmarshalErr) diff --git a/internal/core/cobra_utils_test.go b/internal/core/cobra_utils_test.go index ebd80e53b8..2e54fb1048 100644 --- a/internal/core/cobra_utils_test.go +++ b/internal/core/cobra_utils_test.go @@ -8,7 +8,8 @@ import ( ) type testType struct { - Name string + NameID string + Tag string } func testGetCommands() *Commands { @@ -25,6 +26,22 @@ func testGetCommands() *Commands { return "", nil }, }, + &Command{ + Namespace: "test-positional", + ArgSpecs: ArgSpecs{ + { + Name: "name-id", + Positional: true, + }, + { + Name: "tag", + }, + }, + ArgsType: reflect.TypeOf(testType{}), + Run: func(ctx context.Context, argsI interface{}) (i interface{}, e error) { + return "", nil + }, + }, ) } @@ -53,3 +70,88 @@ func Test_handleUnmarshalErrors(t *testing.T) { ), })) } + +func Test_PositionalArg(t *testing.T) { + t.Run("Error", func(t *testing.T) { + t.Run("Missing1", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional", + Check: TestCheckCombine( + TestCheckExitCode(1), + TestCheckError(&CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: "Try running: scw test-positional ", + }), + ), + })) + + t.Run("Missing2", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional tag=world", + Check: TestCheckCombine( + TestCheckExitCode(1), + TestCheckError(&CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: "Try running: scw test-positional tag=world", + }), + ), + })) + + t.Run("Invalid1", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional name-id=plop tag=world", + Check: TestCheckCombine( + TestCheckExitCode(1), + TestCheckError(&CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: "Try running: scw test-positional plop tag=world", + }), + ), + })) + + t.Run("Invalid2", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional tag=world name-id=plop", + Check: TestCheckCombine( + TestCheckExitCode(1), + TestCheckError(&CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: fmt.Sprintf("Try running: scw test-positional plop tag=world"), + }), + ), + })) + + t.Run("Invalid3", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional plop name-id=plop", + Check: TestCheckCombine( + TestCheckExitCode(1), + TestCheckError(&CliError{ + Err: fmt.Errorf("a positional argument is required for this command"), + Hint: fmt.Sprintf("Try running: scw test-positional plop"), + }), + ), + })) + }) + + t.Run("simple", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional plop", + Check: TestCheckExitCode(0), + })) + + t.Run("full command", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional plop tag=world", + Check: TestCheckExitCode(0), + })) + + t.Run("full command", Test(&TestConfig{ + Commands: testGetCommands(), + Cmd: "scw test-positional -h", + Check: TestCheckCombine( + TestCheckExitCode(0), + TestCheckGolden(), + ), + })) +} diff --git a/internal/core/testdata/test-positional-arg-full-command#01.stderr.golden b/internal/core/testdata/test-positional-arg-full-command#01.stderr.golden new file mode 100644 index 0000000000..7e121af744 --- /dev/null +++ b/internal/core/testdata/test-positional-arg-full-command#01.stderr.golden @@ -0,0 +1,14 @@ +USAGE: + scw [global-flags] test-positional [flags] [arg=value ...] + +ARGS: + name-id + [tag] + +FLAGS: + -h, --help help for test-positional + +GLOBAL FLAGS: + -D, --debug Enable debug mode + -o, --output string Output format: json or human + -p, --profile string The config profile to use