fix: various shell flag argument completions taking fixed set of values (#376)

Co-authored-by: Toni Kangas <toni.kangas@upcloud.com>

Author: scop

internal/commands/command.go

@@ -192,6 +192,8 @@ func (s *BaseCommand) AddFlags(flags *pflag.FlagSet) {
 		s.Cobra().Flags().AddFlag(flag)
 		if _, ok := flag.Annotations[FlagAnnotationNoFileCompletions]; ok {
 			Must(s.Cobra().RegisterFlagCompletionFunc(flag.Name, cobra.NoFileCompletions))
+		} else if values, ok := flag.Annotations[FlagAnnotationFixedCompletions]; ok {
+			Must(s.Cobra().RegisterFlagCompletionFunc(flag.Name, cobra.FixedCompletions(values, cobra.ShellCompDirectiveNoFileComp)))
 		}
 	})
 }

internal/commands/ipaddress/assign.go

@@ -56,6 +56,8 @@ func (s *assignCommand) InitCommand() {
 	config.AddToggleFlag(fs, &s.floating, "floating", false, "Whether the address to be assigned is a floating one.")
 
 	s.AddFlags(fs)
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("access", cobra.FixedCompletions([]string{"public", "utility"}, cobra.ShellCompDirectiveNoFileComp)))
+	commands.Must(s.Cobra().RegisterFlagCompletionFunc("family", cobra.FixedCompletions([]string{"IPv4", "IPv6"}, cobra.ShellCompDirectiveNoFileComp)))
 	commands.Must(s.Cobra().RegisterFlagCompletionFunc("mac", cobra.NoFileCompletions))
 }
 

internal/commands/network/network.go

@@ -54,6 +54,7 @@ func handleNetwork(in string) (*upcloud.IPNetwork, error) {
 		return nil, err
 	}
 
+	commands.Must(fs.SetAnnotation("dhcp-dns", commands.FlagAnnotationFixedCompletions, []string{"true", "false"}))
 	commands.Must(fs.SetAnnotation("address", commands.FlagAnnotationNoFileCompletions, nil))
 
 	if dhcp != "" {

internal/commands/servergroup/create.go

@@ -88,14 +88,16 @@ func (c *createCommand) InitCommand() {
 	c.params = createParams{CreateServerGroupRequest: request.CreateServerGroupRequest{}}
 
 	fs.StringVar(&c.params.Title, "title", defaultCreateParams.Title, "Server group title.")
-	fs.StringVar(&c.params.antiAffinityPolicy, "anti-affinity-policy", defaultCreateParams.antiAffinityPolicy, "Anti-affinity policy. Valid values are `yes` (best effort), `strict` and `no`.  Will take effect upon server start.")
+	aaPolicies := []string{"yes", "strict", "no"}
+	fs.StringVar(&c.params.antiAffinityPolicy, "anti-affinity-policy", defaultCreateParams.antiAffinityPolicy, "Anti-affinity policy. Valid values are "+namedargs.ValidValuesHelp(aaPolicies...)+". Will take effect upon server start.")
 	fs.StringArrayVar(&c.params.labels, "label", defaultCreateParams.labels, "Labels to describe the server group in `key=value` format, multiple can be declared.\nUsage: --label env=dev\n\n--label owner=operations")
 	fs.StringArrayVar(&c.params.servers, "server", defaultCreateParams.servers, "Servers to be added to the server group, multiple can be declared.\nUsage: --server my-server\n\n--server 00333d1b-3a4a-4b75-820a-4a56d70395dd")
 
 	c.AddFlags(fs)
 
 	commands.Must(c.Cobra().MarkFlagRequired("title"))
 	commands.Must(c.Cobra().RegisterFlagCompletionFunc("title", cobra.NoFileCompletions))
+	commands.Must(c.Cobra().RegisterFlagCompletionFunc("anti-affinity-policy", cobra.FixedCompletions(aaPolicies, cobra.ShellCompDirectiveNoFileComp)))
 	commands.Must(c.Cobra().RegisterFlagCompletionFunc("label", cobra.NoFileCompletions))
 
 	// Deprecating servergroup in favour of server-group

internal/commands/servergroup/modify.go

@@ -81,13 +81,15 @@ func (p *modifyParams) processParams(exec commands.Executor, uuid string) error
 // InitCommand implements Command.InitCommand
 func (c *modifyCommand) InitCommand() {
 	fs := &pflag.FlagSet{}
-	fs.StringVar(&c.params.antiAffinityPolicy, "anti-affinity-policy", defaultModifyParams.antiAffinityPolicy, "Anti-affinity policy. Valid values are `yes` (best effort), `strict` and `no`. Will take effect upon server start.")
+	aaPolicies := []string{"yes", "strict", "no"}
+	fs.StringVar(&c.params.antiAffinityPolicy, "anti-affinity-policy", defaultModifyParams.antiAffinityPolicy, "Anti-affinity policy. Valid values are "+namedargs.ValidValuesHelp(aaPolicies...)+". Will take effect upon server start.")
 	fs.StringArrayVar(&c.params.labels, "label", defaultModifyParams.labels, "Labels to describe the server in `key=value` format, multiple can be declared. If set, all the existing labels will be replaced with provided ones.\nUsage: --label env=dev\n\n--label owner=operations")
 	fs.StringVar(&c.params.Title, "title", defaultModifyParams.Title, "New server group title.")
 	fs.StringArrayVar(&c.params.servers, "server", defaultModifyParams.servers, "Servers that belong to the server group, multiple can be declared. If set, all the existing server entries will be replaced with provided ones.\nUsage: --server my-server\n\n--server 00333d1b-3a4a-4b75-820a-4a56d70395dd")
 
 	c.AddFlags(fs)
 	commands.Must(c.Cobra().RegisterFlagCompletionFunc("title", cobra.NoFileCompletions))
+	commands.Must(c.Cobra().RegisterFlagCompletionFunc("anti-affinity-policy", cobra.FixedCompletions(aaPolicies, cobra.ShellCompDirectiveNoFileComp)))
 	commands.Must(c.Cobra().RegisterFlagCompletionFunc("label", cobra.NoFileCompletions))
 
 	// Deprecating servergroup in favour of server-group

internal/commands/util.go

@@ -21,6 +21,8 @@ import (
 const (
 	// FlagAnnotationNoFileCompletions is the annotation name to use for our flags that have no filename completions.
 	FlagAnnotationNoFileCompletions = "upctl_flag_no_file_completions"
+	// FlagAnnotationFixedCompletions is the annotation name to use for our flags that have a fixed set of completions.
+	FlagAnnotationFixedCompletions = "upctl_flag_fixed_completions"
 )
 
 // ParseN parses a complex, querystring-type argument from `in` and splits values to `n` amount of substrings

internal/core/core.go

@@ -8,6 +8,7 @@ import (
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands"
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/commands/all"
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/config"
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/namedargs"
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/terminal"
 	"github.com/UpCloudLtd/upcloud-cli/v3/internal/ui"
 
@@ -90,9 +91,10 @@ func BuildRootCmd(conf *config.Config) cobra.Command {
 	flags.StringVarP(
 		&conf.GlobalFlags.ConfigFile, "config", "", "", "Configuration file path.",
 	)
+	outputFormats := []string{config.ValueOutputHuman, config.ValueOutputJSON, config.ValueOutputYAML}
 	flags.StringVarP(
 		&conf.GlobalFlags.OutputFormat, "output", "o", "human",
-		"Output format (supported: json, yaml and human)",
+		"Output format. Valid values are "+namedargs.ValidValuesHelp(outputFormats...)+".",
 	)
 	config.AddToggleFlag(flags, &conf.GlobalFlags.ForceColours, "force-colours", false, "Force coloured output despite detected terminal support.")
 	config.AddToggleFlag(flags, &conf.GlobalFlags.NoColours, "no-colours", false, "Disable coloured output despite detected terminal support. Colours can also be disabled by setting NO_COLOR environment variable.")
@@ -116,6 +118,7 @@ func BuildRootCmd(conf *config.Config) cobra.Command {
 	rootCmd.SetUsageTemplate(ui.CommandUsageTemplate())
 	rootCmd.SetUsageFunc(ui.UsageFunc)
 
+	commands.Must(rootCmd.RegisterFlagCompletionFunc("output", cobra.FixedCompletions(outputFormats, cobra.ShellCompDirectiveNoFileComp)))
 	commands.Must(rootCmd.RegisterFlagCompletionFunc("client-timeout", cobra.NoFileCompletions))
 
 	return rootCmd

internal/namedargs/text.go

@@ -1,8 +1,29 @@
 package namedargs
 
-import "fmt"
+import (
+	"fmt"
+	"strings"
+)
 
 // Returns description for --zone argument, e.g. "Zone where to create the resource...".
 func ZoneDescription(resource string) string {
 	return fmt.Sprintf("Zone where to create the %s. Run `upctl zone list` to list all available zones.", resource)
 }
+
+// ValidValuesHelp wraps values in backticks and adds human readable separators.
+// For example, "`one`, `two` and `three`".
+func ValidValuesHelp(values ...string) string {
+	if len(values) == 0 {
+		return ""
+	}
+
+	if len(values) == 1 {
+		return fmt.Sprintf("`%s`", values[0])
+	}
+
+	return fmt.Sprintf(
+		"`%s` and `%s`",
+		strings.Join(values[:len(values)-1], "`, `"),
+		values[len(values)-1],
+	)
+}

internal/namedargs/text_test.go

@@ -0,0 +1,42 @@
+package namedargs_test
+
+import (
+	"testing"
+
+	"github.com/UpCloudLtd/upcloud-cli/v3/internal/namedargs"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidValuesHelp(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		values   []string
+		expected string
+	}{
+		{
+			name:     "no values",
+			values:   []string{},
+			expected: "",
+		},
+		{
+			name:     "single values",
+			values:   []string{"foo"},
+			expected: "`foo`",
+		},
+		{
+			name:     "two values",
+			values:   []string{"foo", "bar"},
+			expected: "`foo` and `bar`",
+		},
+		{
+			name:     "multiple values",
+			values:   []string{"foo", "bar", "baz"},
+			expected: "`foo`, `bar` and `baz`",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			actual := namedargs.ValidValuesHelp(test.values...)
+			assert.Equal(t, test.expected, actual)
+		})
+	}
+}