Skip to content

feat(core): support positional args as dynamic args as well #4621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions core/arg_specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ var AllLocalities = "all"

type ArgSpecs []*ArgSpec

// GetPositionalArg returns the last positional argument from the arg specs.
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
}
}
Expand Down
20 changes: 6 additions & 14 deletions core/cobra_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)

positionalArgSpec := cmd.ArgSpecs.GetPositionalArg()

// If this command has no positional argument we execute the run
if positionalArgSpec == nil {
// If this command has no positional argument or the positional arg is already passed, we execute the run
if positionalArgSpec == nil || rawArgs.Has(positionalArgSpec.Name) {
if positionalArgSpec != nil && rawArgs.Has(positionalArgSpec.Name) {
rawArgs = rawArgs.RemoveAllPositional()
}
data, err := run(ctx, cobraCmd, cmd, rawArgs)
if err != nil {
return err
Expand All @@ -55,22 +58,11 @@ func cobraRun(ctx context.Context, cmd *Command) func(*cobra.Command, []string)

positionalArgs := rawArgs.GetPositionalArgs()

// Return an error if a positional argument was provide using `key=value` syntax.
value, exist := rawArgs.Get(positionalArgSpec.Name)
if exist {
otherArgs := rawArgs.Remove(positionalArgSpec.Name)

return &CliError{
Err: errors.New("a positional argument is required for this command"),
Hint: positionalArgHint(meta.BinaryName, cmd, value, otherArgs, len(positionalArgs) > 0),
}
}

// If no positional arguments were provided, return an error
if len(positionalArgs) == 0 {
return &CliError{
Err: errors.New("a positional argument is required for this command"),
Hint: positionalArgHint(meta.BinaryName, cmd, "<"+positionalArgSpec.Name+">", rawArgs, false),
Hint: positionalArgHint(meta.BinaryName, cmd, "<"+positionalArgSpec.Name+">", rawArgs.Remove(positionalArgSpec.Name), false),
}
}

Expand Down
133 changes: 97 additions & 36 deletions core/cobra_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ type testType struct {
Tag string
}

type testTypeManyTags struct {
NameID string
Tags []string
}

type testDate struct {
Date *time.Time
}
Expand Down Expand Up @@ -78,6 +83,45 @@ func testGetCommands() *core.Commands {
return argsI, nil
},
},
&core.Command{
Namespace: "test",
Resource: "many-positional",
ArgSpecs: core.ArgSpecs{
{
Name: "name-id",
Positional: true,
},
{
Name: "tag",
Positional: true,
},
},
AllowAnonymousClient: true,
ArgsType: reflect.TypeOf(testType{}),
Run: func(_ context.Context, argsI interface{}) (i interface{}, e error) {
return argsI, nil
},
},
&core.Command{
Namespace: "test",
Resource: "many-multi-positional",
ArgSpecs: core.ArgSpecs{
{
Name: "name-id",
Positional: true,
},
{
Name: "tags",
Positional: true,
},
},
AcceptMultiplePositionalArgs: true,
AllowAnonymousClient: true,
ArgsType: reflect.TypeOf(testTypeManyTags{}),
Run: func(_ context.Context, argsI interface{}) (i interface{}, e error) {
return argsI, nil
},
},
&core.Command{
Namespace: "test",
Resource: "raw-args",
Expand Down Expand Up @@ -196,42 +240,6 @@ func Test_PositionalArg(t *testing.T) {
}),
),
}))

t.Run("Invalid1", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional name-id=plop tag=world",
Check: core.TestCheckCombine(
core.TestCheckExitCode(1),
core.TestCheckError(&core.CliError{
Err: errors.New("a positional argument is required for this command"),
Hint: "Try running: scw test positional plop tag=world",
}),
),
}))

t.Run("Invalid2", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional tag=world name-id=plop",
Check: core.TestCheckCombine(
core.TestCheckExitCode(1),
core.TestCheckError(&core.CliError{
Err: errors.New("a positional argument is required for this command"),
Hint: "Try running: scw test positional plop tag=world",
}),
),
}))

t.Run("Invalid3", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional plop name-id=plop",
Check: core.TestCheckCombine(
core.TestCheckExitCode(1),
core.TestCheckError(&core.CliError{
Err: errors.New("a positional argument is required for this command"),
Hint: "Try running: scw test positional plop",
}),
),
}))
})

t.Run("simple", core.Test(&core.TestConfig{
Expand All @@ -240,6 +248,24 @@ func Test_PositionalArg(t *testing.T) {
Check: core.TestCheckExitCode(0),
}))

t.Run("simple2", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional name-id=plop tag=world",
Check: core.TestCheckExitCode(0),
}))

t.Run("simple3", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional tag=world name-id=plop",
Check: core.TestCheckExitCode(0),
}))

t.Run("simple4", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional plop name-id=plop",
Check: core.TestCheckExitCode(0),
}))

t.Run("full command", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test positional plop tag=world",
Expand Down Expand Up @@ -272,6 +298,41 @@ func Test_PositionalArg(t *testing.T) {
core.TestCheckGolden(),
),
}))

t.Run("many positional", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test many-positional tag1 name-id=plop",
Check: core.TestCheckExitCode(0),
}))

t.Run("many positional", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test many-positional tag1 name-id=plop",
Check: core.TestCheckCombine(
core.TestCheckExitCode(0),
func(t *testing.T, ctx *core.CheckFuncCtx) {
t.Helper()
res := ctx.Result.(*testType)
assert.Equal(t, "plop", res.NameID)
assert.Equal(t, "tag1", res.Tag)
},
),
}))

t.Run("many multi-positional", core.Test(&core.TestConfig{
Commands: testGetCommands(),
Cmd: "scw test many-multi-positional pos1 pos2 name-id=plop",
Check: core.TestCheckCombine(
core.TestCheckExitCode(0),
func(t *testing.T, ctx *core.CheckFuncCtx) {
t.Helper()
res := ctx.Result.(*testTypeManyTags)
assert.Equal(t, "plop", res.NameID)
assert.Equal(t, "pos1", res.Tags[0])
assert.Equal(t, "pos2", res.Tags[1])
},
),
}))
}

func Test_MultiPositionalArg(t *testing.T) {
Expand Down
Loading