From e0886ddb7d72802a8ef0d44c0a115f502e47ad4f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 7 Feb 2024 10:49:03 +0100 Subject: [PATCH 1/2] Make sure grouped flags are added to the command flag set --- bundle/run/options.go | 2 ++ libs/cmdgroup/command.go | 28 ++++++++++++++++++++++++++++ libs/cmdgroup/command_test.go | 7 +++++++ libs/cmdgroup/template.go | 4 ++-- 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/bundle/run/options.go b/bundle/run/options.go index 580612d0ea..1f47712e23 100644 --- a/bundle/run/options.go +++ b/bundle/run/options.go @@ -23,4 +23,6 @@ func (o *Options) Define(cmd *cobra.Command) { pipelineGroup := wrappedCmd.AddFlagGroup("Pipeline") o.Pipeline.Define(pipelineGroup.FlagSet()) + + wrappedCmd.RefreshFlags() } diff --git a/libs/cmdgroup/command.go b/libs/cmdgroup/command.go index 19c9af16af..ad3ba6efa2 100644 --- a/libs/cmdgroup/command.go +++ b/libs/cmdgroup/command.go @@ -15,6 +15,12 @@ type CommandWithGroupFlag struct { flagGroups []*FlagGroup } +func (c *CommandWithGroupFlag) RefreshFlags() { + for _, fg := range c.flagGroups { + c.cmd.Flags().AddFlagSet(fg.flagSet) + } +} + func (c *CommandWithGroupFlag) Command() *cobra.Command { return c.cmd } @@ -23,6 +29,24 @@ func (c *CommandWithGroupFlag) FlagGroups() []*FlagGroup { return c.flagGroups } +func (c *CommandWithGroupFlag) NonGroupedFlags() *pflag.FlagSet { + nonGrouped := pflag.NewFlagSet("non-grouped", pflag.ContinueOnError) + c.cmd.LocalFlags().VisitAll(func(f *pflag.Flag) { + for _, fg := range c.flagGroups { + if fg.Has(f) { + return + } + } + nonGrouped.AddFlag(f) + }) + + return nonGrouped +} + +func (c *CommandWithGroupFlag) HasNonGroupedFlags() bool { + return c.NonGroupedFlags().HasFlags() +} + func NewCommandWithGroupFlag(cmd *cobra.Command) *CommandWithGroupFlag { cmdWithFlagGroups := &CommandWithGroupFlag{cmd: cmd, flagGroups: make([]*FlagGroup, 0)} cmd.SetUsageFunc(func(c *cobra.Command) error { @@ -64,6 +88,10 @@ func (c *FlagGroup) FlagSet() *pflag.FlagSet { return c.flagSet } +func (c *FlagGroup) Has(f *pflag.Flag) bool { + return c.flagSet.Lookup(f.Name) != nil +} + var templateFuncs = template.FuncMap{ "trim": strings.TrimSpace, "trimRightSpace": trimRightSpace, diff --git a/libs/cmdgroup/command_test.go b/libs/cmdgroup/command_test.go index 2eae31d14e..ea33e66650 100644 --- a/libs/cmdgroup/command_test.go +++ b/libs/cmdgroup/command_test.go @@ -29,6 +29,7 @@ func TestCommandFlagGrouping(t *testing.T) { fs.String("pipeline-type", "", "Type of the pipeline") cmd.Flags().BoolP("bool", "b", false, "Bool flag") + wrappedCmd.RefreshFlags() buf := bytes.NewBuffer(nil) cmd.SetOutput(buf) @@ -48,4 +49,10 @@ Pipeline Flags: Flags: -b, --bool Bool flag` require.Equal(t, expected, buf.String()) + + require.NotNil(t, cmd.Flags().Lookup("job-name")) + require.NotNil(t, cmd.Flags().Lookup("job-type")) + require.NotNil(t, cmd.Flags().Lookup("pipeline-name")) + require.NotNil(t, cmd.Flags().Lookup("pipeline-type")) + require.NotNil(t, cmd.Flags().Lookup("bool")) } diff --git a/libs/cmdgroup/template.go b/libs/cmdgroup/template.go index aac967b0ee..5c1be48fb4 100644 --- a/libs/cmdgroup/template.go +++ b/libs/cmdgroup/template.go @@ -7,8 +7,8 @@ const usageTemplate = `Usage:{{if .Command.Runnable}} {{.Description}}{{end}} {{.FlagSet.FlagUsages | trimTrailingWhitespaces}} {{end}} -{{if .Command.HasAvailableLocalFlags}}Flags: -{{.Command.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .Command.HasAvailableInheritedFlags}} +{{if .HasNonGroupedFlags}}Flags: +{{.NonGroupedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{if .Command.HasAvailableInheritedFlags}} Global Flags: {{.Command.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}` From 69f1d9a6dbf5e5e0131e92f5c0be59c3d815a9b3 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 7 Feb 2024 11:20:21 +0100 Subject: [PATCH 2/2] refactor --- bundle/run/options.go | 12 +++++++----- libs/cmdgroup/command.go | 15 ++++++--------- libs/cmdgroup/command_test.go | 7 ++++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/bundle/run/options.go b/bundle/run/options.go index 1f47712e23..4e50788a9b 100644 --- a/bundle/run/options.go +++ b/bundle/run/options.go @@ -12,17 +12,19 @@ type Options struct { } func (o *Options) Define(cmd *cobra.Command) { - wrappedCmd := cmdgroup.NewCommandWithGroupFlag(cmd) - jobGroup := wrappedCmd.AddFlagGroup("Job") + jobGroup := cmdgroup.NewFlagGroup("Job") o.Job.DefineJobOptions(jobGroup.FlagSet()) - jobTaskGroup := wrappedCmd.AddFlagGroup("Job Task") + jobTaskGroup := cmdgroup.NewFlagGroup("Job Task") jobTaskGroup.SetDescription(`Note: please prefer use of job-level parameters (--param) over task-level parameters. For more information, see https://docs.databricks.com/en/workflows/jobs/create-run-jobs.html#pass-parameters-to-a-databricks-job-task`) o.Job.DefineTaskOptions(jobTaskGroup.FlagSet()) - pipelineGroup := wrappedCmd.AddFlagGroup("Pipeline") + pipelineGroup := cmdgroup.NewFlagGroup("Pipeline") o.Pipeline.Define(pipelineGroup.FlagSet()) - wrappedCmd.RefreshFlags() + wrappedCmd := cmdgroup.NewCommandWithGroupFlag(cmd) + wrappedCmd.AddFlagGroup(jobGroup) + wrappedCmd.AddFlagGroup(jobTaskGroup) + wrappedCmd.AddFlagGroup(pipelineGroup) } diff --git a/libs/cmdgroup/command.go b/libs/cmdgroup/command.go index ad3ba6efa2..a2a776935d 100644 --- a/libs/cmdgroup/command.go +++ b/libs/cmdgroup/command.go @@ -15,12 +15,6 @@ type CommandWithGroupFlag struct { flagGroups []*FlagGroup } -func (c *CommandWithGroupFlag) RefreshFlags() { - for _, fg := range c.flagGroups { - c.cmd.Flags().AddFlagSet(fg.flagSet) - } -} - func (c *CommandWithGroupFlag) Command() *cobra.Command { return c.cmd } @@ -60,10 +54,9 @@ func NewCommandWithGroupFlag(cmd *cobra.Command) *CommandWithGroupFlag { return cmdWithFlagGroups } -func (c *CommandWithGroupFlag) AddFlagGroup(name string) *FlagGroup { - fg := &FlagGroup{name: name, flagSet: pflag.NewFlagSet(name, pflag.ContinueOnError)} +func (c *CommandWithGroupFlag) AddFlagGroup(fg *FlagGroup) { c.flagGroups = append(c.flagGroups, fg) - return fg + c.cmd.Flags().AddFlagSet(fg.FlagSet()) } type FlagGroup struct { @@ -72,6 +65,10 @@ type FlagGroup struct { flagSet *pflag.FlagSet } +func NewFlagGroup(name string) *FlagGroup { + return &FlagGroup{name: name, flagSet: pflag.NewFlagSet(name, pflag.ContinueOnError)} +} + func (c *FlagGroup) Name() string { return c.name } diff --git a/libs/cmdgroup/command_test.go b/libs/cmdgroup/command_test.go index ea33e66650..9122c78090 100644 --- a/libs/cmdgroup/command_test.go +++ b/libs/cmdgroup/command_test.go @@ -18,18 +18,19 @@ func TestCommandFlagGrouping(t *testing.T) { } wrappedCmd := NewCommandWithGroupFlag(cmd) - jobGroup := wrappedCmd.AddFlagGroup("Job") + jobGroup := NewFlagGroup("Job") fs := jobGroup.FlagSet() fs.String("job-name", "", "Name of the job") fs.String("job-type", "", "Type of the job") + wrappedCmd.AddFlagGroup(jobGroup) - pipelineGroup := wrappedCmd.AddFlagGroup("Pipeline") + pipelineGroup := NewFlagGroup("Pipeline") fs = pipelineGroup.FlagSet() fs.String("pipeline-name", "", "Name of the pipeline") fs.String("pipeline-type", "", "Type of the pipeline") + wrappedCmd.AddFlagGroup(pipelineGroup) cmd.Flags().BoolP("bool", "b", false, "Bool flag") - wrappedCmd.RefreshFlags() buf := bytes.NewBuffer(nil) cmd.SetOutput(buf)