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

feat: improve missing argument error messages #711

Merged
merged 9 commits into from
Apr 2, 2024
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
24 changes: 23 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,30 @@ Additional Commands:
ssh Spawn an SSH connection for the server
```

### Command validation

Please use the `util.Validate` method to make sure the command is validated against its usage string during runtime.
This can be done by setting the `Args` field of the `cobra.Command` struct to `util.Validate`:

```go
cmd := &cobra.Command{
Use: "my-command [options]",
Args: util.Validate,
Run: func(cmd *cobra.Command, args []string) {
// ...
},
}
```

If you are using base commands like `base.Cmd` or `base.ListCmd` etc., this is already done for you.

**Note:** Your command usage needs to follow the [docopt](http://docopt.org/) syntax for this to work.
If your command uses optional positional arguments or other complicated usage that necessitates to disable
argument count checking, you use `util.ValidateLenient` instead. It will not throw an error if there are
more arguments than expected.

### Generated files

Generated files (that are created by running `go generate`) should be prefixed with `zz_`. This is to group them
Generated files (that are created by running `go generate`) should be prefixed with `zz_`. This is to group them
together in the file list and to easily identify them as generated files. Also, it allows the CI to check if the
generated files are up-to-date.
3 changes: 2 additions & 1 deletion internal/cmd/all/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package all
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "all",
Short: "Commands that apply to all resources",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/base/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (gc *Cmd) CobraCommand(s state.State) *cobra.Command {
cmd := gc.BaseCobraCommand(s.Client())

if cmd.Args == nil {
cmd.Args = cobra.NoArgs
cmd.Args = util.Validate
}

cmd.TraverseChildren = true
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/base/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (cc *CreateCmd) CobraCommand(s state.State) *cobra.Command {
output.AddFlag(cmd, output.OptionJSON(), output.OptionYAML())

if cmd.Args == nil {
cmd.Args = cobra.NoArgs
cmd.Args = util.Validate
}

cmd.TraverseChildren = true
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package base
import (
"fmt"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -32,9 +31,9 @@ func (dc *DeleteCmd) CobraCommand(s state.State) *cobra.Command {
}

cmd := &cobra.Command{
Use: fmt.Sprintf("delete %s<%s>", opts, strings.ToLower(dc.ResourceNameSingular)),
Use: fmt.Sprintf("delete %s<%s>", opts, util.ToKebabCase(dc.ResourceNameSingular)),
Short: dc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"os"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -33,9 +32,9 @@ type DescribeCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (dc *DescribeCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("describe [options] <%s>", strings.ToLower(dc.ResourceNameSingular)),
Use: fmt.Sprintf("describe [options] <%s>", util.ToKebabCase(dc.ResourceNameSingular)),
Short: dc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(dc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
8 changes: 4 additions & 4 deletions internal/cmd/base/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ type LabelCmds struct {
// AddCobraCommand creates a command that can be registered with cobra.
func (lc *LabelCmds) AddCobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("add-label [--overwrite] <%s> <label>...", strings.ToLower(lc.ResourceNameSingular)),
Use: fmt.Sprintf("add-label [--overwrite] <%s> <label>...", util.ToKebabCase(lc.ResourceNameSingular)),
Short: lc.ShortDescriptionAdd,
Args: cobra.MinimumNArgs(2),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(lc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down Expand Up @@ -89,9 +89,9 @@ func validateAddLabel(_ *cobra.Command, args []string) error {
// RemoveCobraCommand creates a command that can be registered with cobra.
func (lc *LabelCmds) RemoveCobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("remove-label <%s> (--all | <label>...)", strings.ToLower(lc.ResourceNameSingular)),
Use: fmt.Sprintf("remove-label <%s> (--all | <label>...)", util.ToKebabCase(lc.ResourceNameSingular)),
Short: lc.ShortDescriptionRemove,
Args: cobra.MinimumNArgs(1),
Args: util.ValidateLenient,
ValidArgsFunction: cmpl.SuggestArgs(
cmpl.SuggestCandidatesF(lc.NameSuggestions(s.Client())),
cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string {
Expand Down
1 change: 1 addition & 0 deletions internal/cmd/base/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func (lc *ListCmd) CobraCommand(s state.State) *cobra.Command {
fmt.Sprintf("Displays a list of %s.", lc.ResourceNamePlural),
outputColumns,
),
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
PreRunE: s.EnsureToken,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/set_rdns.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"net"
"reflect"
"strings"

"github.com/spf13/cobra"

Expand All @@ -28,9 +27,9 @@ type SetRdnsCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (rc *SetRdnsCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("set-rdns [options] --hostname <hostname> <%s>", strings.ToLower(rc.ResourceNameSingular)),
Use: fmt.Sprintf("set-rdns [options] --hostname <hostname> <%s>", util.ToKebabCase(rc.ResourceNameSingular)),
Short: rc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(rc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
5 changes: 2 additions & 3 deletions internal/cmd/base/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package base
import (
"fmt"
"reflect"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand All @@ -28,9 +27,9 @@ type UpdateCmd struct {
// CobraCommand creates a command that can be registered with cobra.
func (uc *UpdateCmd) CobraCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: fmt.Sprintf("update [options] <%s>", strings.ToLower(uc.ResourceNameSingular)),
Use: fmt.Sprintf("update [options] <%s>", util.ToKebabCase(uc.ResourceNameSingular)),
Short: uc.ShortDescription,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(uc.NameSuggestions(s.Client()))),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/certificate/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package certificate
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "certificate",
Short: "Manage certificates",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/certificate/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ var CreateCmd = base.CreateCmd{
cmd := &cobra.Command{
Use: "create [options] --name <name> (--type managed --domain <domain> | --type uploaded --cert-file <file> --key-file <file>)",
Short: "Create or upload a Certificate",
Args: cobra.ExactArgs(0),
}

cmd.Flags().String("name", "", "Certificate name (required)")
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

Expand Down Expand Up @@ -94,7 +95,7 @@ and source this file from your PowerShell profile.

PS> hcloud completion powershell > hcloud.ps1
`,
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgs: []string{"bash", "fish", "zsh", "powershell"},
DisableFlagsInUseLine: true,
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/active.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import (

"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func newActiveCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "active",
Short: "Show active context",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runActive),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package context
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "context",
Short: "Manage contexts",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/spf13/cobra"
"golang.org/x/term"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -19,7 +20,7 @@ func newCreateCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "create <name>",
Short: "Create a new context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runCreate),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/cmpl"
"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -15,7 +16,7 @@ func newDeleteCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "delete <context>",
Short: "Delete a context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidates(config.ContextNames(s.Config())...)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/context/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func newListCommand(s state.State) *cobra.Command {
"Displays a list of contexts.",
listTableOutput.Columns(),
),
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
RunE: state.Wrap(s, runList),
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/context/use.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/cmpl"
"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
"github.com/hetznercloud/cli/internal/state/config"
)
Expand All @@ -15,7 +16,7 @@ func newUseCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "use <context>",
Short: "Use a context",
Args: cobra.ExactArgs(1),
Args: util.Validate,
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidates(config.ContextNames(s.Config())...)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/datacenter/datacenter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package datacenter
import (
"github.com/spf13/cobra"

"github.com/hetznercloud/cli/internal/cmd/util"
"github.com/hetznercloud/cli/internal/state"
)

func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "datacenter",
Short: "Manage datacenters",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/add_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ var AddRuleCmd = base.Cmd{
cmd := &cobra.Command{
Use: "add-rule [options] (--direction in --source-ips <ips> | --direction out --destination-ips <ips>) --protocol <icmp|udp|tcp|esp|gre> <firewall>",
Short: "Add a single rule to a firewall",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/apply_to_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var ApplyToResourceCmd = base.Cmd{
cmd := &cobra.Command{
Use: "apply-to-resource (--type server --server <server> | --type label_selector --label-selector <label-selector>) <firewall>",
Short: "Applies a Firewall to a single resource",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var CreateCmd = base.CreateCmd{
cmd := &cobra.Command{
Use: "create [options] --name <name>",
Short: "Create a Firewall",
Args: cobra.NoArgs,
}
cmd.Flags().String("name", "", "Name")
cmd.MarkFlagRequired("name")
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/delete_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ var DeleteRuleCmd = base.Cmd{
cmd := &cobra.Command{
Use: "delete-rule [options] (--direction in --source-ips <ips> | --direction out --destination-ips <ips>) --protocol <icmp|esp|gre|udp|tcp> <firewall>",
Short: "Delete a single rule to a firewall",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/firewall/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func NewCommand(s state.State) *cobra.Command {
cmd := &cobra.Command{
Use: "firewall",
Short: "Manage Firewalls",
Args: cobra.NoArgs,
Args: util.Validate,
TraverseChildren: true,
DisableFlagsInUseLine: true,
}
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/remove_from_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var RemoveFromResourceCmd = base.Cmd{
cmd := &cobra.Command{
Use: "remove-from-resource (--type server --server <server> | --type label_selector --label-selector <label-selector>) <firewall>",
Short: "Removes a Firewall from a single resource",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/firewall/replace_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var ReplaceRulesCmd = base.Cmd{
cmd := &cobra.Command{
Use: "replace-rules --rules-file <file> <firewall>",
Short: "Replaces all rules from a Firewall from a file",
Args: cobra.ExactArgs(1),
ValidArgsFunction: cmpl.SuggestArgs(cmpl.SuggestCandidatesF(client.Firewall().Names)),
TraverseChildren: true,
DisableFlagsInUseLine: true,
Expand Down
1 change: 0 additions & 1 deletion internal/cmd/floatingip/assign.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ var AssignCmd = base.Cmd{
return &cobra.Command{
Use: "assign <floating-ip> <server>",
Short: "Assign a Floating IP to a server",
Args: cobra.ExactArgs(2),
ValidArgsFunction: cmpl.SuggestArgs(
cmpl.SuggestCandidatesF(client.FloatingIP().Names),
cmpl.SuggestCandidatesF(client.Server().Names),
Expand Down
Loading