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

ExactArgs + ValidArgs + OnlyValidArgs #745

Closed
patrick-motard opened this issue Sep 14, 2018 · 12 comments · Fixed by #1643 · May be fixed by #841
Closed

ExactArgs + ValidArgs + OnlyValidArgs #745

patrick-motard opened this issue Sep 14, 2018 · 12 comments · Fixed by #1643 · May be fixed by #841
Labels
area/flags-args Changes to functionality around command line flags and args kind/feature A feature request for cobra; new or enhanced behavior

Comments

@patrick-motard
Copy link

patrick-motard commented Sep 14, 2018

I have a command called i3 that must have at exactly one argument given to it, and the argument can either be "load" or "disable". By passing the command "ExactArgs(1)" I can make it do exactly one arg, but it doesn't respect "ValidArgs". I can pass "OnlyValidArgs" to make it respect "ValidArgs", but I can't pass it both. Something like Args: cobra.ExactArgs(1) && cobra.OnlyValidArgs(cobra, args) would be awesome.

var i3Cmd = &cobra.Command{
	Use:       "i3",
	Short:     "A brief description of your command",
	Long:      ``,
	ValidArgs: []string{"load", "disable"},
	Args:      cobra.ExactArgs(1),
        Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("i3 called")
	},
}

PASS go run main.go i3 --> Error: accepts 1 arg(s), received 0
PASS go run main.go i3 hello world --> Error: accepts 1 arg(s), received 2
FAIL go run main.go i3 chicken --> i3 called
PASS go run main.go i3 load --> i3 called

ValidArgs is being ignored because cobra.OnlyValidArgs(cmd, args) isn't called. When i pass in chicken, I want it to error because 'load' is the only valid arg.

Trying again with a custom Arg filter.

var i3Cmd = &cobra.Command{
	Use:       "i3",
	Short:     "A brief description of your command",
	Long:      ``,
	ValidArgs: []string{"load", "disable"},
	Args: func(cmd *cobra.Command, args []string) error {
		if len(args) != 1 {
			return errors.New("Requires exactly 1 arg.")
		}
		return cobra.OnlyValidArgs(cmd, args)
	},
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("i3 called")
	},
}

PASS go run main.go i3 hello --> Error: invalid argument "hello" for "dot i3"
PASS go run main.go i3 load --> i3 called
PASS go run main.go i3 hello world --> Error: Requires exactly 1 arg
PASS go run main.go i3 --> Error: Requires exactly 1 arg.

The example above works... but it doesn't use cobra.ExactArgs(1). I don't like re-inventing the wheel. How can I use both OnlyValidArgs and ExactArgs together? I tried:

var i3Cmd = &cobra.Command{
	Use:       "i3",
	Short:     "A brief description of your command",
	Long:      ``,
	ValidArgs: []string{"load"},
	Args: func(cmd *cobra.Command, args []string) error {
		err := cobra.ExactArgs(1)
		if err != nil {
			return err // <--- this is a compile error
		}	
		return cobra.OnlyValidArgs(cmd, args)
	},
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("i3 called")
	},
}

But ExactArgs returns PositionalArgs.. which looks to be a Type.. I'm not very familiar with Go, so this is confusing to me. I don't know how to combine the two. Any insight would be appreciated!

Also.. If I set ValidArgs and OnlyValidArgs, and an invalid command arg is provided, the user isn't notified of what the valid Args are. Is this the same feature request as #571? That seems like it would be very helpful.

related #284
related #571
related #346

@yoni386
Copy link

yoni386 commented Oct 2, 2018

I would like to use OnlyValidArgs and MinimumNArgs(1)

@Antolius
Copy link

@patrick-motard for your exact use-case you can use cobra.ExactValidArgs(1) as a value for ValidArgs.

More general solution would be something like this:

var cmd = &cobra.Command{
	Use:       "cmd",
	Short:     "General solution",
	ValidArgs: []string{"some", "acceptable", "values"},
	Args:      matchAll(cobra.MinimumNArgs(1), cobra.OnlyValidArgs),
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("cmd called")
	},
}

func matchAll(checks ...cobra.PositionalArgs) cobra.PositionalArgs {
	return func(cmd *cobra.Command, args []string) error {
		for _, check := range checks {
			if err := check(cmd, args); err != nil {
				return err
			}
		}
		return nil
	}
}

Here the matchAll function will effectively combine any number of cobra.PositionalArgs you pass to it, including the ones from cobra package and arbitrary ones you might have defined in your code.

If you think it would be a good idea to add this feature to the library itself I can submit a pull request.

@github-actions
Copy link

github-actions bot commented Apr 9, 2020

This issue is being marked as stale due to a long period of inactivity

@firelizzard18
Copy link

I would use the feature proposed by @Antolius

@umarcor
Copy link
Contributor

umarcor commented Nov 10, 2020

@firelizzard18 you might want to try #841.

@firelizzard18
Copy link

@umarcor I want a function that composes multiple argument validators. Your MR does not provide this. Here is what I am using:

func args(fns ...cobra.PositionalArgs) cobra.PositionalArgs {
	return func(cmd *cobra.Command, args []string) error {
		for _, fn := range fns {
			if err := fn(cmd, args); err != nil {
				return err
			}
		}
		return nil
	}
}

@umarcor
Copy link
Contributor

umarcor commented Nov 10, 2020

@firelizzard18, my bad. The reference should have been #896.

@firelizzard18
Copy link

@umarcor I +1'ed #896. Hopefully that will go somewhere...

@johnSchnake
Copy link
Collaborator

#841 is still active and other issues have mentioned it solves exactly this sort of problem. Leaving the issue open and unmarking as stale since the PR is in progress.

@johnSchnake johnSchnake added area/flags-args Changes to functionality around command line flags and args kind/feature A feature request for cobra; new or enhanced behavior and removed kind/stale labels Mar 5, 2022
@marckhouzam
Copy link
Collaborator

The request and discussion here seem to be about composing multiple validators together.
This is now possible thanks to #896 and the new MatchAll(pargs ...PositionalArgs) function:

cobra/args.go

Lines 111 to 112 in 03c3eb7

// MatchAll allows combining several PositionalArgs to work in concert.
func MatchAll(pargs ...PositionalArgs) PositionalArgs {

@marckhouzam
Copy link
Collaborator

We now even have ExactValidArgs(n int) which seems to be exactly why this issue was opened:

cobra/args.go

Lines 89 to 92 in 03c3eb7

// ExactValidArgs returns an error if
// there are not exactly N positional args OR
// there are any positional args that are not in the `ValidArgs` field of `Command`
func ExactValidArgs(n int) PositionalArgs {

I'm going to go ahead and close this as fixed.

@umarcor
Copy link
Contributor

umarcor commented Mar 23, 2022

@marckhouzam deprecating ExactValidArgs and OnlyValidArgs was proposed and approved in 2019, because that's an inconsistency in the implementation/codebase which is to be fixed in #841 (or #1643). Therefore, I do not recommend suggesting the usage of those functions.

When #841 (or #1643) is merged, the default behaviour of ExactArgs + ValidArgs will be what the OP is asking.

Meanwhile, users can achieve the same result through MatchAll. However, that's meant for custom validators, not for combining the built-in ones. If we are going to suggest that as the official workaround for the incomplete implementation of ValidArgs, IMHO we'd better remove the feature from the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flags-args Changes to functionality around command line flags and args kind/feature A feature request for cobra; new or enhanced behavior
Projects
None yet
7 participants