Repository: helm/helm
Pull request #10513 (2021-12-31 16:52:34)

Allow fuzzy matching during shell completion for zsh and fish
**What this PR does / why we need it**:

This PR improves shell completion for `zsh` and `fish` by allowing for fuzzy matching.

We had made the assumption that when doing shell completion, we only needed choices that had for *prefix* what the user has typed. However, the `zsh` and `fish` shells have a more advanced matching system which first matches on prefix, but if no match is found, then does more advanced matching attempts, such as sub-strings; `fish` even matches
on descriptions of completions.

For example, with both `zsh` and `fish`:
```
$ bin/helm -n test status <TAB>
test-api          test-api-jwt      test-jaeger       test-node-api     test-nodelts-api
$ bin/helm  -n test status jaeger<TAB>
$ bin/helm  -n test status test-jaeger
```
As can be seen in the above example, the fuzzy matching allows the user to avoid typing (or completing) the prefix `test-` before reaching the differentiating part of the release name.  Instead they can simply type the differentiating string (`jaeger` in the example above) and the shell will figure out the rest.

Another example I expect to be common will be for namespace names.  In my clusters, the naming scheme uses prefixes to group namespaces, which means there are a lot of common prefixes.  For example:
```
$ bin/helm -n infra-<TAB>
infra-diagnostics     infra-test-prometheus    infra-test-auth   infra-test-prometheus-query-exporter
$ bin/helm -n infra-test-<TAB>
infra-test-prometheus    infra-test-auth   infra-test-prometheus-query-exporter
$ bin/helm -n infra-test-prometheus<TAB>
infra-test-prometheus    infra-test-prometheus-query-exporter
$ bin/helm -n infra-test-prometheus-q<TAB>
$ bin/helm -n infra-test-prometheus-query-exporter

# Instead, with fuzzy matching:
$ bin/helm -n quer<TAB>
$ bin/helm -n infra-test-prometheus-query-exporter
```
Such fuzzy matching can make completion even more efficient in cases where identical prefixes are common.

The PR allows fuzzy matching to work for the different helm completion situations, except for chart names which is a more complicated case and which I will leave for later.

**Special notes for your reviewer**:

The way to enable such fuzzy matching is simply to return all completion choices no matter what the user has already typed, or put another way, not to filter on `toComplete`.  After that, it is the shell that deals with proper filtering.

The completion scripts provided by Cobra are prepared for such an approach.  For `bash` and `powershell` which don't support fuzzy matching, the completion script itself does the filtering on prefix.  There is therefore no need for helm to pre-filter as we are currently doing.


marckhouzam: I added some new tests to the PR, to make sure we don't regress this feature in the future.
marckhouzam: > This looks great. As a zsh user, this is going to make my life easier.

Thanks for the quick review!

Patch:
diff --git a/cmd/helm/docs.go b/cmd/helm/docs.go
index 1a28a47ec..ef64d41a5 100644
--- a/cmd/helm/docs.go
+++ b/cmd/helm/docs.go
@@ -69,14 +69,7 @@ func newDocsCmd(out io.Writer) *cobra.Command {
 	f.BoolVar(&o.generateHeaders, "generate-headers", false, "generate standard headers for markdown files")
 
 	cmd.RegisterFlagCompletionFunc("type", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		types := []string{"bash", "man", "markdown"}
-		var comps []string
-		for _, t := range types {
-			if strings.HasPrefix(t, toComplete) {
-				comps = append(comps, t)
-			}
-		}
-		return comps, cobra.ShellCompDirectiveNoFileComp
+		return []string{"bash", "man", "markdown"}, cobra.ShellCompDirectiveNoFileComp
 	})
 
 	return cmd
diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go
index aefa836c7..6a59101b7 100644
--- a/cmd/helm/flags.go
+++ b/cmd/helm/flags.go
@@ -69,9 +69,7 @@ func bindOutputFlag(cmd *cobra.Command, varRef *output.Format) {
 	err := cmd.RegisterFlagCompletionFunc(outputFlag, func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		var formatNames []string
 		for format, desc := range output.FormatsWithDesc() {
-			if strings.HasPrefix(format, toComplete) {
-				formatNames = append(formatNames, fmt.Sprintf("%s\t%s", format, desc))
-			}
+			formatNames = append(formatNames, fmt.Sprintf("%s\t%s", format, desc))
 		}
 
 		// Sort the results to get a deterministic order for the tests
@@ -153,24 +151,21 @@ func compVersionFlag(chartRef string, toComplete string) ([]string, cobra.ShellC
 	var versions []string
 	if indexFile, err := repo.LoadIndexFile(path); err == nil {
 		for _, details := range indexFile.Entries[chartName] {
-			version := details.Metadata.Version
-			if strings.HasPrefix(version, toComplete) {
-				appVersion := details.Metadata.AppVersion
-				appVersionDesc := ""
-				if appVersion != "" {
-					appVersionDesc = fmt.Sprintf("App: %s, ", appVersion)
-				}
-				created := details.Created.Format("January 2, 2006")
-				createdDesc := ""
-				if created != "" {
-					createdDesc = fmt.Sprintf("Created: %s ", created)
-				}
-				deprecated := ""
-				if details.Metadata.Deprecated {
-					deprecated = "(deprecated)"
-				}
-				versions = append(versions, fmt.Sprintf("%s\t%s%s%s", version, appVersionDesc, createdDesc, deprecated))
+			appVersion := details.Metadata.AppVersion
+			appVersionDesc := ""
+			if appVersion != "" {
+				appVersionDesc = fmt.Sprintf("App: %s, ", appVersion)
+			}
+			created := details.Created.Format("January 2, 2006")
+			createdDesc := ""
+			if created != "" {
+				createdDesc = fmt.Sprintf("Created: %s ", created)
+			}
+			deprecated := ""
+			if details.Metadata.Deprecated {
+				deprecated = "(deprecated)"
 			}
+			versions = append(versions, fmt.Sprintf("%s\t%s%s%s", details.Metadata.Version, appVersionDesc, createdDesc, deprecated))
 		}
 	}
 
diff --git a/cmd/helm/history.go b/cmd/helm/history.go
index 06ec07d6d..ee6f391e4 100644
--- a/cmd/helm/history.go
+++ b/cmd/helm/history.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"io"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/gosuri/uitable"
@@ -191,12 +190,9 @@ func compListRevisions(toComplete string, cfg *action.Configuration, releaseName
 	var revisions []string
 	if hist, err := client.Run(releaseName); err == nil {
 		for _, release := range hist {
-			version := strconv.Itoa(release.Version)
-			if strings.HasPrefix(version, toComplete) {
-				appVersion := fmt.Sprintf("App: %s", release.Chart.Metadata.AppVersion)
-				chartDesc := fmt.Sprintf("Chart: %s-%s", release.Chart.Metadata.Name, release.Chart.Metadata.Version)
-				revisions = append(revisions, fmt.Sprintf("%s\t%s, %s", version, appVersion, chartDesc))
-			}
+			appVersion := fmt.Sprintf("App: %s", release.Chart.Metadata.AppVersion)
+			chartDesc := fmt.Sprintf("Chart: %s-%s", release.Chart.Metadata.Name, release.Chart.Metadata.Version)
+			revisions = append(revisions, fmt.Sprintf("%s\t%s, %s", strconv.Itoa(release.Version), appVersion, chartDesc))
 		}
 		return revisions, cobra.ShellCompDirectiveNoFileComp
 	}
diff --git a/cmd/helm/list.go b/cmd/helm/list.go
index c361b550d..2f0e3da0b 100644
--- a/cmd/helm/list.go
+++ b/cmd/helm/list.go
@@ -224,7 +224,14 @@ func compListReleases(toComplete string, ignoredReleaseNames []string, cfg *acti
 	client := action.NewList(cfg)
 	client.All = true
 	client.Limit = 0
-	client.Filter = fmt.Sprintf("^%s", toComplete)
+	// Do not filter so as to get the entire list of releases.
+	// This will allow zsh and fish to match completion choices
+	// on other criteria then prefix.  For example:
+	//   helm status ingress<TAB>
+	// can match
+	//   helm status nginx-ingress
+	//
+	// client.Filter = fmt.Sprintf("^%s", toComplete)
 
 	client.SetStateMask()
 	releases, err := client.Run()
diff --git a/cmd/helm/plugin_list.go b/cmd/helm/plugin_list.go
index 6c3926a9d..ddf01f6f2 100644
--- a/cmd/helm/plugin_list.go
+++ b/cmd/helm/plugin_list.go
@@ -18,7 +18,6 @@ package main
 import (
 	"fmt"
 	"io"
-	"strings"
 
 	"github.com/gosuri/uitable"
 	"github.com/spf13/cobra"
@@ -82,9 +81,7 @@ func compListPlugins(toComplete string, ignoredPluginNames []string) []string {
 	if err == nil && len(plugins) > 0 {
 		filteredPlugins := filterPlugins(plugins, ignoredPluginNames)
 		for _, p := range filteredPlugins {
-			if strings.HasPrefix(p.Metadata.Name, toComplete) {
-				pNames = append(pNames, fmt.Sprintf("%s\t%s", p.Metadata.Name, p.Metadata.Usage))
-			}
+			pNames = append(pNames, fmt.Sprintf("%s\t%s", p.Metadata.Name, p.Metadata.Usage))
 		}
 	}
 	return pNames
diff --git a/cmd/helm/repo_list.go b/cmd/helm/repo_list.go
index 537f8bd2c..efaf74155 100644
--- a/cmd/helm/repo_list.go
+++ b/cmd/helm/repo_list.go
@@ -19,7 +19,6 @@ package main
 import (
 	"fmt"
 	"io"
-	"strings"
 
 	"github.com/gosuri/uitable"
 	"github.com/pkg/errors"
@@ -131,9 +130,7 @@ func compListRepos(prefix string, ignoredRepoNames []string) []string {
 	if err == nil && len(f.Repositories) > 0 {
 		filteredRepos := filterRepos(f.Repositories, ignoredRepoNames)
 		for _, repo := range filteredRepos {
-			if strings.HasPrefix(repo.Name, prefix) {
-				rNames = append(rNames, fmt.Sprintf("%s\t%s", repo.Name, repo.URL))
-			}
+			rNames = append(rNames, fmt.Sprintf("%s\t%s", repo.Name, repo.URL))
 		}
 	}
 	return rNames
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index 0de4a738a..be72bbb84 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -106,9 +106,7 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 			nsNames := []string{}
 			if namespaces, err := client.CoreV1().Namespaces().List(context.Background(), metav1.ListOptions{TimeoutSeconds: &to}); err == nil {
 				for _, ns := range namespaces.Items {
-					if strings.HasPrefix(ns.Name, toComplete) {
-						nsNames = append(nsNames, ns.Name)
-					}
+					nsNames = append(nsNames, ns.Name)
 				}
 				return nsNames, cobra.ShellCompDirectiveNoFileComp
 			}
@@ -133,9 +131,7 @@ func newRootCmd(actionConfig *action.Configuration, out io.Writer, args []string
 			&clientcmd.ConfigOverrides{}).RawConfig(); err == nil {
 			comps := []string{}
 			for name, context := range config.Contexts {
-				if strings.HasPrefix(name, toComplete) {
-					comps = append(comps, fmt.Sprintf("%s\t%s", name, context.Cluster))
-				}
+				comps = append(comps, fmt.Sprintf("%s\t%s", name, context.Cluster))
 			}
 			return comps, cobra.ShellCompDirectiveNoFileComp
 		}
diff --git a/cmd/helm/search_repo.go b/cmd/helm/search_repo.go
index ae60292a6..34232fcfa 100644
--- a/cmd/helm/search_repo.go
+++ b/cmd/helm/search_repo.go
@@ -312,8 +312,9 @@ func compListCharts(toComplete string, includeFiles bool) ([]string, cobra.Shell
 		}
 		repoWithSlash := fmt.Sprintf("%s/", repo)
 		if strings.HasPrefix(toComplete, repoWithSlash) {
-			// Must complete with charts within the specified repo
-			completions = append(completions, compListChartsOfRepo(repo, toComplete)...)
+			// Must complete with charts within the specified repo.
+			// Don't filter on toComplete to allow for shell fuzzy matching
+			completions = append(completions, compListChartsOfRepo(repo, "")...)
 			noSpace = false
 			break
 		} else if strings.HasPrefix(repo, toComplete) {


Test patch:
diff --git a/cmd/helm/docs_test.go b/cmd/helm/docs_test.go
index f0082578a..fe5864d5e 100644
--- a/cmd/helm/docs_test.go
+++ b/cmd/helm/docs_test.go
@@ -26,9 +26,9 @@ func TestDocsTypeFlagCompletion(t *testing.T) {
 		cmd:    "__complete docs --type ''",
 		golden: "output/docs-type-comp.txt",
 	}, {
-		name:   "completion for docs --type",
+		name:   "completion for docs --type, no filter",
 		cmd:    "__complete docs --type mar",
-		golden: "output/docs-type-filtered-comp.txt",
+		golden: "output/docs-type-comp.txt",
 	}}
 	runTestCmd(t, tests)
 }
diff --git a/cmd/helm/flags_test.go b/cmd/helm/flags_test.go
index d5576fe9f..07d28c460 100644
--- a/cmd/helm/flags_test.go
+++ b/cmd/helm/flags_test.go
@@ -83,6 +83,13 @@ func outputFlagCompletionTest(t *testing.T, cmdName string) {
 		rels: releasesMockWithStatus(&release.Info{
 			Status: release.StatusDeployed,
 		}),
+	}, {
+		name:   "completion for output flag, no filter",
+		cmd:    fmt.Sprintf("__complete %s --output jso", cmdName),
+		golden: "output/output-comp.txt",
+		rels: releasesMockWithStatus(&release.Info{
+			Status: release.StatusDeployed,
+		}),
 	}}
 	runTestCmd(t, tests)
 }
diff --git a/cmd/helm/history_test.go b/cmd/helm/history_test.go
index 2663e9ee9..07f2d85df 100644
--- a/cmd/helm/history_test.go
+++ b/cmd/helm/history_test.go
@@ -95,6 +95,11 @@ func revisionFlagCompletionTest(t *testing.T, cmdName string) {
 		cmd:    fmt.Sprintf("__complete %s musketeers --revision ''", cmdName),
 		rels:   releases,
 		golden: "output/revision-comp.txt",
+	}, {
+		name:   "completion for revision flag, no filter",
+		cmd:    fmt.Sprintf("__complete %s musketeers --revision 1", cmdName),
+		rels:   releases,
+		golden: "output/revision-comp.txt",
 	}, {
 		name:   "completion for revision flag with too few args",
 		cmd:    fmt.Sprintf("__complete %s --revision ''", cmdName),
diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go
index ff025b809..df6e9af79 100644
--- a/cmd/helm/install_test.go
+++ b/cmd/helm/install_test.go
@@ -275,6 +275,10 @@ func TestInstallVersionCompletion(t *testing.T) {
 		name:   "completion for install version flag with generate-name",
 		cmd:    fmt.Sprintf("%s __complete install --generate-name testing/alpine --version ''", repoSetup),
 		golden: "output/version-comp.txt",
+	}, {
+		name:   "completion for install version flag, no filter",
+		cmd:    fmt.Sprintf("%s __complete install releasename testing/alpine --version 0.3", repoSetup),
+		golden: "output/version-comp.txt",
 	}, {
 		name:   "completion for install version flag too few args",
 		cmd:    fmt.Sprintf("%s __complete install testing/alpine --version ''", repoSetup),
diff --git a/cmd/helm/plugin_test.go b/cmd/helm/plugin_test.go
index 4fb834189..33de33522 100644
--- a/cmd/helm/plugin_test.go
+++ b/cmd/helm/plugin_test.go
@@ -307,6 +307,11 @@ func TestPluginCmdsCompletion(t *testing.T) {
 		cmd:    "__complete plugin update ''",
 		golden: "output/plugin_list_comp.txt",
 		rels:   []*release.Release{},
+	}, {
+		name:   "completion for plugin update, no filter",
+		cmd:    "__complete plugin update full",
+		golden: "output/plugin_list_comp.txt",
+		rels:   []*release.Release{},
 	}, {
 		name:   "completion for plugin update repetition",
 		cmd:    "__complete plugin update args ''",
@@ -317,6 +322,11 @@ func TestPluginCmdsCompletion(t *testing.T) {
 		cmd:    "__complete plugin uninstall ''",
 		golden: "output/plugin_list_comp.txt",
 		rels:   []*release.Release{},
+	}, {
+		name:   "completion for plugin uninstall, no filter",
+		cmd:    "__complete plugin uninstall full",
+		golden: "output/plugin_list_comp.txt",
+		rels:   []*release.Release{},
 	}, {
 		name:   "completion for plugin uninstall repetition",
 		cmd:    "__complete plugin uninstall args ''",
diff --git a/cmd/helm/pull_test.go b/cmd/helm/pull_test.go
index 4d86a5029..901557bd2 100644
--- a/cmd/helm/pull_test.go
+++ b/cmd/helm/pull_test.go
@@ -371,6 +371,10 @@ func TestPullVersionCompletion(t *testing.T) {
 		name:   "completion for pull version flag",
 		cmd:    fmt.Sprintf("%s __complete pull testing/alpine --version ''", repoSetup),
 		golden: "output/version-comp.txt",
+	}, {
+		name:   "completion for pull version flag, no filter",
+		cmd:    fmt.Sprintf("%s __complete pull testing/alpine --version 0.3", repoSetup),
+		golden: "output/version-comp.txt",
 	}, {
 		name:   "completion for pull version flag too few args",
 		cmd:    fmt.Sprintf("%s __complete pull --version ''", repoSetup),
diff --git a/cmd/helm/repo_remove_test.go b/cmd/helm/repo_remove_test.go
index d9e77530f..768295655 100644
--- a/cmd/helm/repo_remove_test.go
+++ b/cmd/helm/repo_remove_test.go
@@ -197,6 +197,10 @@ func TestRepoRemoveCompletion(t *testing.T) {
 		name:   "completion for repo remove",
 		cmd:    fmt.Sprintf("%s __completeNoDesc repo remove ''", repoSetup),
 		golden: "output/repo_list_comp.txt",
+	}, {
+		name:   "completion for repo remove, no filter",
+		cmd:    fmt.Sprintf("%s __completeNoDesc repo remove fo", repoSetup),
+		golden: "output/repo_list_comp.txt",
 	}, {
 		name:   "completion for repo remove repetition",
 		cmd:    fmt.Sprintf("%s __completeNoDesc repo remove foo ''", repoSetup),
diff --git a/cmd/helm/show_test.go b/cmd/helm/show_test.go
index 8dba0aea4..2ecb80a43 100644
--- a/cmd/helm/show_test.go
+++ b/cmd/helm/show_test.go
@@ -98,6 +98,10 @@ func TestShowVersionCompletion(t *testing.T) {
 		name:   "completion for show version flag",
 		cmd:    fmt.Sprintf("%s __complete show chart testing/alpine --version ''", repoSetup),
 		golden: "output/version-comp.txt",
+	}, {
+		name:   "completion for show version flag, no filter",
+		cmd:    fmt.Sprintf("%s __complete show chart testing/alpine --version 0.3", repoSetup),
+		golden: "output/version-comp.txt",
 	}, {
 		name:   "completion for show version flag too few args",
 		cmd:    fmt.Sprintf("%s __complete show chart --version ''", repoSetup),
diff --git a/cmd/helm/testdata/output/docs-type-filtered-comp.txt b/cmd/helm/testdata/output/docs-type-filtered-comp.txt
deleted file mode 100644
index 55104f32e..000000000
--- a/cmd/helm/testdata/output/docs-type-filtered-comp.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-markdown
-:4
-Completion ended with directive: ShellCompDirectiveNoFileComp
diff --git a/cmd/helm/testdata/output/status-comp.txt b/cmd/helm/testdata/output/status-comp.txt
index 4f56ab30a..4c408c974 100644
--- a/cmd/helm/testdata/output/status-comp.txt
+++ b/cmd/helm/testdata/output/status-comp.txt
@@ -1,4 +1,5 @@
 aramis	Aramis-chart-0.0.0 -> uninstalled
 athos	Athos-chart-1.2.3 -> deployed
+porthos	Porthos-chart-111.222.333 -> failed
 :4
 Completion ended with directive: ShellCompDirectiveNoFileComp
diff --git a/cmd/helm/upgrade_test.go b/cmd/helm/upgrade_test.go
index fc2a22d7d..8afcb139b 100644
--- a/cmd/helm/upgrade_test.go
+++ b/cmd/helm/upgrade_test.go
@@ -406,6 +406,10 @@ func TestUpgradeVersionCompletion(t *testing.T) {
 		name:   "completion for upgrade version flag",
 		cmd:    fmt.Sprintf("%s __complete upgrade releasename testing/alpine --version ''", repoSetup),
 		golden: "output/version-comp.txt",
+	}, {
+		name:   "completion for upgrade version flag, no filter",
+		cmd:    fmt.Sprintf("%s __complete upgrade releasename testing/alpine --version 0.3", repoSetup),
+		golden: "output/version-comp.txt",
 	}, {
 		name:   "completion for upgrade version flag too few args",
 		cmd:    fmt.Sprintf("%s __complete upgrade releasename --version ''", repoSetup),