cobra icon indicating copy to clipboard operation
cobra copied to clipboard

Allow custom error message from argument validation

Open Galzzly opened this issue 1 year ago • 2 comments

If there are errors parsing the arguments to a command, a custom error can be output by way of the cmd.SetFlagErrorFunc function. However, this only applies if there is an issue with the arguments provided and does not take into account any Annotations applied to the flags such as whether they are mutually exclusive.

It would be useful to be able to customise the output of the flag validations, so as to ensure that application logging is consistent throughout.

At the minute, it is possible to work around this by performing manual checks for the exclusive flags in the PreRun, and logging/erroring there rather than making use of the MarkFlagsMutuallyExclusive function.

Galzzly avatar Jun 27 '23 10:06 Galzzly

In the same vein, it would be nice to output the usage string iff argument validation fails. Right now the SilenceUsage flag is all or nothing, and it is generally annoying to output the usage string if the command was invoked with valid arguments (but failed anyway, for whatever reason), so we set that flag to true. But we would much prefer to see the usage string when the command is used incorrectly.

MisterSquishy avatar Jul 14 '23 14:07 MisterSquishy

If I understood correctly, this topic is about how (misleading) generated usage output looks like when MarkFlagsMutuallyExclusive is used (and other modifiers) on some of the flags (correct me if I'm wrong). My current approach about flags is that they all required, but with making use of default values taken from config file, then overwritten by environment values (if there is any), and last taken (overwritten) from directly provided flags. Except -h, --help and -c, --config, because existence of config file is not mandatory to configurate cmd. As is, this is sufficient to make generate explanatory usage output (using templated description). But MarkFlagsMutuallyExclusive modifier (in combination with MarkFlagsOneRequired as another scenario) is not represented in any way.
For example, below --client-secret and --client-secret-file are modified with MarkFlagsMutuallyExclusive and MarkFlagsOneRequired.

...
Usage:
  cmd subcmd [flags]

Flags:
...
      --client-id string            OAuth2 client ID. [env: CMD_CLIENT_ID] (default "cmd-service")
      --client-secret string        Plain text OAuth2 client secret. [env: CMD_CLIENT_SECRET] (default "C001D00D-5ECC-BEEF-CA4E-B00B1E54A111")
      --client-secret-file string   Path to the file containing plain text OAuth2 client secret. [env: CMD_CLIENT_SECRET_FILE]
  -h, --help                        Help for subcmd command.
...
Global Flags:
  -c, --config string      Config file (default fallback is $HOME/.cmd.yaml, then /etc/.cmd.yaml) [env: CMD_CONFIG] (default "/etc/.cmd.yaml")
...

Should --client-secret and --client-secret-file flags be organized in their own flag group and referenced in Usage:? Section something like:

...
Usage:
  cmd subcmd [flags] [--client-secret  | --client-secret-file]

Flags:
...
      --client-id string            OAuth2 client ID. [env: CMD_CLIENT_ID] (default "cmd-service")
  -h, --help                        Help for subcmd command.
...
(Mutually exclusive) Client secret flags:
      --client-secret string        Plain text OAuth2 client secret. [env: CMD_CLIENT_SECRET] (default "C001D00D-5ECC-BEEF-CA4E-B00B1E54A111")
      --client-secret-file string   Path to the file containing plain text OAuth2 client secret. [env: CMD_CLIENT_SECRET_FILE]
...

This potentially leads to multiple mutually exclusive flag groups, can't say if it's good or bad. But this (probably) can be done with current API with some workarounds. I think solution for displaying mutually exclusive flags should be done out of the box somehow.

dimandzhi avatar Oct 03 '23 09:10 dimandzhi