From b28b962fbacdc82551a4ee1ceb210b7f6199b20e Mon Sep 17 00:00:00 2001 From: Robert Lin Date: Mon, 27 Nov 2023 16:30:59 -0800 Subject: [PATCH] sg: improve positional completions (#58569) Improves positional completions added for `sg msp` in #58538 by checking for partial flag input (`-` and `--`), which now complete flags correctly, and also improving the way we build positional argument completions with a more robust helper, `completions.CompletePositionalArgs`. --- deps.bzl | 4 +- dev/sg/ci/subcommands.go | 2 +- dev/sg/msp/repo/repo.go | 30 ++++++------- dev/sg/msp/sg_msp.go | 34 +++++++------- dev/sg/sg_generate.go | 2 +- dev/sg/sg_lint.go | 2 +- dev/sg/sg_live.go | 2 +- dev/sg/sg_run.go | 2 +- dev/sg/sg_secret.go | 2 +- dev/sg/sg_start.go | 2 +- dev/sg/sg_tests.go | 2 +- go.mod | 2 +- go.sum | 3 +- lib/cliutil/completions/options.go | 71 +++++++++++++++++++++++------- monitoring/command/generate.go | 2 +- 15 files changed, 98 insertions(+), 64 deletions(-) diff --git a/deps.bzl b/deps.bzl index a458d4dcb8f..d2716e108a5 100644 --- a/deps.bzl +++ b/deps.bzl @@ -1418,8 +1418,8 @@ def go_dependencies(): name = "com_github_cpuguy83_go_md2man_v2", build_file_proto_mode = "disable_global", importpath = "github.com/cpuguy83/go-md2man/v2", - sum = "h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w=", - version = "v2.0.2", + sum = "h1:qMCsGGgs+MAzDFyp9LpAe1Lqy/fY/qCovCm0qnXZOBM=", + version = "v2.0.3", ) go_repository( name = "com_github_creack_pty", diff --git a/dev/sg/ci/subcommands.go b/dev/sg/ci/subcommands.go index 32d1e7502b4..5b3df46563a 100644 --- a/dev/sg/ci/subcommands.go +++ b/dev/sg/ci/subcommands.go @@ -318,7 +318,7 @@ sg ci build docker-images-patch-notest prometheus # Publish all images without testing sg ci build docker-images-candidates-notest `, - BashComplete: completions.CompleteOptions(getAllowedBuildTypeArgs), + BashComplete: completions.CompleteArgs(getAllowedBuildTypeArgs), Flags: []cli.Flag{ &ciPipelineFlag, &cli.StringFlag{ diff --git a/dev/sg/msp/repo/repo.go b/dev/sg/msp/repo/repo.go index 6b9bca0cc4f..52c38e2bcf0 100644 --- a/dev/sg/msp/repo/repo.go +++ b/dev/sg/msp/repo/repo.go @@ -73,25 +73,21 @@ func ServicesAndEnvironmentsCompletion() cli.BashCompleteFunc { if err != nil { return nil } - return func(c *cli.Context) { - switch c.Args().Len() { - case 0: // service not yet provided, try to complete service - completions.CompleteOptions(func() (options []string) { + return completions.CompletePositionalArgs( + func(args cli.Args) (options []string) { + services, _ := listServicesFromRoot(repoRoot) + return services + }, + func(args cli.Args) (options []string) { + svc, err := spec.Open(filepath.Join(repoRoot, ServiceYAMLPath(args.First()))) + if err != nil { + // try to complete services as a fallback services, _ := listServicesFromRoot(repoRoot) return services - })(c) - case 1: // service already provided, try to complete environment - completions.CompleteOptionsOnly(func() (options []string) { - svc, err := spec.Open(filepath.Join(repoRoot, ServiceYAMLPath(c.Args().First()))) - if err != nil { - // try to complete services as a fallback - services, _ := listServicesFromRoot(repoRoot) - return services - } - return svc.ListEnvironmentIDs() - })(c) - } - } + } + return svc.ListEnvironmentIDs() + }, + ) } func ServiceYAMLPath(serviceID string) string { diff --git a/dev/sg/msp/sg_msp.go b/dev/sg/msp/sg_msp.go index e69f04c447c..64574341285 100644 --- a/dev/sg/msp/sg_msp.go +++ b/dev/sg/msp/sg_msp.go @@ -48,9 +48,9 @@ func init() { // All 'sg msp ...' subcommands Command.Subcommands = []*cli.Command{ { - Name: "init", - ArgsUsage: "", - Description: "Initialize a template Managed Services Platform service spec", + Name: "init", + ArgsUsage: "", + Usage: "Initialize a template Managed Services Platform service spec", Flags: []cli.Flag{ &cli.StringFlag{ Name: "kind", @@ -186,11 +186,11 @@ func init() { }, }, { - Name: "generate", - Aliases: []string{"gen"}, - ArgsUsage: "", - Description: "Generate Terraform assets for a Managed Services Platform service spec.", - Usage: `Optionally use '-all' to sync all environments for a service. + Name: "generate", + Aliases: []string{"gen"}, + ArgsUsage: "", + Usage: "Generate Terraform assets for a Managed Services Platform service spec.", + Description: `Optionally use '-all' to sync all environments for a service. Supports completions on services and environments.`, UsageText: ` @@ -267,15 +267,15 @@ sg msp generate -all }, }, { - Name: "terraform-cloud", - Aliases: []string{"tfc"}, - Description: "Manage Terraform Cloud workspaces for a service", - Before: msprepo.UseManagedServicesRepo, + Name: "terraform-cloud", + Aliases: []string{"tfc"}, + Usage: "Manage Terraform Cloud workspaces for a service", + Before: msprepo.UseManagedServicesRepo, Subcommands: []*cli.Command{ { - Name: "sync", - Description: "Create or update all required Terraform Cloud workspaces for an environment", - Usage: `Optionally use '-all' to sync all environments for a service. + Name: "sync", + Usage: "Create or update all required Terraform Cloud workspaces for an environment", + Description: `Optionally use '-all' to sync all environments for a service. Supports completions on services and environments.`, ArgsUsage: " [environment ID]", @@ -366,8 +366,8 @@ Supports completions on services and environments.`, }, }, { - Name: "schema", - Description: "Generate JSON schema definition for service specification", + Name: "schema", + Usage: "Generate JSON schema definition for service specification", Flags: []cli.Flag{ &cli.StringFlag{ Name: "output", diff --git a/dev/sg/sg_generate.go b/dev/sg/sg_generate.go index 751ecf5120a..d22bf5acace 100644 --- a/dev/sg/sg_generate.go +++ b/dev/sg/sg_generate.go @@ -114,7 +114,7 @@ func (gt generateTargets) Commands() (cmds []*cli.Command) { for _, c := range gt { var complete cli.BashCompleteFunc if c.Completer != nil { - complete = completions.CompleteOptions(c.Completer) + complete = completions.CompleteArgs(c.Completer) } cmds = append(cmds, &cli.Command{ Name: c.Name, diff --git a/dev/sg/sg_lint.go b/dev/sg/sg_lint.go index 5fe97ef9874..312b1f726c8 100644 --- a/dev/sg/sg_lint.go +++ b/dev/sg/sg_lint.go @@ -174,7 +174,7 @@ func (lt lintTargets) Commands() (cmds []*cli.Command) { return runner.Check(cmd.Context, repoState) }, // Completions to chain multiple commands - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { for _, c := range lt { options = append(options, c.Name) } diff --git a/dev/sg/sg_live.go b/dev/sg/sg_live.go index c49f24d7553..bfac5a5d9b9 100644 --- a/dev/sg/sg_live.go +++ b/dev/sg/sg_live.go @@ -45,7 +45,7 @@ sg live -n 50 s2 Usage: "Number of commits to check for live version", }, }, - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { return append(environmentNames(), `https\://...`) }), } diff --git a/dev/sg/sg_run.go b/dev/sg/sg_run.go index 1be26be3925..a30fe02916e 100644 --- a/dev/sg/sg_run.go +++ b/dev/sg/sg_run.go @@ -67,7 +67,7 @@ sg run -describe jaeger }, }, Action: runExec, - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { config, _ := getConfig() if config == nil { return diff --git a/dev/sg/sg_secret.go b/dev/sg/sg_secret.go index 81a9534e92d..85948c74a1e 100644 --- a/dev/sg/sg_secret.go +++ b/dev/sg/sg_secret.go @@ -35,7 +35,7 @@ sg secret reset buildkite ArgsUsage: "<...key>", Usage: "Remove a specific secret from secrets file", Action: resetSecretExec, - BashComplete: completions.CompleteOptions(bashCompleteSecrets), + BashComplete: completions.CompleteArgs(bashCompleteSecrets), }, { Name: "list", diff --git a/dev/sg/sg_start.go b/dev/sg/sg_start.go index eacf9590824..db420cfbfcc 100644 --- a/dev/sg/sg_start.go +++ b/dev/sg/sg_start.go @@ -132,7 +132,7 @@ sg start -describe single-program Destination: &onlyServices, }, }, - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { config, _ := getConfig() if config == nil { return diff --git a/dev/sg/sg_tests.go b/dev/sg/sg_tests.go index e2302dd1dc7..1f1946de5b6 100644 --- a/dev/sg/sg_tests.go +++ b/dev/sg/sg_tests.go @@ -40,7 +40,7 @@ sg test -help sg test backend-integration -run TestSearch `, Category: category.Dev, - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { config, _ := getConfig() if config == nil { return diff --git a/go.mod b/go.mod index 2ed003d069f..96a0e1fed8f 100644 --- a/go.mod +++ b/go.mod @@ -437,7 +437,7 @@ require ( github.com/cockroachdb/redact v1.1.5 github.com/containerd/typeurl v1.0.2 // indirect github.com/coreos/go-iptables v0.6.0 - github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect + github.com/cpuguy83/go-md2man/v2 v2.0.3 // indirect github.com/dave/jennifer v1.6.1 // indirect github.com/di-wu/xsd-datetime v1.0.0 github.com/djherbis/buffer v1.2.0 // indirect diff --git a/go.sum b/go.sum index a15843a3b02..2408fbecfda 100644 --- a/go.sum +++ b/go.sum @@ -395,8 +395,9 @@ github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfc github.com/cpuguy83/go-md2man/v2 v2.0.0-20190314233015-f79a8a8ca69d/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cpuguy83/go-md2man/v2 v2.0.3 h1:qMCsGGgs+MAzDFyp9LpAe1Lqy/fY/qCovCm0qnXZOBM= +github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= diff --git a/lib/cliutil/completions/options.go b/lib/cliutil/completions/options.go index 2788bcd59e8..283d924421e 100644 --- a/lib/cliutil/completions/options.go +++ b/lib/cliutil/completions/options.go @@ -6,26 +6,63 @@ import ( "github.com/urfave/cli/v2" ) -// CompleteOptions provides autocompletions based on the options returned by -// generateOptions. generateOptions must not write to output, or reference any resources -// that are initialized elsewhere. -func CompleteOptions(generateOptions func() (options []string)) cli.BashCompleteFunc { - return func(cmd *cli.Context) { - for _, opt := range generateOptions() { - fmt.Fprintf(cmd.App.Writer, "%s\n", opt) - } - // Also render default completions to support flags - cli.DefaultCompleteWithFlags(cmd.Command)(cmd) +// defaultCompletions renders the default completions for a command. +func defaultCompletions() cli.BashCompleteFunc { + return func(c *cli.Context) { cli.DefaultCompleteWithFlags(c.Command)(c) } +} + +// CompleteArgs provides autocompletions based on the options returned by +// generateOptions. generateOptions is provided for every argument. +// +// generateOptions must not write to output, or reference any +// resources that are initialized elsewhere. +func CompleteArgs(generateOptions func() (options []string)) cli.BashCompleteFunc { + return func(c *cli.Context) { + CompletePositionalArgs(func(_ cli.Args) (options []string) { + return generateOptions() + })(c) } } -// CompleteOptionsOnly is like CompleteOptions, but does not render default -// completions - useful for generating completions for arguments beyond the first, -// where flag help is no longer needed. -func CompleteOptionsOnly(generateOptions func() (options []string)) cli.BashCompleteFunc { - return func(cmd *cli.Context) { - for _, opt := range generateOptions() { - fmt.Fprintf(cmd.App.Writer, "%s\n", opt) +// CompletePositionalArgs provides autocompletions for multiple positional +// arguments based on the options returned by the corresponding options generator. +// If there are more arguments than generators, no completion is provided. +// +// Each generator must not write to output, or reference any resources that are +// initialized elsewhere. +func CompletePositionalArgs(generators ...func(args cli.Args) (options []string)) cli.BashCompleteFunc { + return func(c *cli.Context) { + // Let default handler deal with flag completions if the latest argument + // looks like the start of a flag + if c.NArg() > 0 { + if f := c.Args().Get(c.NArg() - 1); f == "-" || f == "--" { + defaultCompletions()(c) + return + } + } + + // More arguments than positional options generators, we have no more + // completions to offer + if len(generators) <= c.NArg() { + return + } + + // Generate the options for this posarg + opts := generators[c.NArg()](c.Args()) + + // If there are no options, render the previous options if we can, as + // user may be typing the previous argument + if len(opts) == 0 && c.NArg() >= 1 { + opts = generators[c.NArg()-1](c.Args()) + } + + // Render the options + for _, opt := range opts { + fmt.Fprintf(c.App.Writer, "%s\n", opt) + } + // Also render default completions if there are no args yet + if c.Args().Len() == 0 { + defaultCompletions()(c) } } } diff --git a/monitoring/command/generate.go b/monitoring/command/generate.go index c5703abe94a..a859ad8dd07 100644 --- a/monitoring/command/generate.go +++ b/monitoring/command/generate.go @@ -107,7 +107,7 @@ func Generate(cmdRoot string, sgRoot string) *cli.Command { Usage: "If non-empty, indicates whether or not to generate multi-instance assets with the provided labels to group on. The standard per-instance monitoring assets will NOT be generated.", }, }, - BashComplete: completions.CompleteOptions(func() (options []string) { + BashComplete: completions.CompleteArgs(func() (options []string) { return definitions.Default().Names() }), Action: func(c *cli.Context) error {