ref(cmd): Implement flag parsing for output format

Signed-off-by: Taylor Thomas <taylor.thomas@microsoft.com>

cmd/helm/flags.go

@@ -49,10 +49,34 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) {
 }
 
 // bindOutputFlag will add the output flag to the given command and bind the
-// value to the given string pointer
-func bindOutputFlag(cmd *cobra.Command, varRef *string) {
-	// NOTE(taylor): A possible refactor here is that we can implement all the
-	// validation for the OutputFormat type here so we don't have to do the
-	// parsing and checking in the command
-	cmd.Flags().StringVarP(varRef, outputFlag, "o", output.Table.String(), fmt.Sprintf("prints the output in the specified format. Allowed values: %s, %s, %s", output.Table, output.JSON, output.YAML))
+// value to the given format pointer
+func bindOutputFlag(cmd *cobra.Command, varRef *output.Format) {
+	cmd.Flags().VarP(newOutputValue(output.Table, varRef), outputFlag, "o", fmt.Sprintf("prints the output in the specified format. Allowed values: %s, %s, %s", output.Table, output.JSON, output.YAML))
+}
+
+type outputValue output.Format
+
+func newOutputValue(defaultValue output.Format, p *output.Format) *outputValue {
+	*p = defaultValue
+	return (*outputValue)(p)
+}
+
+func (o *outputValue) String() string {
+	// It is much cleaner looking (and technically less allocations) to just
+	// convert to a string rather than type asserting to the underlying
+	// output.Format
+	return string(*o)
+}
+
+func (o *outputValue) Type() string {
+	return "format"
+}
+
+func (o *outputValue) Set(s string) error {
+	outfmt, err := output.ParseFormat(s)
+	if err != nil {
+		return err
+	}
+	*o = outputValue(outfmt)
+	return nil
 }

cmd/helm/history.go

@@ -50,6 +50,7 @@ The historical release set is printed as a formatted table, e.g:
 
 func newHistoryCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewHistory(cfg)
+	var outfmt output.Format
 
 	cmd := &cobra.Command{
 		Use:     "history RELEASE_NAME",
@@ -58,25 +59,18 @@ func newHistoryCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Aliases: []string{"hist"},
 		Args:    require.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			output, err := output.ParseFormat(client.OutputFormat)
-			if err != nil {
-				return err
-			}
-
 			history, err := getHistory(client, args[0])
 			if err != nil {
 				return err
 			}
 
-			return output.Write(out, history)
+			return outfmt.Write(out, history)
 		},
 	}
 
 	f := cmd.Flags()
 	f.IntVar(&client.Max, "max", 256, "maximum number of revision to include in history")
-	bindOutputFlag(cmd, &client.OutputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

cmd/helm/install.go

@@ -105,6 +105,7 @@ charts in a repository, use 'helm search'.
 func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewInstall(cfg)
 	valueOpts := &values.Options{}
+	var outfmt output.Format
 
 	cmd := &cobra.Command{
 		Use:   "install [NAME] [CHART]",
@@ -112,24 +113,17 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Long:  installDesc,
 		Args:  require.MinimumNArgs(1),
 		RunE: func(_ *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			output, err := output.ParseFormat(client.OutputFormat)
-			if err != nil {
-				return err
-			}
-
 			rel, err := runInstall(args, client, valueOpts, out)
 			if err != nil {
 				return err
 			}
 
-			return output.Write(out, &statusPrinter{rel, settings.Debug})
+			return outfmt.Write(out, &statusPrinter{rel, settings.Debug})
 		},
 	}
 
 	addInstallFlags(cmd.Flags(), client, valueOpts)
-	bindOutputFlag(cmd, &client.OutputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

cmd/helm/list.go

@@ -59,6 +59,7 @@ flag with the '--offset' flag allows you to page through results.
 
 func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewList(cfg)
+	var outfmt output.Format
 
 	cmd := &cobra.Command{
 		Use:     "list",
@@ -67,13 +68,6 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Aliases: []string{"ls"},
 		Args:    require.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			output, err := output.ParseFormat(client.OutputFormat)
-			if err != nil {
-				return err
-			}
-
 			if client.AllNamespaces {
 				initActionConfig(cfg, true)
 			}
@@ -88,7 +82,7 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 				return err
 			}
 
-			return output.Write(out, newReleaseListWriter(results))
+			return outfmt.Write(out, newReleaseListWriter(results))
 		},
 	}
 
@@ -107,7 +101,7 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.IntVarP(&client.Limit, "max", "m", 256, "maximum number of releases to fetch")
 	f.IntVar(&client.Offset, "offset", 0, "next release name in the list, used to offset from start value")
 	f.StringVarP(&client.Filter, "filter", "f", "", "a regular expression (Perl compatible). Any releases that match the expression will be included in the results")
-	bindOutputFlag(cmd, &client.OutputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

cmd/helm/repo_list.go

@@ -29,18 +29,12 @@ import (
 )
 
 func newRepoListCmd(out io.Writer) *cobra.Command {
-	var outputFormat string
+	var outfmt output.Format
 	cmd := &cobra.Command{
 		Use:   "list",
 		Short: "list chart repositories",
 		Args:  require.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			outfmt, err := output.ParseFormat(outputFormat)
-			if err != nil {
-				return err
-			}
 			f, err := repo.LoadFile(settings.RepositoryConfig)
 			if isNotExist(err) || len(f.Repositories) == 0 {
 				return errors.New("no repositories to show")
@@ -50,7 +44,7 @@ func newRepoListCmd(out io.Writer) *cobra.Command {
 		},
 	}
 
-	bindOutputFlag(cmd, &outputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

cmd/helm/search_hub.go

@@ -44,7 +44,7 @@ Helm Hub. You can find it at https://github.com/helm/monocular
 type searchHubOptions struct {
 	searchEndpoint string
 	maxColWidth    uint
-	outputFormat   string
+	outputFormat   output.Format
 }
 
 func newSearchHubCmd(out io.Writer) *cobra.Command {
@@ -68,13 +68,6 @@ func newSearchHubCmd(out io.Writer) *cobra.Command {
 }
 
 func (o *searchHubOptions) run(out io.Writer, args []string) error {
-	// validate the output format first so we don't waste time running a
-	// request that we'll throw away
-	outfmt, err := output.ParseFormat(o.outputFormat)
-	if err != nil {
-		return err
-	}
-
 	c, err := monocular.New(o.searchEndpoint)
 	if err != nil {
 		return errors.Wrap(err, fmt.Sprintf("unable to create connection to %q", o.searchEndpoint))
@@ -87,7 +80,7 @@ func (o *searchHubOptions) run(out io.Writer, args []string) error {
 		return fmt.Errorf("unable to perform search against %q", o.searchEndpoint)
 	}
 
-	return outfmt.Write(out, newHubSearchWriter(results, o.searchEndpoint, o.maxColWidth))
+	return o.outputFormat.Write(out, newHubSearchWriter(results, o.searchEndpoint, o.maxColWidth))
 }
 
 type hubChartElement struct {

cmd/helm/search_repo.go

@@ -51,7 +51,7 @@ type searchRepoOptions struct {
 	maxColWidth  uint
 	repoFile     string
 	repoCacheDir string
-	outputFormat string
+	outputFormat output.Format
 }
 
 func newSearchRepoCmd(out io.Writer) *cobra.Command {
@@ -79,13 +79,6 @@ func newSearchRepoCmd(out io.Writer) *cobra.Command {
 }
 
 func (o *searchRepoOptions) run(out io.Writer, args []string) error {
-	// validate the output format first so we don't waste time running a
-	// request that we'll throw away
-	outfmt, err := output.ParseFormat(o.outputFormat)
-	if err != nil {
-		return err
-	}
-
 	index, err := o.buildIndex(out)
 	if err != nil {
 		return err
@@ -108,7 +101,7 @@ func (o *searchRepoOptions) run(out io.Writer, args []string) error {
 		return err
 	}
 
-	return outfmt.Write(out, &repoSearchWriter{data, o.maxColWidth})
+	return o.outputFormat.Write(out, &repoSearchWriter{data, o.maxColWidth})
 }
 
 func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Result, error) {

cmd/helm/status.go

@@ -44,6 +44,7 @@ The status consists of:
 
 func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewStatus(cfg)
+	var outfmt output.Format
 
 	cmd := &cobra.Command{
 		Use:   "status RELEASE_NAME",
@@ -51,13 +52,6 @@ func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Long:  statusHelp,
 		Args:  require.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			outfmt, err := output.ParseFormat(client.OutputFormat)
-			if err != nil {
-				return err
-			}
-
 			rel, err := client.Run(args[0])
 			if err != nil {
 				return err
@@ -72,7 +66,7 @@ func newStatusCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 
 	f := cmd.PersistentFlags()
 	f.IntVar(&client.Version, "revision", 0, "if set, display the status of the named release with revision")
-	bindOutputFlag(cmd, &client.OutputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

cmd/helm/upgrade.go

@@ -63,6 +63,7 @@ set for a key called 'foo', the 'newbar' value would take precedence:
 func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	client := action.NewUpgrade(cfg)
 	valueOpts := &values.Options{}
+	var outfmt output.Format
 
 	cmd := &cobra.Command{
 		Use:   "upgrade [RELEASE] [CHART]",
@@ -70,13 +71,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Long:  upgradeDesc,
 		Args:  require.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			// validate the output format first so we don't waste time running a
-			// request that we'll throw away
-			outfmt, err := output.ParseFormat(client.OutputFormat)
-			if err != nil {
-				return err
-			}
-
 			client.Namespace = getNamespace()
 
 			if client.Version == "" && client.Devel {
@@ -164,7 +158,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.BoolVar(&client.SubNotes, "render-subchart-notes", false, "if set, render subchart notes along with the parent")
 	addChartPathOptionsFlags(f, &client.ChartPathOptions)
 	addValueOptionsFlags(f, valueOpts)
-	bindOutputFlag(cmd, &client.OutputFormat)
+	bindOutputFlag(cmd, &outfmt)
 
 	return cmd
 }

pkg/action/history.go

@@ -28,8 +28,8 @@ import (
 type History struct {
 	cfg *Configuration
 
-	Max          int
-	OutputFormat string
+	Max     int
+	Version int
 }
 
 // NewHistory creates a new History object with the given configuration.

pkg/action/install.go

@@ -82,7 +82,6 @@ type Install struct {
 	OutputDir        string
 	Atomic           bool
 	SkipCRDs         bool
-	OutputFormat     string
 	SubNotes         bool
 }
 

pkg/action/list.go

@@ -125,7 +125,6 @@ type List struct {
 	Deployed     bool
 	Failed       bool
 	Pending      bool
-	OutputFormat string
 }
 
 // NewList constructs a new *List

pkg/action/status.go

@@ -26,8 +26,7 @@ import (
 type Status struct {
 	cfg *Configuration
 
-	Version      int
-	OutputFormat string
+	Version int
 }
 
 // NewStatus creates a new Status object with the given configuration.

pkg/action/upgrade.go

@@ -57,7 +57,6 @@ type Upgrade struct {
 	MaxHistory    int
 	Atomic        bool
 	CleanupOnFail bool
-	OutputFormat  string
 	SubNotes      bool
 }