Skip to content
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

Clarification regarding Command.Arguments property #2056

Open
kudla opened this issue Feb 11, 2025 · 7 comments
Open

Clarification regarding Command.Arguments property #2056

kudla opened this issue Feb 11, 2025 · 7 comments
Labels
area/v3 relates to / is being considered for v3 kind/question someone asking a question status/triage maintainers still need to look into this

Comments

@kudla
Copy link

kudla commented Feb 11, 2025

Hi
Guys please could you clarify the idea behind Command.Arguments property.

Actually my use case is a command with one required positional argument

app command <required-position-argument>

So without Command.Arguments declaration cmd.Args().Get(0) works well. Yet in this case there's no validation performed for argument was provided

With adding

Arguments: []cli.Argument{
  &cli.StringArgument{
       Name: "required-position-argument",
  }
}

it works the very same way (without args number validation)

While trying to extend arg declaration with

  &cli.StringArgument{
       Name: "required-position-argument",
       Min: 1,
       Max: 1,
  }

it actually starts validating against arguments number. Yet the argument value is not accessible neither through cmd.Args().Get(0) nor cmd.String("required-position-argument"). Instead it could be reached over something like cmd.Arguments[0].Values[0]

So as the examples seem not covered such a deep cases it is not clear (for v3 specifically)

  • How to make in general the case of one Required argument with a single scalar string value
  • What is the purpose of Argument.Name though. Are such argument values could be accessed by name somehow or is it just for help referring matter only
  • What is the general case for Command.Arguments declaration

Thank you.

@kudla kudla added area/v2 relates to / is being considered for v2 kind/question someone asking a question status/triage maintainers still need to look into this labels Feb 11, 2025
@dearchap dearchap added area/v3 relates to / is being considered for v3 and removed area/v2 relates to / is being considered for v2 labels Feb 16, 2025
@dearchap dearchap changed the title your question title goes here Clarification regarding Command.Arguments property Feb 16, 2025
@dearchap
Copy link
Contributor

@kudla The Argument instances should have either a Destination or Values set for example

val dest string
&cli.StringArgument{
       Name: "required-position-argument",
       Min: 1,
       Max: 1,
       Destination: &dest,
  }
val dest []string
&cli.StringArgument{
       Name: "required-position-argument",
       Min: 1,
       Max: 2,
       Values: &dest,
  }

In the first case since you need only a single string positional argument the dest will have the single value. In the second case since you could have 2 values the dest(string slice) can contain both.

@kudla
Copy link
Author

kudla commented Feb 18, 2025

Hi @dearchap thanks for the reply.
This pretty clarifies a usage for the cases.
Still another question is there a way to access these arguments from the context or the cmd at the action handler

func (ctx context.Context, cmd *cli.Command) error {
 // how to resolve arguments from here (by name e.g.)
 value := 
 orValues := 
}

And what the purpose for Name any way. I assume there should be the way to refer such args by name some how so? Is there?

Thnx

@bearsh
Copy link

bearsh commented Feb 18, 2025

@kudla The Argument instances should have either a Destination or Values set for example

To me, this seems quite strange. Both FlagBase and ArgumentBase have a Destination field but only for a Argument it is kind of required?

Wouldn't id be more intuitive and consistent to have a getter method to get this argument values by name?

@dearchap
Copy link
Contributor

Yes we should probably add a getter method for Argument.

@punmechanic
Copy link

punmechanic commented Feb 21, 2025

I've run into this behavior unexpectedly. PoC:

var rootCmd = &cli.Command{
	Name: "prtools",
	Commands: []*cli.Command{
		{
			Name:   "validate",
			Action: validate,
			Arguments: []cli.Argument{
				&cli.StringArg{Name: "filename", Max: -1},
			},
		},
	},
}

func Run(ctx context.Context, args []string) error {
	return rootCmd.Run(context.Background(), os.Args)
}

I expected to be able to access the args using .StringArgs("filename"), as I can with flags:

func validate(ctx context.Context, cmd *cli.Command) error {
	var (
		results []validationResult
		targets = cmd.Args().Slice()
	)

However, not only does an StringArgs("filename") not exist, cmd.Args().Slice() will yield an empty slice if Max is set to -1., which definitely seems like an error. If Max is set to any positive integer, this code will work as expected - but I want Max to be unbounded because I am processing a glob in a shell so I can't reasonably know how many results it'll have ahead of time.

This is confusing behavior when you consider how flags operate. I prefer being able to define my command structure in a tree at the root of the application with all flags, args, etc, so I don't use Destination/Values/global values to be passed to Actions.

@Skeeve
Copy link
Contributor

Skeeve commented Feb 21, 2025

Correct me if I'm wrong, but didn't you name the string argument "filename", not "name"?

@punmechanic
Copy link

punmechanic commented Feb 21, 2025

Yes, typo; the actual label is immaterial because there's no way to retrieve the value of an argument using a method like this. I've corrected it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v3 relates to / is being considered for v3 kind/question someone asking a question status/triage maintainers still need to look into this
Projects
None yet
Development

No branches or pull requests

5 participants